Restore OTEL_SDK_DISABLED in AzureMonitorExporterOptions callbacks#3157
Restore OTEL_SDK_DISABLED in AzureMonitorExporterOptions callbacks#3157
Conversation
PR #3156 accidentally removed the OTEL_SDK_DISABLED assignment from the AzureMonitorExporterOptions configure callbacks in both AspNetCore and WorkerService packages. This assignment is needed so the OTel SDK's TracerProvider sees the disabled flag when it checks IConfiguration. The TelemetryConfiguration factory (added in #3156) covers MeterProvider, but this callback is still required for TracerProvider which resolves through a different path.
There was a problem hiding this comment.
Pull request overview
This PR attempts to restore setting OTEL_SDK_DISABLED=true inside the AzureMonitorExporterOptions configure callbacks for the AspNetCore and WorkerService packages so that TelemetryConfiguration.DisableTelemetry=true is respected by the OpenTelemetry TracerProvider path.
Changes:
- Re-add
config["OTEL_SDK_DISABLED"] = "true"to theAzureMonitorExporterOptionsoptions callback whentelemetryConfig.DisableTelemetryis true (WorkerService). - Re-add
config["OTEL_SDK_DISABLED"] = "true"to theAzureMonitorExporterOptionsoptions callback whentelemetryConfig.DisableTelemetryis true (AspNetCore).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| NETCORE/src/Microsoft.ApplicationInsights.WorkerService/ApplicationInsightsExtensions.cs | Reintroduces OTEL_SDK_DISABLED assignment in the exporter options configure callback. |
| NETCORE/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs | Reintroduces OTEL_SDK_DISABLED assignment in the exporter options configure callback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Set OTEL_SDK_DISABLED in configuration if DisableTelemetry is true | ||
| if (telemetryConfig.DisableTelemetry) | ||
| { | ||
| config["OTEL_SDK_DISABLED"] = "true"; | ||
| } | ||
|
|
There was a problem hiding this comment.
This OTEL_SDK_DISABLED assignment is redundant: AddTelemetryConfigAndClient already sets IConfiguration["OTEL_SDK_DISABLED"] = "true" during TelemetryConfiguration resolution when DisableTelemetry is true (NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs:125-134). Since telemetryConfig is resolved before this callback executes, this block doesn’t change behavior and adds another place to maintain this logic; consider removing it and keeping the setting centralized in the shared TelemetryConfiguration factory.
| // Set OTEL_SDK_DISABLED in configuration if DisableTelemetry is true | |
| if (telemetryConfig.DisableTelemetry) | |
| { | |
| config["OTEL_SDK_DISABLED"] = "true"; | |
| } |
There was a problem hiding this comment.
Intentionally keeping this as defense-in-depth. The TelemetryConfiguration factory covers MeterProvider, but TracerProvider may resolve through a different path. Setting the same value twice in IConfiguration is harmless and this was the original behavior on main before #3156.
...ORE/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs
Show resolved
Hide resolved
Register a no-op ILoggerProvider at position 0 in the service collection that depends on TelemetryConfiguration. ILoggerFactory resolves all ILoggerProvider instances in registration order during host construction. By inserting this first, TelemetryConfiguration is resolved (which sets OTEL_SDK_DISABLED in IConfiguration) before OpenTelemetry's LoggerProvider factory checks the flag. Without this, LoggerProvider was constructed before OTEL_SDK_DISABLED was set, resulting in logs still being exported.
Problem
PR #3156 accidentally removed the
OTEL_SDK_DISABLEDassignment from theAzureMonitorExporterOptionsconfigure callbacks in both AspNetCore and WorkerService packages. Without this,TracerProviderdoes not see the disabled flag whenDisableTelemetry = true.Fix
Restore the
OTEL_SDK_DISABLEDassignment in the exporter options callbacks. TheTelemetryConfigurationfactory (added in #3156) coversMeterProvider, but this callback is still required forTracerProviderwhich resolves through a different path.Changes
NETCORE/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.csOTEL_SDK_DISABLEDin exporter options callbackNETCORE/src/Microsoft.ApplicationInsights.WorkerService/ApplicationInsightsExtensions.csOTEL_SDK_DISABLEDin exporter options callback