Skip to content
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

Add cardinality configuration option to sdk autoconfigure #5639

Closed
akats7 opened this issue Jul 19, 2023 · 4 comments · Fixed by #5659
Closed

Add cardinality configuration option to sdk autoconfigure #5639

akats7 opened this issue Jul 19, 2023 · 4 comments · Fixed by #5659
Labels
Feature Request Suggest an idea for this project

Comments

@akats7
Copy link

akats7 commented Jul 19, 2023

Is your feature request related to a problem? Please describe.
Currently the default metricReaders registered using AutoConfiguredOpenTelemetrySdkBuilder are all set to use the default cardinality limit selector, I'd like to be able to set cardinality options as config properties.

Describe the solution you'd like
Add cardinality configuration logic to MeterProviderConfiguration in the autoconfigure module

Describe alternatives you've considered
Re-register the already registered readers using MeterProviderCustomizer.

Additional context
I'd like to contribute this.

@akats7 akats7 added the Feature Request Suggest an idea for this project label Jul 19, 2023
@akats7
Copy link
Author

akats7 commented Jul 19, 2023

@jack-berg please let me know your thoughts

@jack-berg
Copy link
Member

This is tricky.. The natural thing to do would be to add support for a new system property / environment variables, i.e. OTEL_METRICS_EXPORTER_CARDINALITY_LIMIT. However, there's a moratorium while we work out a more future facing file based configuration strategy. That file configuration strategy would be the place where cardinality limit configuration would find a permanent home. However, it would be nice if we (otel java) could provide some temporary bridge for the time being, since as you've found out, its pretty challenging to try to set cardinality limits with autoconfigure.

@jkwatson what do you think about adding a OTEL_JAVA_EXPERIMENTAL_* environment variable to set the desired cardinality limit? For simplicities sake, we could have it set a single cardinality limit for all instruments. Cardinality limits can vary as a function of instrument kind, but encoding that into a temporary environment variable feels like overkill. The *_JAVA_EXPERIMENTAL_* signifies this is experimental and is likely to go away / change in the future, and is specific to the java ecosystem (i.e. don't go looking for this in otel js, go, etc).

@jkwatson
Copy link
Contributor

totally fine with an experimental env var. 👍🏽

@akats7
Copy link
Author

akats7 commented Jul 20, 2023

This is tricky.. The natural thing to do would be to add support for a new system property / environment variables, i.e. OTEL_METRICS_EXPORTER_CARDINALITY_LIMIT. However, there's a moratorium while we work out a more future facing file based configuration strategy. That file configuration strategy would be the place where cardinality limit configuration would find a permanent home. However, it would be nice if we (otel java) could provide some temporary bridge for the time being, since as you've found out, its pretty challenging to try to set cardinality limits with autoconfigure.

@jkwatson what do you think about adding a OTEL_JAVA_EXPERIMENTAL_* environment variable to set the desired cardinality limit? For simplicities sake, we could have it set a single cardinality limit for all instruments. Cardinality limits can vary as a function of instrument kind, but encoding that into a temporary environment variable feels like overkill. The *_JAVA_EXPERIMENTAL_* signifies this is experimental and is likely to go away / change in the future, and is specific to the java ecosystem (i.e. don't go looking for this in otel js, go, etc).

@jack-berg Thanks again for the context, sounds like a plan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project
Projects
None yet
3 participants