[AzureMonitorExporter] Trace based logs sampler#53441
[AzureMonitorExporter] Trace based logs sampler#53441rajkumar-rangaraj merged 4 commits intoAzure:mainfrom
Conversation
API Change CheckAPIView identified API level changes in this PR and created the following API reviews Azure.Monitor.OpenTelemetry.AspNetCore |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a trace-based logs sampler feature for Azure Monitor OpenTelemetry exporters. This feature filters out logs that are associated with unsampled traces while allowing logs without trace context to pass through, helping reduce log volume while maintaining correlation with sampled traces.
- Adds
LogFilteringProcessorclass that extendsBatchLogRecordExportProcessorto filter logs based on trace sampling decisions - Introduces
EnableTraceBasedLogsSamplerconfiguration option (default:true) in bothAzureMonitorExporterOptionsandAzureMonitorOptions - Updates exporter registration logic to conditionally use
LogFilteringProcessorbased on the configuration flag
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| LogFilteringProcessor.cs | New processor that filters logs associated with unsampled traces by checking SpanId and TraceFlags |
| LogFilteringProcessorTests.cs | Comprehensive test suite validating filtering behavior for sampled, unsampled, and non-trace logs |
| AzureMonitorExporterOptions.cs | Adds EnableTraceBasedLogsSampler property to control the filtering feature |
| AzureMonitorOptions.cs | Adds EnableTraceBasedLogsSampler property and propagates it to exporter options |
| ExporterRegistrationHostedService.cs | Updates processor initialization to use LogFilteringProcessor when the feature is enabled |
| AzureMonitorExporterExtensions.cs | Updates extension methods to conditionally create LogFilteringProcessor based on configuration |
| AzureMonitorOptionsTests.cs | New test file verifying EnableTraceBasedLogsSampler property behavior and propagation |
| DefaultAzureMonitorOptionsTests.cs | Updates existing tests to verify the new property's default value |
| Azure.Monitor.OpenTelemetry.AspNetCore.csproj | Switches from PackageReference to ProjectReference for local development |
|
other than the comment I made, I'm good with this PR. Check the CI though, seems to be failing. Tag me once the code documentation is improved and I can mark this as approved |
|
@rajkumar-rangaraj @xiang17 when we bumped from I found out that this helps to bring the logs back but not sure this is intended behaviour: .UseAzureMonitor(options =>
{
// Disable trace-based log sampling to ensure all logs (including Information/Debug) are sent
// This was changed in Azure.Monitor.OpenTelemetry.Exporter 1.5.0 which filters logs by default
options.EnableTraceBasedLogsSampler = false;
})We are not using sampling or any features really. Just the most basic appinsights integration and this was a bad breaking change for us somehow. But maybe we do something else wrong:
Do you know what went wrong here? |
No description provided.