-
Notifications
You must be signed in to change notification settings - Fork 389
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
Produce telemetry on yellow triangles in the dependency tree #6593
Produce telemetry on yellow triangles in the dependency tree #6593
Conversation
This was only used in one place, and the called method replicated the guard logic. Moving to ValueTask avoids the allocation and simplifies the interface ahead of new additions to it, for which the concept of IsActive does not apply.
...udio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependencyTreeTelemetryService.cs
Show resolved
Hide resolved
src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/TelemetryPropertyName.cs
Outdated
Show resolved
Hide resolved
@@ -289,6 +292,11 @@ private void DisposeCore() | |||
projectItemSpecs), | |||
token); | |||
|
|||
if (updatedSnapshot != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could result in us sending an incomplete snapshot right? ie You do a big branch switch, close VS before the tree has finished we might send unresolved states that are misleading (ie its just that we never received design-time build data).
Perhaps we should only update the snapshot state when we're up-to-date with the latest design-time build data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the behaviour is the same as before. updatedSnapshot
is only non-null
if the TryUpdate
method actually made a change in response to the update.
Reading your comment again I think I see what you're saying. I'll take a look to see whether we can only flow this through when we get a JointRule
update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its more than when we get a JointRule; we can get inbetween states that might fixed themselves once we've caught up. For example, go change the version number a commonly used package in Package.targets to see this - you'll a bunch of errors that will eventually go away, you don't ever want to report a telemetry event during that because we'll get misleading data (if had let it finish, they would have been resolved).
I think we'll want to make sure that we're up-to-date with the latest version of JointRule or perhaps operation progress. @lifengl Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #6575.
On project unload, publish a telemetry event with data about a project's dependencies.
A sample event:
{"netcoreapp3.1":{"Framework":{"TotalCount":1,"UnresolvedCount":0},"Package":{"TotalCount":1,"UnresolvedCount":0}},"net48":{"Assembly":{"TotalCount":9,"UnresolvedCount":0},"Package":{"TotalCount":1,"UnresolvedCount":0}}}
Microsoft Reviewers: Open in CodeFlow