Add telemetry for the time and files outputted for source generators #63088
Conversation
| // Report the telemetry now before the solution is closed; since this will be reported per solution session ID, it means | ||
| // we can distinguish how many solutions have generators versus just overall sessions. |
There was a problem hiding this comment.
If there's a different way to do this by capturing the context when the solution is opened or something else, I'd love to know it.
.../Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs
Show resolved
Hide resolved
| // We'll log one event per generator | ||
| Logger.Log(functionId, KeyValueLogMessage.Create(map => | ||
| { | ||
| map[nameof(telemetryKey.Identity.AssemblyName) + "Hashed"] = AnalyzerNameForTelemetry.ComputeSha256Hash(telemetryKey.Identity.AssemblyName); |
There was a problem hiding this comment.
At this point I'm hashing everything which I don't like; in the analyzer case we treat built-in analyzers specially and don't do this, but I don't think I have a way to get an equal control in place. I think we could probably do something like look at the public key to determine if it's a Microsoft produced generator, but not sure if that actually works. While we sort this out, we'll go for this pessimistic approach at the moment.
There was a problem hiding this comment.
do we need this? can't we just mark teh bucket a pii information?
There was a problem hiding this comment.
@CyrusNajmabadi I'm matching what we do for analyzers here; it's entirely possible this can be simplified with that newer support. Happy to investigate further and try to fix both at once.
...o/Core/Def/Workspace/VisualStudioSourceGeneratorTelemetryCollectorWorkspaceServiceFactory.cs
Show resolved
Hide resolved
jasonmalinowski
force-pushed
the
add-telemetry-for-generators
branch
from
f5146eb
to
e6c7f0e
Compare
...o/Core/Def/Workspace/VisualStudioSourceGeneratorTelemetryCollectorWorkspaceServiceFactory.cs
Show resolved
Hide resolved
| { | ||
| // Report the telemetry now before the solution is closed; since this will be reported per solution session ID, it means | ||
| // we can distinguish how many solutions have generators versus just overall sessions. | ||
| _visualStudioWorkspaceInstance.ReportStatisticsAndClear(FunctionId.SourceGenerator_SolutionStatistics); |
There was a problem hiding this comment.
worried about ths perf impact.
There was a problem hiding this comment.
Ah, I see GitHub somehow ate my prior comment. Any idea if there's a way I can capture the context but still report telemetry async? Because that'd be perfect here.
There was a problem hiding this comment.
Chatting with the team it sounds like what I want doesn't immediately exist; I'll do a validation run with this to ensure we don't see regressions.
There was a problem hiding this comment.
ok. i look forward to DaveKean's feedback here :D
.../Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
Previously the "assembly name" was the fully qualified name; breaking out the assembly version separately means we can easily use that later in telemetry.
Not sure why these were in the shared project; nothing uses them there.
jasonmalinowski
force-pushed
the
add-telemetry-for-generators
branch
from
e6c7f0e
to
edefbd1
Compare
...o/Core/Def/Workspace/VisualStudioSourceGeneratorTelemetryCollectorWorkspaceServiceFactory.cs
Show resolved
Hide resolved
|
|
||
| namespace Microsoft.CodeAnalysis.SourceGeneratorTelemetry | ||
| { | ||
| internal interface ISourceGeneratorTelemetryCollectorWorkspaceService : IWorkspaceService |
There was a problem hiding this comment.
nit: workspace services don't have to have Workspace in their name.
.../Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
.../Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
.../Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
.../Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
.../Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
| { | ||
| } | ||
|
|
||
| public static string GetGeneratorAssemblyName(ISourceGenerator generator) | ||
| { | ||
| return generator.GetGeneratorType().Assembly.FullName!; | ||
| return generator.GetGeneratorType().Assembly.GetName().Name!; |
There was a problem hiding this comment.
The full name is the form "MySourceGenerator, Version=1.2.3.4"; which means we'd be capturing the version implicitly in the name. It didn't really matter until now, when we want those reported as separate telemetry points.
This adds telemetry for the overall time being spent running generators during a single solution session, and whether individual generators on average actually outputted files or not. This can help us generally understand the execution cost of generators in the wild, as well as how many solutions have generators but aren't actually getting outputs from them.
Since the names of the generator/type may reveal sensitive things (company names), we'll hash this; this is sharing the same code we use for analyzers.
jasonmalinowski
force-pushed
the
add-telemetry-for-generators
branch
from
edefbd1
to
dde5237
Compare
|
The internal performance validation build was clean, so merging. |
This adds telemetry for the overall time being spent running generators during a single solution session, and whether individual generators on average actually outputted files or not. This can help us generally understand the execution cost of generators in the wild, as well as how many solutions have generators but aren't actually getting outputs from them.
There's a lot more work to come after this -- additional data, showing this in the UI, etc, but this at least gets the core parts in place.