-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Rationalize and simplify telemetry logging apis #85062
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
Merged
dibarbet
merged 34 commits into
dotnet:main
from
dibarbet:dibarbet-telemetry-consolidation
Sep 3, 2026
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
e3b2647
Consolidate Roslyn telemetry onto one event sink and one metric sink
dibarbet 6c3501a
Fix Razor VS Code metric data loss; delete the LSP host's ITelemetryR…
dibarbet bd9da50
Move the Razor telemetry bridge's translation into the bridge
dibarbet 16585d5
Rewrite added comments to describe current behavior
dibarbet 6381484
Revert documentation changes
dibarbet d91ab67
Fan out events from RoslynTelemetry; delete AggregateEventSink
dibarbet de78c12
Drop the unused ambient session plumbing; one metric sink per session
dibarbet 1db5b85
Wire the language server's metric sink; separate host and dynamic eve…
dibarbet 16e79fc
Reduce event sink registration to AddEventSink returning a registration
dibarbet c3767fe
Rename LanguageServerTelemetryService; give metric sinks the same reg…
dibarbet 23513fc
Collapse each host's telemetry teardown to one disposables array
dibarbet bf1f140
Register the trace and output window sinks only while they are enabled
dibarbet 34e618a
Keep the ETW sink dynamic in the OOP process
dibarbet 62d18fd
Put the LanguageServerTelemetry attribute on its own line
dibarbet dd7bd60
Remove dead telemetry surface and tighten the new comments
dibarbet 97cad27
Fix regressions found in review
dibarbet 1f97462
Don't dispose the flush loop's cancellation source
dibarbet ddb29cf
Register the ETW sink on demand in devenv too
dibarbet 9d50d06
Pin the metric naming and document two deliberate gaps
dibarbet c1fe39b
Drop forward-looking notes from the telemetry comments
dibarbet 03f12e8
Flush the language server's request telemetry when a server shuts down
dibarbet 4f9491a
Test OOP logger enablement without a telemetry session
dibarbet af1028f
Revert the block pairing bitmask
dibarbet e05ecb0
Apply PR feedback on comments
dibarbet 0195260
Close the flush race, name the instrument kinds, share the naming
dibarbet 4a0b8e2
Make the telemetry test accessors static
dibarbet 6a11d59
Fix analyzer violations in projects with stricter settings
dibarbet 9f43ea2
Enable nullable in the new language server telemetry tests
dibarbet 50d3af4
only allow sink to be registered once
dibarbet 0397ff8
sort
dibarbet 1f2ddb5
Address telemetry consolidation review feedback
dibarbet 0da1307
potential simplification for flush lock
dibarbet f203195
simplify tests
dibarbet aca50e9
Address metric sink review feedback
dibarbet File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
63 changes: 63 additions & 0 deletions
63
...er/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LanguageServerRequestTelemetryTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Collections.Generic; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.CodeAnalysis.Internal.Log; | ||
| using Microsoft.CodeAnalysis.Telemetry; | ||
| using Microsoft.VisualStudio.Telemetry; | ||
| using Microsoft.VisualStudio.Telemetry.Metrics.Events; | ||
| using Roslyn.LanguageServer.Protocol; | ||
| using Xunit; | ||
| using Xunit.Abstractions; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests; | ||
|
|
||
| /// <summary> | ||
| /// Covers the language server's request telemetry end to end: a real LSP request records aggregated | ||
| /// measurements, and shutting the server down posts them to the telemetry session. | ||
| /// </summary> | ||
| public sealed class LanguageServerRequestTelemetryTests(ITestOutputHelper testOutputHelper) | ||
| : AbstractLanguageServerHostTests(testOutputHelper) | ||
| { | ||
| private sealed class RecordingPoster : VSMetricSink.IMetricPoster | ||
| { | ||
| public List<TelemetryEvent> PostedEvents { get; } = []; | ||
|
|
||
| public bool IsOptedIn => true; | ||
|
|
||
| public void Post(TelemetryEvent telemetryEvent, TelemetryMetricEvent metricEvent) | ||
| => PostedEvents.Add(telemetryEvent); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task RealRequestsProduceAggregatedTelemetry() | ||
| { | ||
| var poster = new RecordingPoster(); | ||
| using var sink = VSMetricSink.TestAccessor.CreateSink(poster); | ||
| using var registration = RoslynTelemetry.AddMetricSink(sink); | ||
|
|
||
| var server = await CreateLanguageServerAsync(); | ||
|
|
||
| // Measurements accumulate against instruments; nothing is posted until a flush. | ||
| Assert.Empty(poster.PostedEvents); | ||
|
|
||
| // Shutting the server down disposes its RequestTelemetryLogger, whose Dispose flushes. | ||
| await server.DisposeAsync(); | ||
|
|
||
| // One event per instrument, and the method tag discriminates buckets: initialize and | ||
| // initialized are separate instruments under the same event name. | ||
| var durations = poster.PostedEvents.FindAll(e => e.Name == "vs/ide/vbcs/lsp/requestduration"); | ||
| Assert.Contains(durations, e => Equals(e.Properties["vs.ide.vbcs.lsp.requestduration.method"], Methods.InitializeName)); | ||
| Assert.Contains(durations, e => Equals(e.Properties["vs.ide.vbcs.lsp.requestduration.method"], Methods.InitializedName)); | ||
| Assert.All(durations, e => Assert.Equal( | ||
| WellKnownLspServerKinds.CSharpVisualBasicLspServer.ToTelemetryString(), | ||
| e.Properties["vs.ide.vbcs.lsp.requestduration.server"])); | ||
|
|
||
| var counters = poster.PostedEvents.FindAll(e => e.Name == "vs/ide/vbcs/lsp/requestcounter"); | ||
| Assert.Contains(counters, e => Equals(e.Properties["vs.ide.vbcs.lsp.requestcounter.method"], Methods.InitializeName)); | ||
|
|
||
| Assert.Contains(poster.PostedEvents, e => e.Name == "vs/ide/vbcs/lsp/timeinqueue"); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If this was running in parallel with another test, things could go bad right?
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.
yes, but we don't run tests in parallel within the same run (we only parallelize at the process level). There's likely much more that is broken if we tried to parallelize within a run.