logs: add Enabled support to Logger API, SDK, and LogRecordProcessor - #5380
logs: add Enabled support to Logger API, SDK, and LogRecordProcessor#5380ocelotl wants to merge 11 commits into
Conversation
|
Symbolic approval. Overall good, just a non blocking comment regarding the default implementation for Logger.enabled() |
xrmx
left a comment
There was a problem hiding this comment.
Small nit on the changelog but LGTM
The PRs Hector linked were for logs too. It's still breaking change even to logging (even if it's in an unstable component) so it's nice to call attention in the changelog |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds enabled() support across the Logs API + SDK to allow instrumentation to cheaply decide whether to construct/emit log records.
Changes:
- Introduces
enabled()on APILoggerand SDKLogger/LogRecordProcessor, including multi-processor aggregation behavior. - Adds SDK and API tests covering enabled/no-op/proxy delegation scenarios.
- Adds changelog entries documenting the API/SDK change.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| opentelemetry-sdk/tests/logs/test_logs.py | Adds SDK tests for Logger.enabled() behavior with/without processors and config gating. |
| opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/init.py | Implements enabled() on LogRecordProcessor, multi-processors, and SDK Logger. |
| opentelemetry-api/tests/logs/test_proxy.py | Adds API tests for ProxyLogger.enabled() delegation and NoOpLogger.enabled(). |
| opentelemetry-api/src/opentelemetry/_logs/_internal/init.py | Adds enabled() to API Logger plus implementations in NoOpLogger and ProxyLogger. |
| .changelog/5380.changed | Notes a breaking requirement for Logger subclasses regarding enabled(). |
| .changelog/5380.added | Announces enabled() support addition across API/SDK and processors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Implements the Enabled chain from the OpenTelemetry Logs specification: - API Logger.enabled(context, severity_number, event_name) -> bool: default True; NoOpLogger returns False; ProxyLogger delegates. - SDK LogRecordProcessor.enabled(...) -> bool: non-abstract default True so existing processors opt-in by overriding. - SDK SynchronousMultiLogRecordProcessor/ConcurrentMultiLogRecordProcessor: enabled() returns False when no processors are registered, False when all processors return False, otherwise True. - SDK Logger.enabled(): checks LoggerConfig.is_enabled first, then delegates to the multi-processor with the instrumentation scope. Closes open-telemetry#5360
…g fragment Add pylint disable=no-self-use annotations to the enabled() default implementations in the Logger API and LogRecordProcessor SDK base classes, matching the existing pattern used for force_flush(). Also add the missing .changelog/5380.added fragment required by the changelog CI check.
Since the logs API is not yet stable, adding enabled() as an abstract method does not break a released version. All concrete Logger subclasses (NoOpLogger, ProxyLogger, and the SDK Logger) already provide an implementation.
…__.py Co-authored-by: Aaron Abbott <aaronabbott@google.com>
…__.py Co-authored-by: Aaron Abbott <aaronabbott@google.com>
Set enabled.return_value = True on the mock processor and assert it was called, so the test verifies the multi-processor wiring rather than passing on a bare Mock's truthiness.
pylint reports unused-argument on the parameter lines, so the disable on the return statement did not suppress it. Fold it into the disable on the def line where it is effective.
Reflow assertions to the project's ruff-format style so the precommit check passes.
Even though logs are not yet stable, adding an abstract enabled() breaks external Logger subclasses, so call it out in the changelog.
|
Good point, agreed. Prefixed the |
Pull request dashboard statusWaiting on maintainers · refreshed 2026-08-17 17:31 UTC Merge when ready. Status above doesn't look right?
|
Summary
Implements the
Enabledchain from the OpenTelemetry Logs specification, closes #5360.Logger.enabled(context, severity_number, event_name) -> bool: non-abstract default returningTrue;NoOpLoggerreturnsFalse;ProxyLoggerdelegates to the real logger.LogRecordProcessor.enabled(context, instrumentation_scope, severity_number, event_name) -> bool: non-abstract default returningTrue— processors opt into filtering by overriding this method.SynchronousMultiLogRecordProcessor/ConcurrentMultiLogRecordProcessor:enabled()returnsFalsewhen no processors are registered,Falsewhen all processors returnFalse,Trueotherwise.Logger.enabled(): checksLoggerConfig.is_enabledfirst, then delegates to the multi-processor with the logger's instrumentation scope.Test plan
False, processor present →True, disabled logger config →False, args forwarded to processor, all processors disabled →False, one processor enabled →TrueNoOpLogger.enabled()→False,ProxyLogger.enabled()delegates to real logger,ProxyLogger.enabled()falls back to no-op