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

Exponential Bucket Histogram - part 3 #3473

Merged
merged 5 commits into from
Jul 22, 2022

Conversation

reyang
Copy link
Member

@reyang reyang commented Jul 22, 2022

Following #3462.

Changes

  • Introduced a lookup-table-based leading zero algorithm.
  • Improved the perf while handling IEEE 754 subnormal.
  • Implemented the scale reduction calculation (auto-merging is not covered by this PR).
  • Extracted math related functions to OpenTelemetry.Internal.MathHelper.

@reyang reyang requested a review from a team July 22, 2022 18:13
Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving to unblock progress on Exp. Histogram. No user facing impact until this is integrated with the SDK.

@codecov
Copy link

codecov bot commented Jul 22, 2022

Codecov Report

Merging #3473 (91d7404) into main (7de3f00) will decrease coverage by 0.08%.
The diff coverage is 80.88%.

❗ Current head 91d7404 differs from pull request most recent head bf5143e. Consider uploading reports for the commit bf5143e to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3473      +/-   ##
==========================================
- Coverage   86.43%   86.35%   -0.09%     
==========================================
  Files         268      269       +1     
  Lines        9660     9712      +52     
==========================================
+ Hits         8350     8387      +37     
- Misses       1310     1325      +15     
Impacted Files Coverage Δ
...penTelemetry/Metrics/ExponentialBucketHistogram.cs 57.89% <41.17%> (-15.44%) ⬇️
src/OpenTelemetry/Internal/MathHelper.cs 91.89% <91.89%> (ø)
....Prometheus/Implementation/PrometheusSerializer.cs 77.77% <100.00%> (ø)
src/OpenTelemetry/Metrics/CircularBufferBuckets.cs 100.00% <100.00%> (ø)
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs 76.47% <0.00%> (-5.89%) ⬇️
src/OpenTelemetry/Logs/OpenTelemetryLogger.cs 86.66% <0.00%> (-4.45%) ⬇️

@cijothomas cijothomas merged commit 795aedc into open-telemetry:main Jul 22, 2022
@reyang reyang deleted the reyang/exp3 branch July 22, 2022 18:25
@reyang reyang added pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package metrics Metrics signal related labels Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metrics Metrics signal related pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants