Make RoslynTelemetry instance-based with an ambient Current by dibarbet · Pull Request #85151 · dotnet/roslyn

dibarbet · 2026-09-03T20:02:40Z

This is PR 1 of 3 and is a pure, behavior-preserving refactor. With nothing calling SetCurrent, all telemetry resolves to one private default instance, exactly preserving today's semantics.

In language-server daemon mode, one process hosts multiple independent LSP servers. RoslynTelemetry currently holds process-wide static sink arrays, so every client's telemetry collapses into one TelemetrySession. Making telemetry instance-based with an ambient Current lays the groundwork for a follow-up to give each logical server its own session.

RoslynTelemetry is now internal sealed partial, with instance sink arrays and instance AddEventSink, AddMetricSink, Flush, Log, LogBlock, LogBlockTime, Count, Record, and RecordBlockTime APIs. Current and SetCurrent remain static, while the default instance is private. Logger keeps its exact static surface and forwards to Current, leaving the roughly 436 Logger.* call sites untouched.

Current reads the ambient AsyncLocal directly and falls back to the private default instance. This keeps asynchronous control-flow semantics exact, including child tasks that outlive the scope where they captured the ambient instance.

Validation:

dotnet build Compilers.slnf — clean
dotnet build Ide.slnf — clean
RoslynTelemetryTests — 15/15 passed on both net10.0 and net472

Follow-ups will add per-server telemetry sessions (PR 2) and move the Features telemetry aggregators onto the instance (PR 3).

Microsoft Reviewers: Open in CodeFlow

azure-pipelines · 2026-09-03T20:03:02Z

Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 415c03c3-4e89-40bd-89c8-c28b3cab5da8

azure-pipelines · 2026-09-04T23:08:46Z

Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Copilot review overview

🟡 Changes recommended

It introduces potential perf overhead on hot telemetry no-op paths and leaves some flush sites vulnerable to flushing the wrong instance once ambient telemetry is used.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 3 · 1

New issues introduced by this change (4)

