opentelemetry-java icon indicating copy to clipboard operation
opentelemetry-java copied to clipboard

Prototype exposing the exemplar reservoir for upcoming spec changes

Open jsuereth opened this issue 2 years ago • 3 comments

  • Create ExemplarReservoirFactory to help abstract over Double/Long hell
  • Create AggregationExtension that allows specifying an exemplar reservoir factory
  • Add a test which hacks RNG to make sure this all works correctly.

This is for open discussion on how we want to expose customization of ExemplarReservoir to users. One of the two remaining blockers for stabilizing Exemplar specification.

cc @jack-berg

jsuereth avatar Nov 03 '23 14:11 jsuereth

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Files Coverage Δ
...etry/sdk/metrics/internal/view/SumAggregation.java 92.30% <100.00%> (+1.39%) :arrow_up:
...nal/view/Base2ExponentialHistogramAggregation.java 95.45% <88.88%> (-4.55%) :arrow_down:
...try/sdk/metrics/internal/view/DropAggregation.java 66.66% <0.00%> (-13.34%) :arrow_down:
...ernal/view/ExplicitBucketHistogramAggregation.java 94.73% <87.50%> (-5.27%) :arrow_down:
...dk/metrics/internal/view/LastValueAggregation.java 82.35% <91.66%> (+7.35%) :arrow_up:
...cs/internal/exemplar/ExemplarReservoirFactory.java 66.66% <66.66%> (ø)
.../sdk/metrics/internal/view/DefaultAggregation.java 70.37% <66.66%> (-12.97%) :arrow_down:

... and 5 files with indirect coverage changes

:loudspeaker: Thoughts on this report? Let us know!.

codecov[bot] avatar Nov 03 '23 14:11 codecov[bot]

Just the one comment but conceptually I think this makes sense.

jack-berg avatar Nov 03 '23 15:11 jack-berg

Wanted to add another comment about things we should likely be exposing to users with this change (once specification is written and publicized):

  • ExemplarReservoirFactory would be exposed in some fashion. We can play with its final API to limit it if we'd like, or even expose it to InstrumentationDescriptor if we think that's necessary. I like keeping it simple, as it is today.
  • ExemplarReservoir the interface would have to be exposed so you can implement it.
  • I also believe that due to the complexities in making an efficient reservoir, we should expose:
    • FixedSizeExemplarReservoir as the baseline abstraction users can use to build their own reservoirs. This would remain abstract.
    • ReservoirCellSelector as the key component a user would be overriding in most implementations.
    • ReservoirCell as the key mechanism to pre-allocate memory and avoid memory issues in reservoirs. This would be marked final.

jsuereth avatar Nov 03 '23 15:11 jsuereth