Skip to content

Conversation

@rads-1996
Copy link
Member

@rads-1996 rads-1996 commented Nov 5, 2025

Description

Implement open-telemetry/opentelemetry-specification#4612 to filter logs based on trace_based_sampling parameter.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Copilot AI review requested due to automatic review settings November 5, 2025 15:11
@github-actions github-actions bot added the Monitor - Exporter Monitor OpenTelemetry Exporter label Nov 5, 2025
Copy link
Contributor

Copilot AI left a 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 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_level and _should_drop_logs_for_unsampled_traces
  • Modified _log_to_envelope to return None for filtered logs instead of always returning a TelemetryItem
  • 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")

@JacksonWeber
Copy link
Member

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

@rads-1996 rads-1996 force-pushed the trace-based-sampling branch from 49f8272 to ec5841d Compare November 12, 2025 17:05
@rads-1996 rads-1996 changed the title Trace Based Sampling and Minimum Severity Log Filters Implementation Trace Based Sampling Log Filter Implementation Nov 12, 2025
@rads-1996 rads-1996 force-pushed the trace-based-sampling branch from f544a41 to 6a39281 Compare November 12, 2025 17:49
@github-actions
Copy link

github-actions bot commented Nov 12, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

azure-monitor-opentelemetry

@rads-1996 rads-1996 force-pushed the trace-based-sampling branch from a6ba829 to 54c8e31 Compare November 12, 2025 20:07
Copy link
Member

@hectorhdzg hectorhdzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rads-1996 rads-1996 force-pushed the trace-based-sampling branch from 2ff3a3a to d521a99 Compare November 13, 2025 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Monitor - Exporter Monitor OpenTelemetry Exporter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants