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 2 #3468

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

reyang
Copy link
Member

@reyang reyang commented Jul 21, 2022

Following #3462.

Changes

  • Introduced a circular-buffer-based implementation of continuous buckets, which will be used by exponential histogram.
  • Improved the handling of subnormal and abnormal numbers.
  • Slightly improved the readability of existing code (formatting, better documents, etc.).

@reyang reyang requested a review from a team July 21, 2022 20:18
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. No change is user facing yet.

@codecov
Copy link

codecov bot commented Jul 21, 2022

Codecov Report

Merging #3468 (2577e97) into main (54e83a9) will increase coverage by 0.17%.
The diff coverage is 83.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3468      +/-   ##
==========================================
+ Coverage   86.30%   86.48%   +0.17%     
==========================================
  Files         267      268       +1     
  Lines        9634     9660      +26     
==========================================
+ Hits         8315     8354      +39     
+ Misses       1319     1306      -13     
Impacted Files Coverage Δ
...Metrics/ExponentialBucketHistogramConfiguration.cs 0.00% <0.00%> (ø)
...penTelemetry/Metrics/ExponentialBucketHistogram.cs 73.33% <71.42%> (-3.34%) ⬇️
src/OpenTelemetry/Metrics/CircularBufferBuckets.cs 100.00% <100.00%> (ø)
src/OpenTelemetry/Logs/OpenTelemetryLogger.cs 86.66% <0.00%> (-4.45%) ⬇️
src/OpenTelemetry/BatchExportProcessor.cs 82.24% <0.00%> (-1.87%) ⬇️
...Telemetry/Internal/SelfDiagnosticsEventListener.cs 97.65% <0.00%> (+0.78%) ⬆️
...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs 76.66% <0.00%> (+1.66%) ⬆️
...ter.ZPages/Implementation/ZPagesActivityTracker.cs 100.00% <0.00%> (+2.85%) ⬆️
...lemetry/Internal/SelfDiagnosticsConfigRefresher.cs 92.30% <0.00%> (+5.76%) ⬆️
src/OpenTelemetry/Logs/Pool/LogRecordSharedPool.cs 100.00% <0.00%> (+21.05%) ⬆️

@cijothomas cijothomas merged commit cf3dfc5 into open-telemetry:main Jul 21, 2022
@reyang reyang deleted the reyang/exp2 branch July 21, 2022 20:39
/// <summary>
/// A histogram buckets implementation based on circular buffer.
/// </summary>
internal class CircularBufferBuckets
Copy link
Member

Choose a reason for hiding this comment

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

nit: This we probably should seal for perf but also to prevent co-ops?

@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