Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Allow limits to be set through TracerProvider interface #1859

owais started this conversation in Ideas
Discussion options

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.

You must be logged in to vote

Replies: 4 comments 3 replies

Comment options

owais
May 19, 2021
Collaborator Author

@open-telemetry/python-approvers @open-telemetry/python-maintainers ^

You must be logged in to vote
0 replies
Comment options

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)

You must be logged in to vote
2 replies
Comment options

+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).

Comment options

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.

Comment options

owais
May 24, 2021
Collaborator Author

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.

You must be logged in to vote
1 reply
Comment options

This sounds good to me.

Comment options

owais
May 27, 2021
Collaborator Author

Proposal implemented here: #1877

You must be logged in to vote
0 replies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Category
Ideas
Labels
None yet

AltStyle によって変換されたページ (->オリジナル) /