Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable API analysis if telemetry is disabled #47218

Merged
merged 1 commit into from
Aug 28, 2020

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Aug 27, 2020

Related to #41411

@sharwell sharwell requested a review from a team as a code owner August 27, 2020 23:33
@mavasani
Copy link
Contributor

@sharwell Just to confirm, you are seeing this incremental analyzer kick in only when FSA is enabled correct? Otherwise, given how expensive it is, I am wondering if we need to optimize it further.

@CyrusNajmabadi
Copy link
Member

No problem with this change. I have an open question about if we actually use this data in any way. i.e. we collect it... and what do we then do with it? If the answer is currently 'nothing', i would be thrilled to just rip it out entirely (as part of another change).

@mavasani
Copy link
Contributor

and what do we then do with it?

I believe the telemetry analyzer was added to collect telemetry for partner teams. @jinujoseph might have context which team(s) uses this data.

@sharwell
Copy link
Member Author

you are seeing this incremental analyzer kick in only when FSA is enabled correct

I always have FSA enabled, but I'm not sure how they would be related.

@mavasani
Copy link
Contributor

I always have FSA enabled, but I'm not sure how they would be related.

Never mind, seems like the intent was to have this analyzer on by default for everyone to collect telemetry on the low-priority solution crawler queue. If not, the analyzer can check the current BackgroundAnalysisScope option value and bail out unless it is set to EntireSolution, but then we will collect telemetry only for users that have this option set. I think we should discuss with whichever partner team is using this telemetry whether this needs to be on by default or opt-in only for those who explicitly set BackgroundAnalysisScope.EntireSolution

@tmat
Copy link
Member

tmat commented Aug 28, 2020

There has been an email conversation in past with @DamianEdwards about this telemetry.

@mavasani
Copy link
Contributor

Thanks. Even if we want it on by default, we should throttle it to run very limited number of times per solution, maybe just once when the solution is first loaded.

Any case, this seems like a classic case for offline analysis, not something that should run on user's CPU and compete with resources for functionality that is important to them @vatsalyaagrawal

@sharwell sharwell merged commit 1309e6b into dotnet:master Aug 28, 2020
@ghost ghost added this to the Next milestone Aug 28, 2020
@sharwell sharwell deleted the handle-optout branch August 28, 2020 14:58
@allisonchou allisonchou modified the milestones: Next, 16.8.P3 Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants