-
Notifications
You must be signed in to change notification settings - Fork 755
-
Today we support the following configurable limits via environment variables:
OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT
OTEL_SPAN_EVENT_COUNT_LIMIT
OTEL_SPAN_LINK_COUNT_LIMIT
It would be useful if TracerProvider could allow users to set limits through code as well in addition to env vars. I propose we add a new SpanLimits class that can be used to set limits with code or load them from env, and update TracerProvider to accept an instance of this new class. TracerProvider already accepts Resource, Sampler and IdGenerator objects today. Adding SpanLimits would be consistent with the existing interface. Usage would look like:
limits = SpanLimits(max_attributes=100, max_events=100, max_links=100) provider = TracerProvider(limits=limits)
Alternatively, TracerProvider could accept each limit individually instead of passing all limits as a class. Usage would look like:
provider = TracerProvider(max_attributes_limit=100, max_events_limit=100, max_links_limit=100)
Both interfaces work equally well but I think having a class has some advantages such as:
- All logic related to limits such as setting defaults, accepting UNSET, loading values from env would be consolidated in a class and not leak into TracerProvider. TracerProvider has no business loading/parsing configuration of any kind IMO.
- A class is more future proof. It is easier to extend the class and add new features in future without major surgeries. For example:
- SpanLimits class can easily be updated to load limits config from a remote config source dynamically without recreating providers or restarting the service.
- SpanLimits could change dynamically based on some heuristics similar to Sampler. Spans/Resources could call private methods as
self._limits.max_attributes(self). SpanLimits could then return different values based on some heuristics.
It is easy to imagine many use cases that a class could enable in the future. Today both approaches are somewhat similar but using a class keeps the doors open for more features/changes in the future.
- Another benefit of using a class is that it is a little bit easier for users to manage in code. For example, if users configure limits and pass them to multiple tracer providers, they'd need to define, import and hold references to many variables vs instantiating a single variable and passing it around to be used multiple times.
- We'll also need limits in MeterProvider soon (Add an option to limit length of values of attributes and metric values opentelemetry-specification#1130 ). I think it would be nicer to have a single SpanLimits implementation and be able to pass it to both TracerProvider and MeterProvider just like we pass Resource objects today. I feel both user code and our library code would be unnecessarily a bit more scattered if we pass raw values to be MeterProvider and TracerProvider.
One downside to using a class is that Spans/Resources will probably need an extra attribute lookup for each limit but this can be easily optimized for performance later if required.
I realize the benefits are probably not enormous but they're definitely not negligible.
Happy to hear thoughts on these two approaches or any other ideas.
Update: Other otel implementations implement this with classes/structs but all of them use SpanLimits name instead of Limits for this. I feel Limits is better as we can it on Resource attributes as well and may be use it to configure limits for metrics as well in future but in order to stay consistent with other SDKs, I'm changing the proposal to propose SpanLimits instead. We can add a different API to configure Meter limits in future. We can still use SpanLimits with resource attributes even if the name suggests otherwise or we can completely ignore Resources when it comes to limits as they usually are not susceptible to very large number or size of attributes unlike spans.
References from other SDKs:
Java
Node/JS
Go
Rust
Swift
Rust also allows setting individual limit values in addition to a struct.
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 4 comments 3 replies
-
@open-telemetry/python-approvers @open-telemetry/python-maintainers ^
Beta Was this translation helpful? Give feedback.
All reactions
-
following the zen of Python where it says to keep things as easy and straight as possible from the developer perspective, could potentially prefer
provider = TracerProvider(max_attributes_limit=100, max_events_limit=100, max_links_limit=100)
Beta Was this translation helpful? Give feedback.
All reactions
-
+1 for Limits class for the reasons list above. It is also consistent with the other "configurable" things that are passed into the TracerProvider today (Sampler, Resource, Processor, IdGenerator).
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
this convinced me to change opinion and now vote for the class-style implementation (as it better fits to pre-existing style for other configurable options.
Beta Was this translation helpful? Give feedback.
All reactions
-
Other otel implementations implement this with classes/structs but all of them use SpanLimits name instead of Limits for this. I feel Limits is better as we can it on Resource attributes as well and may be use it to configure limits for metrics as well in future but in order to stay consistent with other SDKs, I'm changing the proposal to propose SpanLimits instead. We can add a different API to configure Meter limits in future. We can still use SpanLimits with resource attributes even if the name suggests otherwise or we can completely ignore Resources when it comes to limits as they usually are not susceptible to very large number or size of attributes unlike spans.
Beta Was this translation helpful? Give feedback.
All reactions
-
This sounds good to me.
Beta Was this translation helpful? Give feedback.
All reactions
-
Proposal implemented here: #1877
Beta Was this translation helpful? Give feedback.