[Monitor Exporter] Make the connection string optional when multi-endpoint routing is enabled - #63004
Merged
Rajkumar Rangaraj (rajkumar-rangaraj) merged 5 commits intoSep 15, 2026
Conversation
Routing takes every destination from the telemetry, so a process that only routes has no component of its own. It still had to nominate one, which received nothing yet had its instrumentation key stamped on SDK statistics as though it owned the traffic. InitializeConnectionVars now takes the gate and returns routing-only connection vars instead of throwing, but only when routing is on and nothing is configured anywhere. The existing two-argument overload delegates with the gate off, so every current caller keeps the same behaviour and the same message. SDK statistics are skipped in this configuration: they identify a component by key and choose their region from its ingestion endpoint, and a routed destination supplies neither - it is chosen per item, long after the transmitter is built. Event 77 reports the mode rather than leaving it to be inferred. The storage directory omits the instrumentation key segment when there is none rather than substituting a placeholder, so every existing directory name is byte-identical and no persisted backlog is orphaned on upgrade. User, process name and application directory already identify an application on a machine.
Fixes from two review rounds on the optional-connection-string change. Rename IsRoutingOnly to IsUnconfigured: the name described why the mode exists rather than what it is, and the placeholder values are indistinguishable from a real destination by inspection. Key the storage directory seed on an explicit flag from the caller rather than on the instrumentation key being empty. A configured connection string can legitimately parse to an empty key, and inferring from the value would have moved such a process's directory and stranded its backlog. Build neither the FileBlobProvider nor the TransmitFromStorageHandler when unconfigured. Nothing writes to that storage, because every send is routed, and nothing may drain it, because there is no component to drain it to. Refuse the unrouted send path in TrackAsync instead of relying on its callers being gated elsewhere, so a new caller cannot silently deliver telemetry to the placeholder endpoint under an empty key. Skip Customer SDK Stats registration without a connection string; these identify the customer's own component. The guard takes an IPlatform so it reads the same source the transmitter reads. Raise event 77 to Warning and add event 78 for the refused send.
|
Azure Pipelines: Successfully started running 1 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Copilot started reviewing on behalf of
Rajkumar Rangaraj (rajkumar-rangaraj)
September 14, 2026 23:34
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The primary successful routed-send path without a connection string lacks integration coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Enables connection-string-free operation for multi-endpoint routing while preventing unrouted telemetry and SDK statistics from using placeholder destinations.
Changes:
- Adds unconfigured connection variables and guarded transmission behavior.
- Preserves routed storage while disabling component-specific storage and statistics.
- Adds diagnostics and focused tests.
File summaries
| File | Description |
|---|---|
CHANGELOG.md |
Documents optional connection strings for routing. |
AzureMonitorTransmitter.cs |
Implements unconfigured routing mode and send guards. |
ConnectionVars.cs |
Represents an unconfigured connection. |
CustomerSdkStatsRegistration.cs |
Skips customer statistics without a connection string. |
AzureMonitorExporterEventSource.cs |
Adds startup and dropped-telemetry diagnostics. |
StorageHelper.cs |
Supports connection-string-free storage hashing. |
OptionalConnectionStringTests.cs |
Tests configuration, storage, diagnostics, and guards. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Adds a 'nohost' argument to the multi-endpoint demo for all three signals, so the configuration this change enables can actually be exercised. Without it the demo refused to start unless MULTIENDPOINT_HOST_CONNECTION_STRING was set, which is the requirement being removed. The exporter's ConnectionString is left unset rather than assigned null or empty, because an empty string is a parse error rather than an absent value.
Copilot's PR review, and both earlier review rounds, flagged the same gap: every new test proved construction, storage wiring or refusal, while nothing asserted that routed telemetry is still delivered when the process has no connection string. A regression that stopped routing-only sends entirely would have gone undetected. Exports three destinations through the real exporter and transmitter with no connection string, and asserts each request's URI, that the serialized instrumentation key is the routed one, that nothing was addressed to the placeholder ingestion endpoint, and that no envelope carries an empty key. The positive assertions use the same serialized shape as the negative one so the latter cannot quietly stop matching. Also links the CHANGELOG entry to the pull request.
Rajkumar Rangaraj (rajkumar-rangaraj)
requested a balanced review from Copilot
September 14, 2026 23:50
Copilot started reviewing on behalf of
Rajkumar Rangaraj (rajkumar-rangaraj)
September 14, 2026 23:51
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Removing an existing host connection string can strand routed retry data, and the demo and integration test do not reliably guarantee an unconfigured environment.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 3
- Review effort level: Balanced
Both review models and Copilot independently found the same hole: the new integration tests built their transmitter with DefaultPlatform.Instance, which snapshots the real environment. On any machine or CI agent where APPLICATIONINSIGHTS_CONNECTION_STRING is set, the transmitter would be configured and every assertion would still hold, so the one test added to cover the unconfigured success path was the test most easily neutered by an ambient variable. It now uses MockPlatform and asserts the transmitter has no instrumentation key before returning. The demo had the same hole from the other direction: 'nohost' only skipped its own variable, so the exporter could still pick up the standard one and report "Host: NONE" while configured. It now refuses to run in that case rather than producing a verification run that proves nothing. Also notes at the seed site that dropping a connection string moves the storage directory, which strands what the previous configuration persisted.
xiang17
approved these changes
Sep 15, 2026
Rajkumar Rangaraj (rajkumar-rangaraj)
merged commit Sep 15, 2026
69cc14b
into
Azure:main
25 checks passed
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.
Why
Multi-endpoint routing takes every destination from the telemetry itself: each item carries
microsoft.instrumentation_keyandmicrosoft.ingestion_endpoint, and is sent to the endpoint it names. A process that only routes therefore has no Application Insights component of its own.It still had to supply a connection string. That forced the operator to nominate a component that receives nothing — and worse, the nominated instrumentation key was stamped on SDK statistics as though it owned all the routed traffic, misattributing every customer's volume to whichever component happened to be named.
What changes
A connection string is now optional, but only when the
Azure.Monitor.OpenTelemetry.EnableMultiEndpointRoutingAppContext switch is on. In that configurationConnectionVars.CreateUnconfigured()supplies an empty key and placeholder endpoints that seed the REST client, and:TrackAsyncrefuses and logs event 78FileBlobProvider/ drain handlerWarning, explaining the process can send nothing of its ownTelemetry without routing attributes was already dropped under this switch, so nothing that previously reached a destination stops doing so.
What does not change
Nothing, for anyone who has a connection string or who has routing switched off. A missing connection string is still an
InvalidOperationException; a malformed one still throws; routing is not a fallback for a typo. No public API changed —api/*.csis untouched.The one change that could have hurt existing users was the offline storage directory name: it is a hash seeded with the instrumentation key, so moving it would strand any persisted backlog. The seed now branches on an explicit
omitInstrumentationKeyflag passed by the caller, never on the key being empty — because a configured connection string can legitimately parse to an empty key, and such a process already has a directory. The tests pin all three directory names to literal SHA-256 values rather than recomputing them, so a change to the hash itself cannot satisfy them.Testing
1,181 tests pass on net8.0, net9.0, net10.0 and net462. The new
OptionalConnectionStringTestscovers the unconfigured path, the unchanged routing-off path, the configured-connection-string path in both modes, the storage seed, and the refusal of the unrouted send.Review notes
This was reviewed over three rounds by two independent models, which converged on no P0 or P1 findings. Their P2 findings are all addressed in the second commit: the send guard is now structural in
TrackAsyncrather than relying on callers being gated in two other files;HasConnectionStringtakes anIPlatformso it reads exactly what the transmitter reads; and three tests that would have passed with their production guard deleted were strengthened.Two P3 observations were left deliberately. There is no end-to-end test asserting Customer SDK Stats still register for a normal user, because running the real registration leaves an undisposable
MeterProviderand a cached transmitter behind for the rest of the test run — the guard is asserted directly instead. And theTransmitterFactorycache keys an absent connection string to"", which is pre-existing and introduces no new collision, sinceDefaultPlatformsnapshots the environment at construction.