-
Notifications
You must be signed in to change notification settings - Fork 851
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
Memory mode: Adding support for synchronous instruments - explicit histogram #6153
Memory mode: Adding support for synchronous instruments - explicit histogram #6153
Conversation
…moryMode.java Co-authored-by: jack-berg <[email protected]>
…lder.java Co-authored-by: jack-berg <[email protected]>
…t1' into memory-mode-sync-instruments-part1
…t: Exponential Histogram
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6153 +/- ##
============================================
- Coverage 91.03% 90.94% -0.10%
- Complexity 5648 5670 +22
============================================
Files 618 620 +2
Lines 16438 16535 +97
Branches 1663 1682 +19
============================================
+ Hits 14965 15037 +72
- Misses 1011 1020 +9
- Partials 462 478 +16 ☔ View full report in Codecov by Sentry. |
Added ProfileBenchmark.
Added ProfileBenchmark.
…nc-instruments-explicit-histogram
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of minor comments but looks good. Also, can you rebase and mark ready for review?
...t/java/io/opentelemetry/sdk/metrics/internal/state/tester/ExplicitBucketHistogramTester.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/opentelemetry/sdk/metrics/internal/data/HistogramPointDataValidations.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/opentelemetry/sdk/metrics/internal/data/HistogramPointDataValidations.java
Outdated
Show resolved
Hide resolved
…struments-explicit-histogram
@jack-berg I fixed all comments and rebased it from the |
Epic
This is one of many PRs that implement #5105. See the complete plan here.
Specifically, this PR adds the implementation of
MemoryMode
for the synchronous instrument: explicit histogram.What was done here?
This PR, relative to Exponential Histogram PR, is rather trivial. It contains:
MutableHistogramPointData
and when REUSABLE_DATA is set, it simply re-uses that reusable point to store the aggregated result and returns it.HistogramPointData
ImmutableHistogramPointData
into a shared class, to be used also at the mutable version.It is based on #6136; hence, once it is merged, I'll merge it from the main branch to show its true content.
I will convert it from draft to PR once the previous PR is merged.