[processor/deltatocumulative]: explicit-bounds histograms #33983
Conversation
sh0rez
changed the title
sh0rez
force-pushed
the
deltatocumulative-histograms
branch
2 times, most recently
from
bf80765 to
8645aa8
Compare
sh0rez
changed the title
|
@RichieSams can you please take a look? @djaglowski because @jpkrohling is out, would you be able to merge once reviewed and approved? |
| panic("todo") | ||
| // bounds different: no way to merge, so reset observation to new boundaries | ||
| if !pslice.Equal(dp.ExplicitBounds(), in.ExplicitBounds()) { | ||
| in.MoveTo(dp.HistogramDataPoint) |
There was a problem hiding this comment.
Should we add a metric for how often this occurs?
There was a problem hiding this comment.
sgtm, but will do later as we don't have a good way of accessing a Meter here without refactoring the telemetry package
| in.MoveTo(dp.HistogramDataPoint) | ||
| return dp | ||
| } | ||
|
|
There was a problem hiding this comment.
Do we need to do the StartTimeUnixNano checks like we do for Sums?
There was a problem hiding this comment.
we are! the code I think you talk about is in delta.Accumulator[T], which applies to all datatypes
There was a problem hiding this comment.
Ahhh. that's why I missed it :)
|
Added two comments. Overall LGTM |
sh0rez
force-pushed
the
deltatocumulative-histograms
branch
from
57d7906 to
e309836
Compare
|
👋 👋 is there any way of testing this PR out? We currently got bitten by the lack of support for normal histograms, moving to exponentials is not a real option right now and this looks like it adds the required support |
|
@xocasdashdash yes, when building your own collector (https://opentelemetry.io/docs/collector/custom-collector/) you can specify this branch as the version for deltatocumulative (using a replace directive) |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
adds aggregation support for fixed-bucket / explicit bounds histograms
sh0rez
force-pushed
the
deltatocumulative-histograms
branch
from
99715c4 to
b89bee5
Compare
…etry#33983) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> adds aggregation support for fixed-bucket / explicit bounds histograms **Link to tracking Issue:** open-telemetry#30705 **Testing:** unit tests added **Documentation:** none
Description:
adds aggregation support for fixed-bucket / explicit bounds histograms
Link to tracking Issue: #30705
Testing: unit tests added
Documentation: none