Severity	Finding
	`src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Telemetry/LanguageServerTelemetry.cs` — LanguageServerTelemetry registers event/metric sinks against RoslynTelemetry.Current in…
	`src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs` — AbstractWorkspaceTelemetryService registers sinks against RoslynTelemetry.Current during…
	`src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry.cs` — RoslynTelemetry.Current now reads AsyncLocal on every telemetry call (via Logger and direct…
	`src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry.cs` — SetCurrent’s doc comment implies any disposal restores the previous instance, but the…

What changed in this PR

This PR refactors RoslynTelemetry from a static holder of process-wide sink state into an instance-based type with an ambient Current (via AsyncLocal), while keeping Logger’s static API surface intact by forwarding through RoslynTelemetry.Current.

Changes:

Convert RoslynTelemetry to internal sealed partial with instance sink storage and instance logging/metrics APIs, plus static Current/SetCurrent.
Update Logger and selected direct call sites to use RoslynTelemetry.Current.*.
Expand unit coverage to validate ambient flow, nesting, and instance isolation semantics.

File	Description
src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry.cs	Introduces ambient `Current`/`SetCurrent`, moves event logging to instance methods, updates test reset semantics.
src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry_Metrics.cs	Moves metric sinks + metric APIs to instance methods; updates timed metric block to target an instance.
src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry_LogBlock.cs	Makes log-block factory private and keeps block sink-capture behavior.
src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/Logger.cs	Preserves static `Logger` surface by forwarding calls to `RoslynTelemetry.Current`.
src/Workspaces/Remote/ServiceHub/Services/ProcessTelemetry/RemoteProcessTelemetryService.cs	Registers remote-process event sinks via `RoslynTelemetry.Current`.
src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/PerformanceTrackerService.cs	Routes performance telemetry through `RoslynTelemetry.Current`.
src/Workspaces/Remote/ServiceHub/Services/AssetSynchronization/RemoteAssetSynchronizationService.cs	Routes synchronization counters through `RoslynTelemetry.Current`.
src/Workspaces/CoreTest/Log/RoslynTelemetryTests.cs	Adds/updates tests for `Current`, `SetCurrent`, async flow, and per-instance behavior.
src/Workspaces/Core/Portable/Log/RoslynTelemetry.Workspaces.cs	Moves Workspaces-layer `LogBlockTime` helper to an instance method.
src/Workspaces/Core/Portable/CodeActions/CodeAction.cs	Uses `RoslynTelemetry.Current.RecordBlockTime` for preview timing.
src/VisualStudio/VisualStudioDiagnosticsToolWindow/PerfMargin/PerfMarginPanel.cs	Registers PerfMargin sink via `RoslynTelemetry.Current`.
src/VisualStudio/VisualStudioDiagnosticsToolWindow/OptionPages/PerformanceLoggersPage.cs	Registers Performance Logger sinks via `RoslynTelemetry.Current`.
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs	Registers integration-test sink via `RoslynTelemetry.Current`.
src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs	Updates test sink registration to `RoslynTelemetry.Current`.
src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests_Telemetry.cs	Updates “anything is listening” check to go through `RoslynTelemetry.Current`.
src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs	Registers sinks and flushes via `RoslynTelemetry.Current`.
src/LanguageServer/Protocol/Handler/Telemetry/RequestTelemetryLogger.cs	Routes LSP request telemetry through `RoslynTelemetry.Current`.
src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Telemetry/VSCodeRequestTelemetryLogger.cs	Routes VSCode-specific LSP telemetry through `RoslynTelemetry.Current`.
src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Telemetry/LanguageServerTelemetry.cs	Registers sinks and flushes via `RoslynTelemetry.Current`.
src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LanguageServerRequestTelemetryTests.cs	Updates metric sink registration to `RoslynTelemetry.Current`.
src/Features/Core/Portable/Diagnostics/Service/DocumentAnalysisExecutor.cs	Routes diagnostic timing telemetry through `RoslynTelemetry.Current`.
src/Features/Core/Portable/Diagnostics/Service/DiagnosticAnalyzerService_GetDiagnosticsForSpan.cs	Routes span diagnostics timing telemetry through `RoslynTelemetry.Current`.
src/Features/Core/Portable/CodeRefactorings/CodeRefactoringService.cs	Routes refactoring timing telemetry through `RoslynTelemetry.Current`.
src/Features/Core/Portable/CodeFixes/Service/CodeFixService.cs	Routes code-fix timing/delay telemetry through `RoslynTelemetry.Current`.
src/EditorFeatures/Core/Suggestions/SuggestedActionsSource_Async.cs	Routes suggested-actions timing telemetry through `RoslynTelemetry.Current`.
src/EditorFeatures/Core/Suggestions/SuggestedActions/EditorSuggestedAction.cs	Routes apply timing telemetry through `RoslynTelemetry.Current`.
src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs	Routes checksum updater counters through `RoslynTelemetry.Current`.
src/EditorFeatures/Core/InlineRename/UI/SmartRename/SmartRenameViewModel_Telemetry.cs	Routes SmartRename telemetry through `RoslynTelemetry.Current`.

+    /// <summary>
+    /// Sets the telemetry instance for the current asynchronous control flow. Disposing the result
+    /// restores the previous instance.
+    /// </summary>


jasonmalinowski · 2026-09-08T23:40:17Z

        var poster = new RecordingPoster();
        using var sink = VSMetricSink.TestAccessor.CreateSink(poster);
-        using var registration = RoslynTelemetry.AddMetricSink(sink);
+        using var registration = RoslynTelemetry.Current.AddMetricSink(sink);


Should the tests now just create instances of RoslynTelemetry() so we're not implicitly using static state anymore?

dibarbet · 2026-09-09T17:48:12Z

outdated

…instance-refactor

Copilot

Copilot review overview

🟡 Changes recommended

ExternalAccessFSharpResources.Designer.cs now has a namespace/ResourceManager base-name mismatch with the project’s embedded resource LogicalName and existing Microsoft.CodeAnalysis.ExternalAccess.FSharp.* consumers, which will break compilation and/or runtime resource lookup.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 · 1

New issues introduced by this change (1)

Severity	Finding
	`src/VisualStudio/ExternalAccess/Core/FSharp/ExternalAccessFSharpResources.Designer.cs` — The generated resources accessor is now in the `Microsoft.VisualStudio.LanguageServices.ExternalAcce…

Pre-existing issues (1)

Severity	Finding
	`src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry.cs` — SetCurrent’s doc comment implies any disposal restores the previous instance, but the… View comment

Issues resolved since last review (3)

Severity	Finding
	`src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry.cs` — RoslynTelemetry.Current now reads AsyncLocal on every telemetry call (via Logger and direct… View resolved comment
	`src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs` — AbstractWorkspaceTelemetryService registers sinks against RoslynTelemetry.Current during… View resolved comment
	`src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Telemetry/LanguageServerTelemetry.cs` — LanguageServerTelemetry registers event/metric sinks against RoslynTelemetry.Current in… View resolved comment

Files not reviewed (2)

src/LanguageServer/Protocol/LanguageServerProtocolResources.Designer.cs: Generated file
src/VisualStudio/ExternalAccess/Core/FSharp/ExternalAccessFSharpResources.Designer.cs: Generated file

Suppressed comments (1)

src/VisualStudio/ExternalAccess/Core/FSharp/ExternalAccessFSharpResources.Designer.cs:43

The ResourceManager base name was changed to Microsoft.VisualStudio.LanguageServices.ExternalAccess.FSharp.ExternalAccessFSharpResources, but the .csproj explicitly embeds the .resources under Microsoft.CodeAnalysis.ExternalAccess.FSharp.ExternalAccessFSharpResources.resources (LogicalName). This mismatch will cause resource lookups to fail at runtime even if compilation succeeds.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 415c03c3-4e89-40bd-89c8-c28b3cab5da8

Copilot

Copilot review overview

🟡 Changes recommended

Add the RecordBlockTime regression test and remove the unrelated generated-file rewrite.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 · 2

New issues introduced by this change (2)

Severity	Finding
	`src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry_Metrics.cs` — Please add a regression test for the instance captured by `RecordBlockTime`: start the scope on one…
	`src/LanguageServer/Protocol/LanguageServerProtocolResources.Designer.cs` — Could we drop this generated-file rewrite from the telemetry refactor?…

Pre-existing issues (1)

Severity	Finding
	`src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry.cs` — SetCurrent’s doc comment implies any disposal restores the previous instance, but the… View comment

Issues resolved since last review (1)

Severity	Finding
	`src/VisualStudio/ExternalAccess/Core/FSharp/ExternalAccessFSharpResources.Designer.cs` — The generated resources accessor is now in the `Microsoft.VisualStudio.LanguageServices.ExternalAcce… View resolved comment

Files not reviewed (1)

src/LanguageServer/Protocol/LanguageServerProtocolResources.Designer.cs: Generated file

-    public static IDisposable? RecordBlockTime(FunctionId functionId, string metricName)
-        => s_metricSinks.IsEmpty ? null : new TimedBlock(functionId, metricName);
+    public IDisposable? RecordBlockTime(FunctionId functionId, string metricName)
+        => _metricSinks.IsEmpty ? null : new TimedBlock(this, functionId, metricName);


+    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")]
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute()]
+    [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()]
+    internal class LanguageServerProtocolResources {


Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 415c03c3-4e89-40bd-89c8-c28b3cab5da8

Copilot

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review tier: Lite
Findings: 1 · 2

Pre-existing issues (3)

Severity	Finding
	`src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry_Metrics.cs` — Please add a regression test for the instance captured by `RecordBlockTime`: start the scope on one… View comment
	`src/LanguageServer/Protocol/LanguageServerProtocolResources.Designer.cs` — Could we drop this generated-file rewrite from the telemetry refactor?… View comment
	`src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/RoslynTelemetry.cs` — SetCurrent’s doc comment implies any disposal restores the previous instance, but the… View comment

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/Workspaces/CoreTest/Log/RoslynTelemetryTests.cs:60

This test only proves that two reads are reference-equal; it does not verify that Current fell back to the private default instance. It would still pass if an ambient custom instance leaked into the test or if Current returned another stable singleton. Use the new IsDefault helper in the assertion as well.

src/Workspaces/CoreTest/Log/RoslynTelemetryTests.cs:150

This adds coverage for event blocks capturing their sink list, but the new instance-specific RecordBlockTime and LogBlockTime paths are not tested when Current changes before disposal. A regression that resolves Current at timed-block disposal would therefore pass the suite; add an analogous two-instance test that starts the scope on the first instance, switches to the second, and verifies the measurement/event is delivered only to the first sink.

    public void BlockEndUsesSinksCapturedAtStart()

jasonmalinowski · 2026-09-09T20:39:11Z

-            ImmutableInterlocked.InterlockedExchange(ref s_metricSinks, []);
+            ImmutableInterlocked.InterlockedExchange(ref s_default._eventSinks, []);
+            ImmutableInterlocked.InterlockedExchange(ref s_default._metricSinks, []);
+            s_current.Value = null;


Maybe rename the method to "Reset" since it's doing more than just removing sinks now.

dotnet-policy-service Bot added the VSCode label Sep 3, 2026

dibarbet mentioned this pull request Sep 3, 2026

Create per-server daemon telemetry sessions #85155

Open

dibarbet and others added 2 commits September 4, 2026 15:43

Make RoslynTelemetry instance-based

6855984

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Remove RoslynTelemetry ambient fast path

7ea3695

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 415c03c3-4e89-40bd-89c8-c28b3cab5da8

dibarbet force-pushed the dev/dibarbet/telemetry-instance branch from 15b1ba6 to 7ea3695 Compare September 4, 2026 22:44

dibarbet marked this pull request as ready for review September 4, 2026 23:08

Copilot AI lite review requested due to automatic review settings September 4, 2026 23:08

dibarbet requested a review from a team as a code owner September 4, 2026 23:08

Copilot started reviewing on behalf of dibarbet September 4, 2026 23:09 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

jasonmalinowski approved these changes Sep 8, 2026

View reviewed changes

dibarbet added 2 commits September 9, 2026 10:54

Merge remote-tracking branch 'upstream/main' into dibarbet-telemetry-…

595da3c

…instance-refactor

feedback

787cb2b

Copilot AI review requested due to automatic review settings September 9, 2026 18:55

Copilot started reviewing on behalf of dibarbet September 9, 2026 18:56 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread src/VisualStudio/ExternalAccess/Core/FSharp/ExternalAccessFSharpResources.Designer.cs Outdated

Restore generated FSharp resources

949e99d

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 415c03c3-4e89-40bd-89c8-c28b3cab5da8

Copilot AI review requested due to automatic review settings September 9, 2026 19:53

Copilot started reviewing on behalf of dibarbet September 9, 2026 19:54 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Restore generated language server resources

dc012e9

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 415c03c3-4e89-40bd-89c8-c28b3cab5da8

Copilot AI review requested due to automatic review settings September 9, 2026 20:12

Copilot started reviewing on behalf of dibarbet September 9, 2026 20:12 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

jasonmalinowski approved these changes Sep 9, 2026

View reviewed changes

dibarbet merged commit 6928b14 into main Sep 9, 2026
24 checks passed

dotnet-policy-service Bot added this to the Next milestone Sep 9, 2026

This was referenced Sep 10, 2026

[release/11.0.1xx] Source code updates from dotnet/roslyn dotnet/dotnet#9225

Merged

[main] Source code updates from dotnet/roslyn dotnet/dotnet#9228

Merged

dotnet-bot mentioned this pull request Sep 11, 2026

[Automated] PRs inserted in VS build main-12211.24 #85259

Closed

dibarbet mentioned this pull request Sep 11, 2026

Update roslyn to 5.12.0-1.26461.1 dotnet/vscode-csharp#9751

Merged

Navigation Menu

Uh oh!

Make RoslynTelemetry instance-based with an ambient Current - #85151

Make RoslynTelemetry instance-based with an ambient Current#85151
dibarbet merged 6 commits into
mainfrom
dev/dibarbet/telemetry-instance

dibarbet commented Sep 3, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Sep 3, 2026

Uh oh!

azure-pipelines Bot commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jasonmalinowski Sep 8, 2026

Uh oh!

Uh oh!

dibarbet commented Sep 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

jasonmalinowski Sep 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dibarbet commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Sep 3, 2026

Uh oh!

azure-pipelines Bot commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jasonmalinowski Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dibarbet commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

jasonmalinowski Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dibarbet commented Sep 3, 2026 •

edited

Loading

dibarbet commented Sep 9, 2026 •

edited

Loading