Histogram Aggregation - OTLP exporter #2582
Conversation
beanliu
requested review from
Oberon00,
anuraaga,
arminru,
bogdandrutu,
carlosalberto,
jkwatson,
pavolloffay,
thisthat and
tylerbenson
as code owners
Codecov Report
@@ Coverage Diff @@
## main #2582 +/- ##
============================
============================
Continue to review full report at Codecov.
|
exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/MetricAdapter.java
Show resolved
Hide resolved
|
Please rebase, I think this can be merged/finished before the aggregator PR (if ready) :) |
|
the PR has been rebased and updated according to the latest changes. |
exporters/otlp/common/src/main/java/io/opentelemetry/exporter/otlp/internal/MetricAdapter.java
Outdated
Show resolved
Hide resolved
| List<Double> boundaries = doubleHistogramPoint.getBoundaries(); | ||
| if (!boundaries.isEmpty()) { | ||
| builder.addAllExplicitBounds(boundaries); | ||
| } |
There was a problem hiding this comment.
Counts as well. So we add boundaries and counts if boundaries not empty, otherwise the counts will be a list with one long equivalent with count.
There was a problem hiding this comment.
counts will be a list with one long equivalent with count.
that looks like a valid OTLP histogram point to me according to https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/metrics/v1/metrics.proto#L428.
boundaries and counts both being empty might be a problem to the downstream consumers since it's not in the contract.
There was a problem hiding this comment.
There was a problem hiding this comment.
Comment is written with the mindset that there will be more options to define the boundaries like linear or exponential
There was a problem hiding this comment.
not sure if I follow. seems to me that the minimal size of buckets should be one according to https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/metrics/v1/metrics.proto#L414
// The number of elements in bucket_counts array must be by one greater than
// the number of elements in explicit_bounds array.
and the service we built treats histogram data points without buckets invalid.
4 participants
follow up PR of #2581