-
Notifications
You must be signed in to change notification settings - Fork 933
Stabilize LogRecordProcessor.Enabled #4717
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
base: main
Are you sure you want to change the base?
Conversation
|
@open-telemetry/php-maintainers @open-telemetry/rust-maintainers @open-telemetry/go-maintainers, PTAL as OTel PHP, Rust, and Go are implementing this part of the specification. |
|
@pellared can you please seek feedback from languages that already implemented it? Just to confirm they are happy with the feature. |
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.
Thanks.
This is what I have done here #4717 (comment) 😉 Awaiting approval from a PHP maintainer. I also think we can wait until next spec SIG meeting before merging. |
This demonstrates my reading comprehension. |
| - all registered `LogRecordProcessors` implement [`Enabled`](#enabled-1), | ||
| and a call to `Enabled` on each of them returns `false`. |
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.
This feature only really makes sense (to me) if you are chaining log processors, and (unfortunately) chaining log processors isn't a first-class concept in the Logs SDK (yet), and therefore not well supported via declarative configuration.
I also have a (smaller) concern about the lack of clarity on the overlap between this feature and Logger Config, which is related to why the Go SIG does not want to implement Logger Config.
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.
This feature only really makes sense (to me) if you are chaining log processors, and (unfortunately) chaining log processors isn't a first-class concept in the Logs SDK (yet), and therefore not well supported via declarative configuration.
Patterns like processor chaining are already described in the Supplementary Guidelines: Advanced Processing. I don’t support introducing new abstractions if existing ones can already express the same behavior.
and therefore not well supported via declarative configuration.
Regarding the claim that chaining isn’t well supported declaratively, could you clarify why?
The ComponentsProvider should be able to recursively create and resolve components (e.g., processors). The same mechanism would likely apply to samplers (e.g. open-telemetry/opentelemetry-configuration#340, open-telemetry/opentelemetry-configuration#341). There are already custom span processors implemented as wrappers/decorators, so this isn’t a new concept and should be feasible to support. If this is not possible then I think we should create an issue for declarative configuration rather than block this PR.
I also have a (smaller) concern about the lack of clarity on the overlap between this feature and Logger Config, which is related to why #4648.
Note that LogRecordProcessor.Enabled feature addresses use cases that LoggerConfig cannot cover.
-
Fine-grained runtime control over processor activation
LogRecordProcessor.Enabledallows enabling/disabling individual processors dynamically (e.g., based on configuration, environment variables, or runtime signals).LoggerConfig, by contrast, operates at the logger level — once a logger is configured, all its processors remain active.- Example: selectively disable an expensive export processor in low-latency environments without removing it from configuration.
-
Selective filtering or conditional processing
- A
LogRecordProcessorcan check itsEnabledflag before performing expensive transformations or network calls. - This enables conditional pipelines such as:
[Processor A (Enabled=true)] → [Processor B (Enabled=false)] → [Exporter] - With
LoggerConfig, you can’t skip individual processors — only the entire logger.
- A
-
Feature gating and experimental rollouts
LogRecordProcessor.Enabledcan be toggled per feature flag or configuration rollout.- This makes it easy to gradually enable new log enrichment or sampling processors for specific tenants or environments.
-
Dynamic performance tuning
- In high-throughput systems, you might disable certain processors (e.g., attribute-enriching or structured formatting ones) at runtime when load increases, then re-enable them once the system stabilizes.
LoggerConfigis static; it requires reinitializing the logger or SDK to change the pipeline.
-
Composable processor chains
- When chaining processors (e.g., enrichment → sampling → export),
Enabledcan control which processors are active in the chain without rebuilding it. - This pattern is essential for experimentation or A/B testing of different processing strategies.
- When chaining processors (e.g., enrichment → sampling → export),
-
Support for high-performance native destinations (e.g., Linux
user_events, Windows ETW)- Some processors may target low-latency, kernel-integrated destinations like:
- Linux
user_events(efficient user-space tracing) - ETW (Event Tracing for Windows)
- Linux
- These destinations often require minimal overhead and can’t tolerate the full overhead of all active processors.
- Using
LogRecordProcessor.Enabled, such processors can be conditionally activated only when native tracing is enabled, ensuring the SDK remains performant by skipping unnecessary processing paths. LoggerConfigcannot express this kind of dynamic toggling or destination-aware enablement.
- Some processors may target low-latency, kernel-integrated destinations like:
More:
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.
I agree that there are good use cases for LogRecordProcessor.Enabled, but that doesn't invalidate the lack of clarity around other use cases which is what led to #4648.
As I said though that's a smaller concern and I think that can be ironed out after stability.
For the declarative configuration concern, I totally agree it's possible. The problem (to me) is that it's custom and adhoc, which leads to a poorer declarative configuration experience. And I think we're missing an opportunity to think of new features from a declarative configuration first perspective.
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.
I agree that there are good use cases for
LogRecordProcessor.Enabled, but that doesn't invalidate the lack of clarity around other use cases which is what led to #4648.As I said though that's a smaller concern and I think that can be ironed out after stability.
Thanks, I think that’s a fair point. I agree that clarity around the boundary between LogRecordProcessor.Enabled and other mechanisms (like LoggerConfig) will need to be better articulated, and it’s reasonable to revisit that once the feature stabilizes.
For the declarative configuration concern, I totally agree it's possible. The problem (to me) is that it's custom and adhoc, which leads to a poorer declarative configuration experience. And I think we're missing an opportunity to think of new features from a declarative configuration first perspective.
On the declarative configuration side, yes, the current approach is indeed somewhat ad hoc. However, I’d argue that this flexibility is intentional. The goal is to enable experimentation and unblock valid use cases now without prematurely constraining future declarative design. Once we have a clearer picture of how users actually combine these features, we can evolve the configuration model in a more systematic way. For instance, we can add filtering processors or other specialized components once we see that people ask for them or use them frequently in practice.
Fixes #4458
According to spec compliance matrix this feature is already implemented in 3 languages, even though it is marked as optional.
In OTel Go, the design and implementation of this feature have remained unchanged since 2025-03-05. Since that release, we haven’t received any feedback or proposals suggesting changes.
Given this, I would like to propose stabilizing this feature in the specification. Doing so is also a prerequisite for stabilizing the OTel Go Logs SDK.
From #4458 (comment)