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

Do not duplicate validation of exponential histogram configuration #4406

Closed
MrAlias opened this issue Aug 3, 2023 · 1 comment
Closed

Do not duplicate validation of exponential histogram configuration #4406

MrAlias opened this issue Aug 3, 2023 · 1 comment

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Aug 3, 2023

The validation of the exponential histogram, already performed in sdk/metric/aggregation, is duplicated in the exponential histogram implementation (sdk/metric/internal/aggregate). The view will use the Err from ExponentialHistogram in sdk/metric/aggregation to validate the configuration prior to getting here:

if err := agg.Err(); err != nil {
global.Error(
err, "not using aggregation with view",
"criteria", criteria,
"mask", mask,
)
agg = nil
}

Having a duplicate validation of the configuration here will lead to two parts of the code-base doing the same thing and introduce the possibility for many bugs.

Similar to the explicit bucket histogram, we should just assume the configuration passed here is valid and proceed.

Originally posted by @MrAlias in #4245 (comment)

@MrAlias
Copy link
Contributor Author

MrAlias commented Aug 9, 2023

normalizeConfig was removed in the original PR.

@MrAlias MrAlias closed this as completed Aug 9, 2023
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