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

Specify MeterProvider configurable cardinality limits #2960

Merged
merged 37 commits into from May 8, 2023

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Nov 18, 2022

Fixes #1891.

EDIT: Updated to specify cardinality limits at the View/Instrument level with a Reader-level default. Updated to use a hard limit

Changes

Adds optional support for a maximum cardinality limit.

The recommended default is 2000, based on this comment by #1891 (comment) @jack-berg.

The Prometheus-WG SIG discussed this on Nov 9, 2022 and reached this recommended solution to the problem outlined in #1891. The consequence of exceeding these limits is in line with the current Prometheus server behavior, which drops targets that misbehave. The discussed was summarized here: #1891 (comment)

CHANGELOG.md Outdated Show resolved Hide resolved
@arminru arminru added area:sdk Related to the SDK area:configuration Related to configuring the SDK spec:metrics Related to the specification/metrics directory labels Nov 24, 2022
@jmacd jmacd changed the title Specify MetricReader configurable limits Specify MeterProvider configurable cardinality limits Nov 28, 2022
@jmacd
Copy link
Contributor Author

jmacd commented Nov 28, 2022

This PR has been updated to specify the limit at the MeterProvider level. PTAL.

specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
jmacd and others added 2 commits November 30, 2022 12:34
Co-authored-by: Georg Pirklbauer <georg.pirklbauer@dynatrace.com>
Co-authored-by: Georg Pirklbauer <georg.pirklbauer@dynatrace.com>
@github-actions
Copy link

github-actions bot commented Dec 8, 2022

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 8, 2022
@trask
Copy link
Member

trask commented Apr 21, 2023

/easycla

@carlosalberto
Copy link
Contributor

@open-telemetry/specs-approvers Please take a look at the latest iteration. This PR has been discussed/standing for a long time now, and would be great to have it adjusted/merged soon.

specification/metrics/sdk.md Outdated Show resolved Hide resolved
jmacd and others added 3 commits May 4, 2023 12:38
Co-authored-by: Alan West <3676547+alanwest@users.noreply.github.com>
@carlosalberto
Copy link
Contributor

@jmacd Should we include this in the May release?

@carlosalberto
Copy link
Contributor

Talked to @jmacd, we are ready to roll this in. Thanks a lot for the iteration (and patience)!

@carlosalberto carlosalberto merged commit 1997dd1 into open-telemetry:main May 8, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:configuration Related to configuring the SDK area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metrics SDK: define standard circuit breakers for uncollected, infinite cardinality recordings