-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add telemetry indicating when file-based programs are used #80538
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
Conversation
[property: JsonPropertyName("FileCounts")] IEnumerable<int> FileCounts, | ||
[property: JsonPropertyName("SdkStyleProject")] bool SdkStyleProject) | ||
[property: JsonPropertyName("SdkStyleProject")] bool SdkStyleProject, | ||
[property: JsonPropertyName("HasSolutionFile")] bool HasSolutionFile, |
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.
Unfortunately this will need a client side update
https://github.com/dotnet/vscode-csharp/blob/f05060119a9c236cf8e315cf3434e7be20df0e54/src/shared/projectConfiguration.ts#L48
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.
You can confirm it is working in VSCode by
- Run command
Developer: Set Log Level
toTrace
- Open the telemetry window (command
Developer: Show Telemetry
) - You should see a log entry like
[Extension Telemetry] ms-dotnettools.csharp/ProjectConfiguration
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 sweat, I will open a PR on the client, and try to verify.
For this, CDK reports its own project configuration-like event when it loads a project. Historically the C# project configuration event was only used when CDK was not active. Now we will have to make sure the queries know that both CDK and C# may be reporting this telemetry from the same session. |
Opened dotnet/vscode-csharp#8673 OK, I verified that the telemetry is sent. However we basically only get events for the "file based program" and "rich misc file" cases. It's not clear to me if that's a problem that needs solving or not. I think if the queries can pull together both CDK and C# extension telemetry for the same session, then it should be fine as-is. |
We do get them for first load of real projects right? Or are you testing in a CDK scenario? |
Today we aggregate the 2 different "project load" events from C# and C#DK to get a full picture. My understanding is when C#DK is active, the load event from C# is suppressed (i.e. not sent) so I'd expect this change to ultimately result in new property(ies) on both the project load events. |
I was testing in a CDK scenario. Here is an example of the file-based app telemetry with C#+CDK active (formatting applied manually).
Then, when C# only is running, there are two events in my manual test--one for the ordinary project loading, and one for the FBP. Trimmed down the properties to an "illustrative" subset:
In the C#+CDK case, I wasn't able to tell which telemetry event from CDK I should look for, re: the load of the ordinary project. Any tips on that would be appreciated.
It appears that the way this suppression works is, the "base C# project system", is simply not used for ordinary projects, and thus its telemetry events are not sent for ordinary projects. With this change, the "base C# project system", will start sending events for "rich misc files" and "file-based programs", regardless of whether CDK is loaded. |
C#DK telemetry is reported a bit differently. You can use VSTelemetryMonitor with the steps outlined here to enable it.
Yup, this is correct.
Yeah this is what will happen. The C#DK event will also be sent for the normal solution load. So a single session can now contain the project load events from both C# and C#DK. |
Will do the work to make vscode-csharp consume this build, and finally report the new telemetry, once an official build for this change is available. |
* upstream/main: (149 commits) Fix Simplify and add additional asserts Update proposal adjuster to acquire feature flags from VS (dotnet#80541) Remove First/Last Docs Docs Deprecate method and use workaround Remove support for ccreating EmbeddedStrings Remove methods Move more into extensions Move more into extensions Proper slice Proper slice Proper slice Move to slices Extract extensions Downstream Simplify the virtual char api Add telemetry indicating when file-based programs are used (dotnet#80538) Fix thread safety issue in BuildServerConnection.TryCreateServer environment variable handling (dotnet#80498) ...
Closes dotnet/vscode-csharp#8586
@dibarbet @jasonmalinowski could you please take a look?
One bit of info that I didn't include, was: "did project initialization complete or not". I don't think that was one of the bits @DamianEdwards was asking for. It was more something I was interested in knowing, though not essential. I just didn't see an obvious way of obtaining that info during the design-time build.
I did verify that expected bits of info are present when debugging when C#+CDK are in use.
I wasn't sure if changes on CDK side are needed, in order to get a full picture of the case that file-based programs are not being used. I also wasn't sure if consumption side changes to
ProjectLoadTelemetryEvent
are needed, to ensure the telemetry actually makes it to the destination.