-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[AzureMonitorExporter] Trace based logs sampler #53441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AzureMonitorExporter] Trace based logs sampler #53441
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/LogFilteringProcessor.cs
Show resolved
Hide resolved
sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorExporterOptions.cs
Outdated
Show resolved
Hide resolved
sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/AzureMonitorOptions.cs
Outdated
Show resolved
Hide resolved
sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/LogFilteringProcessor.cs
Show resolved
Hide resolved
...tor/Azure.Monitor.OpenTelemetry.AspNetCore/src/Azure.Monitor.OpenTelemetry.AspNetCore.csproj
Show resolved
Hide resolved
|
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 |
...lemetry.AspNetCore/tests/Azure.Monitor.OpenTelemetry.AspNetCore.Tests/InitializationTests.cs
Show resolved
Hide resolved
...tor/Azure.Monitor.OpenTelemetry.AspNetCore/src/Azure.Monitor.OpenTelemetry.AspNetCore.csproj
Show resolved
Hide resolved
|
@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.