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

Async instrument state: implement InstrumentCardinalityLimit #410

Open
jmacd opened this issue Mar 3, 2023 · 0 comments
Open

Async instrument state: implement InstrumentCardinalityLimit #410

jmacd opened this issue Mar 3, 2023 · 0 comments

Comments

@jmacd
Copy link
Member

jmacd commented Mar 3, 2023

Is your feature request related to a problem? Please describe.
The async instrument code path has a "feature" I would eliminate. It suppresses duplicate observations and ensures that each attribute set is counted once even when you repeat an observation (i.e., a duplicate measurement) in the same callback.

I would change the implementation to correctly sum these. Currently, #385 adds a View-state limit to the asynchronous instruments, because if implemented with the current duplicate suppression inside Async-state, the overflow aggregator observations are not correctly summed.

Describe the solution you'd like
The Async-state package should implement InstrumentCardinalityLimit by preventing more than a certain number of observed attribute sets. However, as currently implemented, the result of overflow handling would also re-order observations arbitrarily, because of an intermediate map, combined with Golang map iteration. To address this, the Async-state package should pass-through the observations in the order they are made, but then cut off the use of new attribute sets if the callback makes too many observations (correctly aggregating them into the overflow set).

Describe alternatives you've considered
Current code base -- considered not great, thus not much testing is done and no overflow handling is done in Async-state.

Additional context
open-telemetry/opentelemetry-specification#2960

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant