Trace Based Sampling Log Filter Implementation#43811
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements log filtering capabilities for the Azure Monitor OpenTelemetry exporter, adding support for minimum severity level filtering and trace-based sampling. The implementation allows logs to be filtered out based on their severity level or association with unsampled traces.
Key changes:
- Added two new filtering functions:
_is_less_than_minimum_severity_leveland_should_drop_logs_for_unsampled_traces - Modified
_log_to_envelopeto returnNonefor filtered logs instead of always returning aTelemetryItem - Added comprehensive test coverage for both filtering mechanisms
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| _exporter.py | Modified log export logic to filter envelopes and handle None returns from conversion function |
| _utils.py | Added two utility functions for severity level and trace-based filtering logic |
| _constants.py | Added environment variable constants for the new filtering features |
| test_logs.py | Added comprehensive test cases for severity filtering, trace-based filtering, and combined filtering |
Comments suppressed due to low confidence (1)
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/logs/_exporter.py:201
- The Event telemetry path (lines 189-201) lacks filtering logic that is present in Exception and Message telemetry paths. Event telemetry should also be filtered based on trace-based sampling and minimum severity level for consistency. Consider adding the same filtering checks after line 201.
elif _log_data_is_event(log_data): # Event telemetry
_set_statsbeat_custom_events_feature()
envelope.name = "Microsoft.ApplicationInsights.Event"
event_name = ""
if log_record.attributes.get(_MICROSOFT_CUSTOM_EVENT_NAME): # type: ignore
event_name = str(log_record.attributes.get(_MICROSOFT_CUSTOM_EVENT_NAME)) # type: ignore
else:
event_name = _map_body_to_message(log_record.body)
data = TelemetryEventData( # type: ignore
name=event_name,
properties=properties,
)
envelope.data = MonitorBase(base_data=data, base_type="EventData")
|
Discussed with Hector, and let's align on using the configuration option for trace-based log sampling. We can also remove the addition of minimum-severity based log sampling for now and await the upstream changes. The relevant Node.js PR making this change is: Azure/azure-sdk-for-js#28993 |
49f8272 to
ec5841d
Compare
f544a41 to
6a39281
Compare
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
a6ba829 to
54c8e31
Compare
f1cc00b to
15e1584
Compare
…/opentelemetry/exporter/export/logs/_processor.py Update docstring Co-authored-by: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com>
15e1584 to
7e17176
Compare
* Trace Based Sampling and Minimum Severity Log Filters Implementation * Added CHANGELOG * Fix lint and added tests * Modified severity logic * Added type safety * Fix cspell errors * Remove extra line * Remove extraneous new line * Fix filter labels * Fix description * Distro changes for Trace based sampling * Remove previous logic and add code for processor and tests * Remove extra line * Add back space * Remove unused import * Fix cspell errors * Fix lint errors * Fix argument * Update sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/logs/_processor.py Update docstring Co-authored-by: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com> * Fix mypy and naming consistency * Fix lint --------- Co-authored-by: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com>
Description
Implement open-telemetry/opentelemetry-specification#4612 to filter logs based on
trace_based_samplingparameter.All SDK Contribution checklist:
General Guidelines and Best Practices