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

Fix guard of measured value to not record empty #4452

Merged
merged 2 commits into from
Aug 18, 2023

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Aug 16, 2023

A guard was added in #4446 to prevent non-normal float64 from being recorded. This was added in the low-level record method meaning that the higher-level measure method will still keep a record of the invalid value measurement, just with a zero-value.

This fixes that issue by moving the guard to the measure method.

A guard was added in open-telemetry#4446 to prevent non-normal float64 from being
recorded. This was added in the low-level `record` method meaning that
the higher-level `measure` method will still keep a record of the
invalid value measurement, just with a zero-value.

This fixes that issue by moving the guard to the `measure` method.
@MrAlias MrAlias added bug Something isn't working pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics Skip Changelog PRs that do not require a CHANGELOG.md entry labels Aug 16, 2023
@MrAlias MrAlias added this to the v1.17.0/v0.40.0 milestone Aug 16, 2023
@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Merging #4452 (db94497) into main (9b47674) will not change coverage.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #4452   +/-   ##
=====================================
  Coverage   78.9%   78.9%           
=====================================
  Files        254     254           
  Lines      20642   20642           
=====================================
  Hits       16301   16301           
  Misses      3996    3996           
  Partials     345     345           
Files Changed Coverage Δ
...metric/internal/aggregate/exponential_histogram.go 96.1% <100.0%> (ø)

@MrAlias MrAlias merged commit 16ce491 into open-telemetry:main Aug 18, 2023
@MrAlias MrAlias deleted the guard-expo-hist-meas branch August 18, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics bug Something isn't working pkg:SDK Related to an SDK package Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants