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

Reasoning for cardinality limit #5603

Closed
akats7 opened this issue Jul 6, 2023 · 6 comments
Closed

Reasoning for cardinality limit #5603

akats7 opened this issue Jul 6, 2023 · 6 comments
Labels
Feature Request Suggest an idea for this project

Comments

@akats7
Copy link

akats7 commented Jul 6, 2023

Hi all,

I wanted to inquire about the preset cardinality limit of 2000. I'm curious why this is a hard cap and set in the sdk instead of simply being enforced by the monitoring system.

Is there a reason its not configurable and set to this number specifically?

In our use case we're dealing with kafka metrics where topic is a critical attribute and is high cardinality by nature so we've been reaching the cardinality limit on certain nodes where the number of topics is higher. Removing topic as an attribute is not really an option for us.

Wanted to bring this up and get the thoughts of the team, thanks!

@akats7 akats7 added the Feature Request Suggest an idea for this project label Jul 6, 2023
@jkwatson
Copy link
Contributor

jkwatson commented Jul 6, 2023

@akats7
Copy link
Author

akats7 commented Jul 6, 2023

We are using the recommended default from the specification here: https://github.com/open-telemetry/opentelemetry-specification/blob/31e988297d9c3f1f89c3915aec13df4be1348d94/specification/metrics/sdk.md?plain=1#L719

@jkwatson Got it, so I understand making that the default but would it be possible to make that a parameter that can be overridden, I imagine our use case is not exclusive to us and for us to lower the cardinality we would have to break other best practices with semantic conventions. And even then we would still have to sacrifice attributes and omit entire metrics that are critical

@jack-berg
Copy link
Member

It will be configurable in the 1.28.0 release, which will be published tomorrow. We originally added it to protect against bad instrumentation producing high cardinality and causing apps to crash. When the java metrics SDK went stable, there wasn't any advice in the specification about protecting against high memory usage so we took a conservative approach.

@akats7
Copy link
Author

akats7 commented Jul 6, 2023

@jack-berg That's great to hear, thank you for providing that context.

@jack-berg
Copy link
Member

Oops I said something incorrect. Configurability of cardinality limits was added in #5494, which was first published in 1.27.0. 1.28.0 added #5560, which captures measurements above the cardinality limit in an overflow series.

Closing this issue.

@akats7
Copy link
Author

akats7 commented Jul 10, 2023

@jack-berg Would it be reasonable to be able to update the global max_cardinality through a config prop to avoid setting it per view.

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
Development

No branches or pull requests

3 participants