Repository navigation
Share the Azure Monitor telemetry storage directory between managed and AOT CLI - #56409
Conversation
… with StorageSubDirectory Azure/azure-sdk-for-net#63337 (fixing Azure/azure-sdk-for-net#62997) added the Azure.Monitor.OpenTelemetry.Exporter.StorageSubDirectory AppContext value but no release contains it yet. This commit vendors a package built from Azure/azure-sdk-for-net@8c4894e (version 1.10.0-alpha.sdklocal.20260924, same dependency set as 1.9.0) behind a local feed so the SDK change and its tests can be validated. Revert this commit and bump AzureMonitorOpenTelemetryExporterPackageVersion to the first official release containing the feature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… CLI The Azure Monitor exporter names its persistent storage sub directory from the process name and AppContext.BaseDirectory, which differ between dotnet.dll and dotnet-aot. Telemetry was therefore split into two directories and the managed side, which runs less often, rarely drained its backlog. Set the Azure.Monitor.OpenTelemetry.Exporter.StorageSubDirectory AppContext value (Azure/azure-sdk-for-net#62997) to 'dotnet-cli' in the shared TelemetryClient so every entry point uses one directory. The instrumentation key and user name still isolate the directory. Add an E2E test, in both directions, that one entry point persists telemetry and the other leases it from the same directory to upload it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 2 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
https://github.com/Azure/azure-sdk-for-net/releases?q=telemetry&expanded=true watching this for a new upload |
|
Seems good minus the revert commit. Whenever they release a fix, we can publish the PR. |
|
There's a new 7 day limit on updates - https://github.com/Azure/azure-sdk-for-net/releases?q=exporter&expanded=true it's now been a week after coordination. |
Remove the temporary locally built package and feed now that the StorageSubDirectory support is available in the official release. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment was marked as spam.
This comment was marked as spam.
There was a problem hiding this comment.
🟢 Approval recommended
The official exporter setting is correctly initialized for both entry points and covered by focused bidirectional tests.
0 open findings
What changed in this PR
Shares Azure Monitor telemetry persistence between managed and Native AOT CLI entry points.
Changes:
- Upgrades the Azure Monitor exporter to 1.10.0.
- Configures a shared
dotnet-clistorage subdirectory. - Adds bidirectional E2E coverage and documentation.
| File | Description |
|---|---|
eng/Versions.props |
Updates the exporter dependency. |
src/Cli/dotnet/Telemetry/TelemetryClient.cs |
Configures shared telemetry storage. |
test/dotnet.Tests/TelemetryTests/PersistentStorageTelemetryE2ETests.cs |
Verifies cross-entry-point draining. |
documentation/project-docs/telemetry.md |
Documents shared storage behavior. |
🧠 Review effort: Balanced
I think this is less of a problem for Aspire because it is an actual CLI app, not a NAOT Library trying to behave like an app. |
|
/ba-g build analysis seems stuck |
|
/tactics |
Follow-up to #56274.
Problem
The Azure Monitor exporter names its persistent storage sub directory by hashing
instrumentationKey;userName;processName;AppContext.BaseDirectory.dotnet.dllanddotnet-aothave different base directories, so their telemetry lands in two separate directories.The managed side runs less often, rarely drains its own backlog (it is too slow to actually have enough time to push data without adding significant exit lag), and that telemetry is lost.
Change
Azure added the
Azure.Monitor.OpenTelemetry.Exporter.StorageSubDirectoryAppContext value for us in Azure/azure-sdk-for-net#63337 (fixes Azure/azure-sdk-for-net#62997), released in Azure.Monitor.OpenTelemetry.Exporter 1.10.0. This PR updates the SDK to consume that official package.TelemetryClientsets the value todotnet-cli.TelemetryClient.csis compiled into bothdotnetanddotnet-aot, so one line covers both entry points. The instrumentation key and user name still contribute to the hash, so different users and resources remain isolated.documentation/project-docs/telemetry.mddescribes the shared directory.ItDrainsTelemetryPersistedByTheOtherEntryPoint, run in both directions: one entry point persists telemetry, then the other leases the same blob from the same directory to upload it. With theSetDataline removed, both cases fail because the directories differ.Validation
PersistentStorageTelemetryE2ETestsandTelemetryClientTests(33 tests) pass.Microsoft.DotNet.Cli.Telemetry.Testspasses.dotnet exec dotnet.dll --helpand AOTdotnet --helpwrite to the same sub directory.Tactics
Summary
The Azure Monitor exporter derives its persistent storage subdirectory from a hash that includes
AppContext.BaseDirectory, sodotnet.dll(managed) anddotnet-aotwrite telemetry to different directories. The managed side runs less often and rarely drains its backlog, so its persisted telemetry is lost. This PR bumpsAzure.Monitor.OpenTelemetry.Exporterto 1.10.0 (eng/Versions.props), which supports theAzure.Monitor.OpenTelemetry.Exporter.StorageSubDirectoryAppContext switch, and sets it todotnet-cliinTelemetryClient.cs(compiled into both entry points). The change is small and confined to telemetry, and fits a servicing release because it recovers lost telemetry without affecting CLI functionality.Customer Impact
Customers see no functional symptom. The impact is on SDK telemetry: data persisted by the managed CLI is stranded and never uploaded. This affects every install that uses both entry points, in the release this branch (
release/11.0.1xx) ships. There is no user-facing workaround, and none is needed. Instrumentation key and user name still contribute to the hash, so users and resources stay isolated.Regression?
Unknown — not enough information to determine origin. The PR is a follow-up to #56274 and fixes a gap in how the managed and AOT entry points share persistent storage.
Testing
ItDrainsTelemetryPersistedByTheOtherEntryPoint, run in both directions (managed→AOT and AOT→managed). Per the PR description, both cases fail when theSetDataline is removed.PersistentStorageTelemetryE2ETestsandTelemetryClientTests(33 tests) pass, as doesMicrosoft.DotNet.Cli.Telemetry.Tests, per the PR description.dotnet exec dotnet.dll --helpand AOTdotnet --helpwrite to the same subdirectory./ba-goverride was issued because Build Analysis appeared stuck.Risk
Low. The change is about 70 lines across 4 files: one version bump, one
AppContext.SetDatacall, a doc update and tests. The main risk is the dependency bump from the exporter, mitigated by the targeted E2E and unit tests. Another is that a newly named shared directory is read by both entry points. Both are covered by the bidirectional tests.