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.

Sign up for GitHub

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

Jump to bottom

Histogram bounds must be validated #2566

Closed
cijothomas opened this issue Nov 5, 2021 · 3 comments · Fixed by #2573
Closed

Histogram bounds must be validated #2566

cijothomas opened this issue Nov 5, 2021 · 3 comments · Fixed by #2573
Assignees
@mic-max
Labels
metrics Metrics signal related
Milestone
1.2.0

Comments

@cijothomas
Copy link
Member

cijothomas commented Nov 5, 2021 •
edited
Loading

Similar to instrument/view name, we need to validate the custom bounds (no duplicates, increasing order etc.), if provided.
If we can fail-fast, do that. - done in #2573
Else log and move on.
@cijothomas cijothomas added the metrics Metrics signal related label Nov 5, 2021
This was referenced Nov 5, 2021
Closed
Merged
@cijothomas cijothomas reopened this Nov 10, 2021
@mic-max mic-max mentioned this issue Nov 10, 2021
Closed
3 tasks
@cijothomas cijothomas added this to the 1.2.0 milestone Nov 16, 2021
@cijothomas
Copy link
Member Author

#3117 and #3120 addressed the "log and move-on" part.

@cijothomas
Copy link
Member Author

Closing as this is handled. If there are follow up, @mic-max please share here.

@mic-max
Copy link
Contributor

mic-max commented Apr 13, 2022 •
edited
Loading

Closing as this is handled. If there are follow up, @mic-max please share here.

https://github.com/open-telemetry/opentelemetry-dotnet/pull/3117/files#r849928032 Just this one case where double.NaN could be used as a boundary. I'll make a PR adding some tests extracted from #2592 to this as well since they should still be beneficial

Created an issue to track: #3171

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mic-max mic-max

Labels
metrics Metrics signal related
Projects
None yet
Milestone
1.2.0
2 participants
@cijothomas @mic-max