Repository navigation
Reliable CLI telemetry via a persist-then-drain OpenTelemetry exporter - #55211
Conversation
Azure.Monitor.OpenTelemetry.Exporter + OpenTelemetry are designed for long-running services; their buffer-and-POST-on-shutdown model is unreliable for short-lived dotnet CLI processes that exit before uploads complete. Replace the AddAzureMonitorTraceExporter registration with a two-phase pipeline: - Phase 1 (persist): a custom BaseExporter<Activity> driven by a simple (synchronous) export processor maps each span/event to the Application Insights (Breeze) wire format and writes it to durable on-disk storage via OpenTelemetry.PersistentStorage.FileSystem before the process exits. - Phase 2 (drain): a background worker started on CLI startup leases persisted blobs and POSTs them to the Azure Monitor ingestion endpoint, deleting on success and retaining for retry on failure. It never blocks or fails the CLI. The current run's telemetry is delivered by a subsequent invocation (eventual consistency across invocations). Metrics, OTLP export, and the local disk log are unchanged. Gated behind MICROSOFT_ENABLE_TELEMETRY_AZURE_MONITOR. Fixes #55184 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Microsoft.DotNet.Cli.Telemetry library that makes Azure Monitor telemetry reliable for short-lived CLI processes by persisting telemetry to durable storage synchronously, then draining/uploads persisted payloads opportunistically on subsequent CLI invocations. The dotnet and dotnet-aot CLIs are wired to use the persist-then-drain path locally, while CI continues to use the standard Azure Monitor exporter.
Changes:
- Added
Microsoft.DotNet.Cli.Telemetrylibrary implementing persist-then-drain exporters for traces and logs plus Azure Monitor–compatible Breeze serialization and upload/206 partial-success handling. - Updated
dotnetCLI telemetry wiring to use persist-then-drain locally and standard Azure Monitor exporting in CI. - Added unit tests for serializers/transport/uploader and an end-to-end CLI test validating persisted telemetry blobs.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.DotNet.Cli.Telemetry.Tests/PersistentStorageTelemetryUploaderTests.cs | Unit tests for drain behavior (success, failure, leasing, partial acceptance remainder). |
| test/Microsoft.DotNet.Cli.Telemetry.Tests/HttpTelemetryUploadTransportTests.cs | Unit tests for gzip upload and HTTP 200/206/5xx outcomes. |
| test/Microsoft.DotNet.Cli.Telemetry.Tests/BreezePartialContentTests.cs | Unit tests for parsing/reslicing retriable payloads from 206 partial-success responses. |
| test/Microsoft.DotNet.Cli.Telemetry.Tests/AzureMonitorTelemetrySerializerTests.cs | Unit tests validating Activity → Breeze envelope mapping and JSON validity. |
| test/Microsoft.DotNet.Cli.Telemetry.Tests/AzureMonitorLogSerializerTests.cs | Unit tests validating LogRecord → Breeze envelope mapping and correlation. |
| test/Microsoft.DotNet.Cli.Telemetry.Tests/AzureMonitorConnectionStringTests.cs | Unit tests for connection string parsing and defaults. |
| test/Microsoft.DotNet.Cli.Telemetry.Tests/Microsoft.DotNet.Cli.Telemetry.Tests.csproj | New MSTest.Sdk test project for the telemetry library. |
| test/dotnet.Tests/TelemetryTests/PersistentStorageTelemetryE2ETests.cs | E2E test ensuring CLI persists telemetry blobs to configured storage directory. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/README.md | Library documentation describing persist-then-drain behavior and mappings. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/PersistentStorageTracerProviderBuilderExtensions.cs | Trace exporter registration extension (OTel-style Add…Exporter). |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/PersistentStorageTraceExporter.cs | Trace exporter that persists Breeze payloads and starts background drain. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/PersistentStorageTelemetryOptions.cs | Options type for connection string, storage directory, lease/max drain behavior. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/PersistentStorageLoggerOptionsExtensions.cs | Log exporter registration extension (OTel-style Add…Exporter). |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/PersistentStorageLogExporter.cs | Log exporter persisting Breeze payloads and starting background drain. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Microsoft.DotNet.Cli.Telemetry.csproj | New internal (IsPackable=false) AOT/trim-safe library project + metadata plumbing. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/TelemetryUploadResult.cs | Models upload outcome + optional retriable remainder for 206 partial-success. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/TelemetryResourceContextFactory.cs | Builds AI context tags (role/instance/version/sdkVersion) from OTel Resource. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/TelemetryResourceContext.cs | Record holding immutable per-process resource context used by serializers. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/PersistentStorageTelemetryUploader.cs | Background drain loop that leases blobs, uploads, deletes, and re-persists remainders. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/ITelemetryUploadTransport.cs | Transport abstraction for uploader (enables unit-testing without real network). |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/ITelemetryBlobStorage.cs | Storage/blob abstractions (wrap persistent storage implementation behind seams). |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/HttpTelemetryUploadTransport.cs | HttpClient + gzip upload + 206 response parsing/reslicing for retriable items. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/FileSystemTelemetryBlobStorage.cs | File-system-backed storage implementation via OTel persistent storage package. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/BreezeWriter.cs | Common NDJSON envelope writing + property formatting + timestamp formatting. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/BreezeSchema.cs | Breeze envelope constants + field length limits matching Azure exporter behavior. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/BreezePartialContent.cs | 206 partial-success helpers: parse response + slice retriable envelopes from NDJSON. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/BreezeEnvelope.cs | POCO envelope/schema models + System.Text.Json source-gen context for AOT/trim safety. |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/AzureMonitorTelemetrySerializer.cs | Activity → Breeze envelope NDJSON serialization (Azure exporter–compatible). |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/AzureMonitorLogSerializer.cs | LogRecord → Breeze envelope NDJSON serialization (Azure exporter–compatible). |
| src/Cli/Microsoft.DotNet.Cli.Telemetry/Implementation/AzureMonitorConnectionString.cs | Minimal connection string parsing for ikey + ingestion endpoint + track URI. |
| src/Cli/dotnet/Telemetry/TelemetryClient.cs | Switches local runs to persist-then-drain exporter; CI uses stock exporter + flush policy. |
| src/Cli/dotnet/dotnet.csproj | Adds project reference to new telemetry library and exposes internals to dotnet.Tests. |
| src/Cli/dotnet-aot/dotnet-aot.csproj | Adds project reference to new telemetry library for AOT CLI consumption. |
| sdk.slnx | Adds new telemetry project and its test project to the solution. |
| eng/Versions.props | Adds OpenTelemetry persistent storage package version property. |
| documentation/project-docs/telemetry.md | Documents new trace-export disable flag and persist-then-drain delivery model. |
| Directory.Packages.props | Adds central versions for OpenTelemetry and OpenTelemetry.PersistentStorage.FileSystem. |
| BreezeWriter.WriteEnvelope(stream, envelope, TelemetryJsonContext.Default.RequestEnvelope); } | ||
| else | ||
| { var envelope = new TelemetryEnvelope<RemoteDependencyData> | ||
| { |
| // exits. Locally we only need a brief flush because spans are persisted synchronously as they | ||
| // end and delivered by a later invocation. | ||
| private static readonly bool s_isCIEnvironment = new CIEnvironmentDetectorForTelemetry().IsCIEnvironment(); | ||
| private static readonly int s_flushTimeoutMs = s_isCIEnvironment ? Timeout.Infinite : 10; |
| { | ||
| using var listener = CreateListener(); | ||
| using var activity = Source.StartActivity("dotnet build", ActivityKind.Internal)!; | ||
| // Two tags with the same key: only the first should be written to avoid invalid JSON. |
Co-authored-by: nagilson <23152278+nagilson@users.noreply.github.com>
Resolved. There was a single conflict in |
nagilson
left a comment
There was a problem hiding this comment.
I looked broadly at the code but not into the specifics as I hope that we will be able to get the 2 small changes into the official library.
I'm in favor of pushing this forward quickly since it is .net11, since we already know the exiting logic does not really work, and also to unblock dotnetup. By the time we are talking about RC1 I'd want to revisit this decision to see if this code is still needed, and I think we should also try to get one more person to double check this plan.
I think what would be most helpful for me would be if you've been able to test the implementation by using various errors and pushing to the real endpoint to see if there's a big change in how much data gets dropped vs sent. I have a lot of (throwaway) scripts / setup to do that already from my investigation so happy to give you what I have. It takes about an hour for the data to actually get ingested and more like 3 hours anytime it's something new so its a slow process to test this stuff.
| the process exits. | ||
| 2. **Drain (in the background).** On startup, a background worker leases previously persisted payloads and POSTs | ||
| them to the Azure Monitor ingestion endpoint. Successfully delivered payloads are deleted; failed uploads are | ||
| retained and retried on a later invocation. |
There was a problem hiding this comment.
- The exception to this is in CI, where drain should happen at the end of the application but with a timeout and a hook directly onto the POST to exit early.
- Switch CI telemetry from ForceFlush(Infinite) to Shutdown(20s) so the full export pipeline (including inflight HTTP POSTs) completes before process exit. - Add DOTNET_CLI_TELEMETRY_SHUTDOWN_TIMEOUT_MS env var knob to override the default 20s CI shutdown timeout. - Remove fixed HttpClient.Timeout; cancellation now flows from the provider's shutdown budget through a CancellationToken to each HTTP call. - Give exporters ownership of the drain lifecycle: OnShutdown cancels the drain CTS and waits for the background task within the timeout budget. - Add per-blob linked CancellationTokenSource in the drain loop so the tighter of (caller budget, lease period) wins. - Add ItDeletesBlobWhenUploadSucceeds test. - Guard E2E test from drain deleting blobs by setting shutdown timeout to 1ms. - Update telemetry.md to document CI shutdown-with-timeout behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds RealEndpointTelemetryE2ETests: a gated suite that POSTs CLI-representative telemetry to a live Application Insights /v2.1/track endpoint and asserts the itemsReceived/itemsAccepted/errors breakdown the service returns synchronously. - Skips (Assert.Inconclusive) unless DOTNET_CLI_TELEMETRY_E2E_CONNECTION_STRING is set, so it never runs or hits the network during a normal CI pass. - Reuses the shipping AzureMonitorTelemetrySerializer to build valid payloads and validates the production HttpTelemetryUploadTransport path end-to-end. - Measures drop rate immediately from the POST response and acts as a canary for breaking changes in the App Insights ARM ingestion wire contract. - Stamps a marker + per-run id on every envelope so emitted rows can be found when verifying ingestion by hand. - Documents how to run the harness in the project README. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The unique e2e.run.id / e2e.marker tags land in Application Insights customDimensions on every emitted row, so a run's data is filterable across the traces, requests, and dependencies tables. To support asserting ingestion after execution: - Allow the run id to be pinned via DOTNET_CLI_TELEMETRY_E2E_RUN_ID so a harness script can choose the id, run the tests, wait for ingestion (~1h), then query for exactly that id. Defaults to a fresh GUID otherwise. - Emit a ready-to-run KQL query (scoped to the run id) to the test output. - Document the pin-run-id + query-back verification workflow in the README. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The dotnet CLI does not query the raw Application Insights tables; its telemetry flows through a processing pipeline into a downstream table searched by the Message/EventName field. Embed the per-run token as the message *event name* so it maps to MessageData.message and surfaces as Message/EventName downstream, instead of relying on a raw customDimensions KQL query. - Message event name is now cli-telemetry-real-endpoint-e2e/<runId>. - Add an always-on (non-networked) regression guard asserting the token lands in MessageData.message, protecting the findability contract in CI. - Update the test output + README verification steps to search by Message/EventName. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…k/TestBuild dotnet-aot.Tests imports AotSourceFiles.props (which compiles TelemetryClient.cs) and references Azure.Monitor, so AddAzureMonitorTraceExporter resolves. But it was missing the Microsoft.DotNet.Cli.Telemetry project reference that dotnet-aot.csproj already has, so the AddPersistentStorageExporter call (guarded by MICROSOFT_ENABLE_TELEMETRY_AZURE_MONITOR) failed to compile with CS1061, breaking every CI leg that builds dotnet-aot.Tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…net folder FileBlobProvider creates its storage directory in its constructor, and FileSystemTelemetryBlobStorage was constructing it eagerly when the exporter was registered. That registration runs in TelemetryClient's static constructor, which executes before the per-invocation opt-out early-return, so merely touching TelemetryClient created the ~/.dotnet telemetry folder even when telemetry was opted out. This broke ItDoesNotCreateAFirstUseSentinelFileNorAnAspNetCertificateSentinelFileUnderTheDotDotNetFolderWhenInternalReportInstallSuccessIsInvoked across all platforms. (The Azure Monitor path is unaffected because its exporter is only built at provider.Build() time, which is gated behind Enabled.) Defer building FileBlobProvider via a thread-safe Lazy<T> so the directory is created only when telemetry is actually persisted or drained. Add regression tests asserting that constructing the storage and registering the exporter touch no file system, and that the first persist creates the directory. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/ba-g unrelated test failure in SWA test |
…arness The downstream CLI telemetry pipeline only forwards events emitted in the real CLI shape (ActivityEvents named dotnet/cli/<event>); a synthetic event name is dropped and never reaches the destination table. Rework the real-endpoint E2E harness to emit a genuine dotnet/cli/toplevelparser/command event and stamp the run id into the SessionId common property (the field specific runs are looked up by), so ingested rows are actually findable. Update the README verification section to the SessionId query workflow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 46b9b720-1650-4169-9665-9c88ded7b668
… exporter Our CLI telemetry library hand-ports the Azure Monitor exporter's Breeze envelope mapping (all exporter types are internal and not source-build friendly), so the "byte-for-byte compatible" claim was previously only asserted against hand-authored JSON, never against the real exporter. Add AzureMonitorExporterDifferentialTests: for each span/event shape it drives the real Azure.Monitor.OpenTelemetry.Exporter through a capturing HTTP transport and asserts our AzureMonitorTelemetrySerializer produces the exact same Breeze envelopes (order-insensitive JSON DeepEquals). The exporter package is a test-only reference; the shipping library still avoids it. Two shipping-code alignments surfaced by the differential test: - BreezeWriter.FormatTime now emits the UTC "Z" round-trip form (matching the exporter) instead of "+00:00". - ExceptionData.properties is now always emitted (empty object when there are no properties), matching the exporter's TelemetryExceptionData serialization (MessageData still omits empty properties, as the exporter does). Both forms were already accepted by the ingestion endpoint; these changes make the emitted bytes identical to the exporter's. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 46b9b720-1650-4169-9665-9c88ded7b668
…s recover them (they use a 2 minute timer to recover failed blobs) We use the file blob provider from OTel OTels file blob provider will rename the blob with .lock to prevent it from being looked at to POST again OTel uses opentelemetry-dotnet-contrib/src/OpenTelemetry.PersistentStorage.FileSystem/FileBlobProvider.cs at … this const as 2 Minutes OTel subscribes to a timer at 2 minutes opentelemetry-dotnet-contrib/src/OpenTelemetry.PersistentStorage.FileSystem/FileBlobProvider.cs at 9… and timers only fire after the first interval Timer.Elapsed Event (System.Timers) | Microsoft Learn OTel does a try delete with the blob opentelemetry-dotnet-contrib/src/OpenTelemetry.PersistentStorage.FileSystem/FileBlobProvider.cs at 9… and renames it back out of .lock on this timer at opentelemetry-dotnet-contrib/src/OpenTelemetry.PersistentStorage.FileSystem/PersistentStorageHelper.… OnMaintenanceEvent -> RemoveExpiredBlobs -> RemoveExpiredLease -> File.Move(.blob@<expiry>.lock, .blob) We assume the lock opentelemetry-dotnet-contrib/src/OpenTelemetry.PersistentStorage.FileSystem/PersistentStorageHelper… here means that its been deleted and posted OTels check to rename checks if it still needs a rename so us modifying it won't break them I basically use a finally block to rename it rather than rely on the OTel timer to do so (cherry picked from commit f0f72db)
nagilson
left a comment
There was a problem hiding this comment.
Thank you for coordinating and for taking action to help fix this problem. I hope to replace all of this with the 2 minor adjustments to the Azure OTel Exporter so we do not own all of this code, but for now lets get this in!
We did some review of the code, and from our discussion you've looked at it point-by-point but I have not looked at it line by line due to the size and scope involved here. I did however look at the broad picture + tests + some potential snags and didn't find any other overarching concerns beside the fix patch applied above.
Fixes #55184.
Motivation
Azure.Monitor.OpenTelemetry.Exporter+ the OpenTelemetry SDK are designed for long-running services, not one-and-done CLI processes. In a typicaldotnetinvocation, telemetry delivery is unreliable because:POSTfails — but a CLI process usually exits before the POST even completes, so the offline-storage backup never engages.ForceFlushwaits on batch dequeue, not on actual delivery to the ingestion endpoint.Net effect: telemetry delivery is coupled to process lifetime, and short processes silently drop data.
This PR introduces a small, reusable library —
Microsoft.DotNet.Cli.Telemetry— that decouples capturing telemetry from transmitting it, so a CLI reliably records what happened before it exits:dotnetcommand uploads what the previous one persisted.The
dotnetCLI is wired to use this locally, while CI runs (which are one-shot, with no "next" invocation to drain) keep using the stockAddAzureMonitorTraceExporterwith a blocking flush. The library is also consumed by the Native-AOTdotnet-aotCLI, so it is trim/AOT-clean.This is intended to be the first slice of a pattern we expect several CLIs to adopt (e.g.
dotnetup, which emits logs rather than spans — hence both a trace and a log exporter here).What's reused out-of-the-box vs. reimplemented
A goal of this work was to lean on canonical components wherever they exist, and to clearly isolate the parts that are reimplementations of Azure Monitor internals — code we'd ideally consume from a shared, supported source rather than maintain ourselves.
✅ Out-of-the-box components (no reimplementation)
BaseExporter<Activity>/BaseExporter<LogRecord>,SimpleActivityExportProcessor,SimpleLogRecordExportProcessor, and theTracerProviderBuilder/OpenTelemetryLoggerOptionsregistration convention (Add…Exporter).OpenTelemetry.PersistentStorage.FileSystem(FileBlobProvider/PersistentBlob). We wrap it behind a thinITelemetryBlobStorageseam; the atomic lease-by-rename and blob lifecycle are the library's.Azure.Monitor.OpenTelemetry.Exporter(AddAzureMonitorTraceExporter), used unchanged for one-shot CI processes.♻️ Reimplementations of Azure Monitor code (ideally reused from a canonical source)
These live under
Implementation/and are faithful ports of logic that is currentlyinternaltoAzure.Monitor.OpenTelemetry.Exporter. They exist only because that logic isn't exposed as a reusable, source-build-friendly package. If/when a canonical, referenceable source is available, these should be deleted in favor of it:AzureMonitorTelemetrySerializer.csTraceHelperspan→envelope mappingRequestData/RemoteDependencyData, events →MessageData/ExceptionData.AzureMonitorLogSerializer.csLogsHelper.OtelToAzureMonitorLogsLogRecord→MessageData/ExceptionData,LogLevel→severityLevel, scopes/attributes/EventId→ properties.BreezeEnvelope.cs/BreezeSchema.cs/BreezeWriter.csSystem.Text.Jsonsource generation (AOT/trim-safe) instead of the hand-rolled writer.AzureMonitorConnectionString.cs/v2.1/trackURI.HttpTelemetryUploadTransport.csTransmissionStateManager/IngestionResponsehandlingTelemetryResourceContextFactory.csai.*context tags +ai.internal.sdkVersionAzure.Monitor.OpenTelemetry.Exporter(keeps the library source-build clean).Why not just reference the Azure Monitor exporter for these? Its serialization/transport is
internal, and the package isn't part of source-build, so the shared library can't take a type dependency on it without breakingdotnet-aotand source-build. The reimplementations are deliberately kept byte-compatible with what the exporter produces so existing Application Insights dashboards keep working — and are isolated inImplementation/precisely so they're easy to rip out later.Layout
Follows canonical OpenTelemetry .NET exporter conventions: public surface + exporters at the project root (
Microsoft.DotNet.Cli.Telemetry), internal plumbing underImplementation/(Microsoft.DotNet.Cli.Telemetry.Implementation). See the projectREADME.mdfor usage and configuration.Tests
Microsoft.DotNet.Cli.Telemetry.Tests— unit coverage for the connection-string parser, both serializers, the gzip/206 transport, partial-content re-slicing, and the uploader (36 tests).dotnet.Tests— an out-of-process E2E test that spawns the CLI and asserts telemetry is persisted to the storage directory.Library,
dotnet, anddotnet-aotall build clean (0 warnings, including AOT/trim analyzers).Notes for reviewers