Skip to content

Conversation

@JacksonWeber
Copy link
Contributor

@JacksonWeber JacksonWeber commented Oct 7, 2025

Which problem is this PR solving?

Updates the logs SDK to support the minimum severity filtering and trace-based log filtering via the FilteringLogRecordExporter.

Short description of the changes

This pull request introduces a configurable logger filtering mechanism to the OpenTelemetry SDK logs package, allowing log records to be filtered based on severity and trace sampling, and enabling per-logger configuration through a new pattern-based API. The core changes add a LoggerConfigurator interface, implement pattern-based configuration, and apply these configurations during log record emission. This improves flexibility and compliance with logging specifications.

Logger configuration and filtering enhancements

This pull request introduces a new feature to the OpenTelemetry JS SDK for logs: the FilteringLogRecordProcessor. This processor allows users to filter log records by minimum severity and/or trace sampling before they are exported. The PR includes the implementation, documentation, and comprehensive tests for this functionality.

Key changes:

Feature: Filtering Log Records

  • Added the FilteringLogRecordProcessor class, which can filter log records based on minimum severity or whether the associated trace is sampled, before passing them to another processor. The processor is configurable and can be combined with existing processors. [1] [2] [3]

Documentation

  • Updated the sdk-logs README with usage examples and explanations for minimum severity filtering, trace-based filtering, and combining both filters using the new FilteringLogRecordProcessor.

Testing

  • Added comprehensive tests for the FilteringLogRecordProcessor, covering scenarios for minimum severity filtering, trace-based filtering, default behaviors, and combined filtering logic. [1] [2]

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Unit tests for each type of filtering added

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@JacksonWeber JacksonWeber requested a review from a team as a code owner October 7, 2025 02:12
@codecov
Copy link

codecov bot commented Oct 7, 2025

Codecov Report

❌ Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.13%. Comparing base (47f1b4e) to head (ec14540).

Files with missing lines Patch % Lines
...sdk-logs/src/export/FilteringLogRecordProcessor.ts 84.61% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5991      +/-   ##
==========================================
- Coverage   95.16%   95.13%   -0.03%     
==========================================
  Files         316      317       +1     
  Lines        9207     9233      +26     
  Branches     2075     2087      +12     
==========================================
+ Hits         8762     8784      +22     
- Misses        445      449       +4     
Files with missing lines Coverage Δ
...sdk-logs/src/export/FilteringLogRecordProcessor.ts 84.61% <84.61%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Specification: https://github.com/open-telemetry/opentelemetry-specification/blame/f31acdf68b1c886269ab39011a338a8d72dbebea/specification/logs/sdk.md#L102-L133

@JacksonWeber - please also mark all features that are marked as in-development in the spec as @experimental in the TSdoc. This way we can avoid blocking https://github.com/open-telemetry/opentelemetry-js/milestone/19

@JacksonWeber
Copy link
Contributor Author

Specification: https://github.com/open-telemetry/opentelemetry-specification/blame/f31acdf68b1c886269ab39011a338a8d72dbebea/specification/logs/sdk.md#L102-L133

@JacksonWeber - please also mark all features that are marked as in-development in the spec as @experimental in the TSdoc. This way we can avoid blocking https://github.com/open-telemetry/opentelemetry-js/milestone/19

Experimental features have been marked as such. Thanks for the feedback!

@pichlermarc pichlermarc added the spec-feature This is a request to implement a new feature which is already specified by the OTel specification label Oct 8, 2025
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Hmm, overall I'm a bit confused by this feature - the spec for it seems still a bit vague and seems to duplicate functionality from existing features. 🤔 That's not this PR's issue, I feel like it's a symptom of a spec that's not quite where we'd need it to be yet.

I suppose the value proposition for having a LoggerConfigurator is to save on allocations in the pipeline - which is important for a hot path like this. (Q: @JacksonWeber is this the motivation for working on this feature?) If that's not the goal, then we could accomplish the same thing today in the LogRecordProcessor.

Though it seems to me that the optimizing LogRecordImpl to avoid expensive operations until it's properties are first accessed, and then applying filtering in the LogRecordProcessor may be a better way to go about it without adding additional concept to the public API.

The Go SIG seems to have a similar stance: open-telemetry/opentelemetry-specification#4644

// Get logger configuration
const loggerConfig = this._sharedState.getLoggerConfig(
this.instrumentationScope
);
Copy link
Member

Choose a reason for hiding this comment

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

Given that emit is a hot-path, I think we should optimize here wherever possible.

One thing I noticed is that the way it's implemented right now is:

Do we need the map lookup or should we just cache it in the logger? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The primary motivation for this feature work is just to get an implementation of trace-based log sampling and minimum severity-based log sampling implemented in a way that's easy for users to enable as part of the configuration of their LoggerProvider.

Thanks for the link to the issue in the Go repo, I'm happy to refactor this PR to avoid the creation/usage of the LoggerConfigurator and just implement these two filtering strategies in the LogRecordProcessor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pichlermarc the PR has been updated to just add a new logRecordProcessor that handles both types of filtering. Please review when you have a chance.

@JacksonWeber JacksonWeber changed the title feat(sdk-logs): Add loggerConfigurator feat(sdk-logs): Add FilteringLogRecordProcessor Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:sdk-logs spec-feature This is a request to implement a new feature which is already specified by the OTel specification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants