From af4fd0e429bdd8253a6fb44462ea038ed3fa1caa Mon Sep 17 00:00:00 2001 From: George Krajcsovits Date: Fri, 17 Nov 2023 23:31:12 +0100 Subject: [PATCH] [chore] [translator/prometheusremotewrite] ensure unknown reset hint (#29326) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure that native histogram reset hint is UNKNOWN and document reasoning. See discussion in the related PR: #28663 Signed-off-by: György Krajcsovits --- pkg/translator/prometheusremotewrite/histograms.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/translator/prometheusremotewrite/histograms.go b/pkg/translator/prometheusremotewrite/histograms.go index ec25abcefcca5..bae65448ae00b 100644 --- a/pkg/translator/prometheusremotewrite/histograms.go +++ b/pkg/translator/prometheusremotewrite/histograms.go @@ -74,7 +74,17 @@ func exponentialToNativeHistogram(p pmetric.ExponentialHistogramDataPoint) (prom nSpans, nDeltas := convertBucketsLayout(p.Negative(), scaleDown) h := prompb.Histogram{ - Schema: scale, + // The counter reset detection must be compatible with Prometheus to + // safely set ResetHint to NO. This is not ensured currently. + // Sending a sample that triggers counter reset but with ResetHint==NO + // would lead to Prometheus panic as it does not double check the hint. + // Thus we're explicitly saying UNKNOWN here, which is always safe. + // TODO: using created time stamp should be accurate, but we + // need to know here if it was used for the detection. + // Ref: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/28663#issuecomment-1810577303 + // Counter reset detection in Prometheus: https://github.com/prometheus/prometheus/blob/f997c72f294c0f18ca13fa06d51889af04135195/tsdb/chunkenc/histogram.go#L232 + ResetHint: prompb.Histogram_UNKNOWN, + Schema: scale, ZeroCount: &prompb.Histogram_ZeroCountInt{ZeroCountInt: p.ZeroCount()}, // TODO use zero_threshold, if set, see