Create per-server daemon telemetry sessions - #85155
Open
dibarbet wants to merge 12 commits into
Open
Conversation
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
dibarbet
force-pushed
the
dev/dibarbet/daemon-telemetry-sessions
branch
from
September 4, 2026 23:04
7d3a453 to
f7dbc47
Compare
dibarbet
marked this pull request as ready for review
September 4, 2026 23:10
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
There are merge-blocking gaps around cancellation handling in ServiceBrokerConnectHandler and a regression in telemetry flush isolation coverage that should be restored.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
 |
src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/BrokeredServices/ServiceBrokerConnectHandler.cs — The broker connection is started via Task.Run with CancellationToken.None, so a cancellation… |
 |
src/Workspaces/CoreTest/Log/RoslynTelemetryTests.cs — RecordingMetricSink.Flush was changed to a no-op, which removes the ability for these tests to… |
What changed in this PR
Introduces per-language-server telemetry isolation when running under the daemon host model, so each server connection can own its own telemetry session and sinks without cross-contamination between concurrently hosted servers.
Changes:
- Partition daemon discovery/pipe naming by effective telemetry level and plumb
--telemetryLevelthrough thin-client argument parsing. - Create/own per-server
RoslynTelemetry+ optionalTelemetrySessionper daemon connection; re-establish telemetry ambient across request dispatch, service creation, broker activations, and async boundaries. - Move remaining language-server request/project-load/Razor bridge telemetry state to per-server ownership and add/adjust unit tests for daemon isolation and session settings.
| File | Description |
|---|---|
| src/Workspaces/CoreTest/Log/RoslynTelemetryTests.cs | Adjusts core telemetry unit tests alongside new per-instance expectations. |
| src/LanguageServer/roslyn-language-server/ThinClientArguments.cs | Parses and carries --telemetryLevel and forwards it to the server args. |
| src/LanguageServer/roslyn-language-server/Program.cs | Passes parsed thin-client arguments into daemon connect flow. |
| src/LanguageServer/roslyn-language-server/DaemonClient.cs | Partitions daemon pipe selection by resolved telemetry level. |
| src/LanguageServer/Protocol/RoslynLanguageServer.cs | Registers RoslynTelemetry as a base service for LSP services. |
| src/LanguageServer/Protocol/LspServices/LspServices.cs | Captures and reapplies per-server telemetry ambient during lazy service creation. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Telemetry/VSCodeRequestTelemetryLogger.cs | Converts request telemetry logger state from static to per-server instance state. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Telemetry/LanguageServerTelemetry.cs | Creates standalone/devkit sessions per server, registers sinks per RoslynTelemetry, and correlates daemon session IDs. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs | Resolves telemetry level once, creates the process/default session, and reports features telemetry at shutdown. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/LanguageServer/LanguageServerHost.cs | Creates/owns per-server telemetry instance/session in daemon mode and scopes ambient telemetry for server lifetime. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/LanguageServer/LanguageServerConnectionManager.cs | Plumbs daemon session ID into per-connection server host creation. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/WorkspaceProjectFactoryService.cs | Uses per-server request telemetry logger instance for project-load start events. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/Razor/TelemetryReporterWrapper.cs | Bridges Razor telemetry via a RoslynTelemetry→TelemetrySession association. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/ProjectTelemetry/ProjectLoadTelemetryReporter.cs | Makes project-load correlation ID per-server instead of static/process-wide. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/ProjectInitializationHandler.cs | Uses per-server request telemetry logger instance for initialization-complete events. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs | Captures per-server telemetry and reapplies it for async project reload batches. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileWatching/LspFileChangeWatcher.cs | Captures/reapplies per-server telemetry across watcher disposal/unregistration async boundary. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileWatching/DefaultFileChangeWatcher.FileChangeContext.cs | Captures/reapplies per-context telemetry for filesystem watcher callbacks. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/DevKitProjectLoadingServiceContributor.cs | Passes per-server request telemetry logger into brokered project services. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/BrokeredServices/ServiceBrokerConnectHandler.cs | Re-establishes telemetry ambient when connecting broker services under suppressed execution context. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/BrokeredServices/BrokeredServiceBridgeProvider.cs | Documents execution-context capture point for broker activations. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/WorkspaceProjectFactoryServiceTests.cs | Updates test wiring for per-server request telemetry logger instance. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/VSMetricSinkTests.cs | Refactors test helpers (moved to shared utilities file). |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/RecordingTelemetrySinks.cs | Adds reusable recording sinks/poster for telemetry-related unit tests. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerHostTests.cs | Updates daemon test harness to model process vs per-server telemetry sessions. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/TelemetryReporterTests.cs | Updates reporter/wrapper construction and adds standalone session settings validation. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ServiceBrokerFactoryTests.cs | Extends coverage to ensure broker calls/telemetry are isolated per server. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LanguageServerRequestTelemetryTests.cs | Refactors/moves test poster helper usage. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Daemon/LanguageServerDaemonTests.cs | Adds test asserting isolated per-server telemetry sessions correlated to daemon session. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Daemon/DaemonPipeNameTests.cs | Updates expectations and adds test for telemetry-level partitioning. |
| src/LanguageServer/DaemonConnection/TelemetryLevelResolver.cs | Centralizes “effective telemetry level” resolution (arg vs env). |
| src/LanguageServer/DaemonConnection/DaemonPipeName.cs | Includes telemetry level in daemon pipe-name hashing input. |
Suppressed comments (1)
src/Workspaces/CoreTest/Log/RoslynTelemetryTests.cs:169
- This PR removes the only regression test that verifies RoslynTelemetry.Flush() only flushes sinks registered on the instance being flushed (not other instances). With per-server telemetry instances, preserving this isolation invariant seems important; consider restoring the test.
Partition daemon clients by telemetry consent and isolate server telemetry attribution and lifetime. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Let each LanguageServerHost create and dispose its daemon child session, while Protocol captures only the ambient RoslynTelemetry instance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: beeb17b9-9104-41c1-a3c0-f95dc9df4d39
Capture telemetry from the ambient context during host and service construction, and remove telemetry plumbing from the connection manager and daemon source APIs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: beeb17b9-9104-41c1-a3c0-f95dc9df4d39
Remove the unused standalone telemetry owner and keep the daemon root owner scoped to the daemon task. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: beeb17b9-9104-41c1-a3c0-f95dc9df4d39
StreamJsonRpc dispatches inbound brokered calls on the execution context captured when the connection was created, so they already carry the owning server's telemetry. Verified by a test covering both a brokered service call and Dev Kit's initialization observer. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: beeb17b9-9104-41c1-a3c0-f95dc9df4d39
Share telemetry test sinks and remove unrelated formatting and cleanup from the per-server telemetry diff. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c5d4f77e-69f4-4116-aa63-b7c1ab0822cb
dibarbet
force-pushed
the
dev/dibarbet/daemon-telemetry-sessions
branch
from
September 10, 2026 00:00
f7dbc47 to
158508c
Compare
JoeRobich
approved these changes
Sep 12, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR stacks on #85151. It gives each language server hosted by a daemon its own telemetry session.
vs.roslyn.languageserver.daemonsessionidfor correlation.LanguageServerHostcreate and own each per-server telemetry session from the ambient daemon owner. The connection manager remains transport/lifetime coordination only, and the daemon source captures its daemon instance when constructed for explicit lifecycle logging.RoslynLanguageServer; request dispatch and lazy service construction reapply it, while non-request asynchronous boundaries captureRoslynTelemetry.Currentand restore it with nested scopes.Microsoft Reviewers: Open in CodeFlow