Skip to content

Commit

Permalink
[prometheus components] For histograms without sums, leave the sum un…
Browse files Browse the repository at this point in the history
…set (#9120)

* [prometheusreceiver/prometheusremotewriteexporter] for histograms without sums, leave the sum unset

* use hasSum instead of pointer
  • Loading branch information
dashpole authored Sep 7, 2022
1 parent 7ae3abe commit 213d9c6
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 26 deletions.
11 changes: 10 additions & 1 deletion exporter/prometheusremotewriteexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,8 @@ func Test_PushMetrics(t *testing.T) {

emptyDataPointHistogramBatch := getMetricsFromMetricList(validMetrics1[validEmptyHistogram], validMetrics2[validEmptyHistogram])

histogramNoSumBatch := getMetricsFromMetricList(validMetrics1[validHistogramNoSum], validMetrics2[validHistogramNoSum])

summaryBatch := getMetricsFromMetricList(validMetrics1[validSummary], validMetrics2[validSummary])

// len(BucketCount) > len(ExplicitBounds)
Expand Down Expand Up @@ -494,7 +496,14 @@ func Test_PushMetrics(t *testing.T) {
name: "valid_empty_histogram_case",
metrics: &emptyDataPointHistogramBatch,
reqTestFunc: checkFunc,
expectedTimeSeries: 6,
expectedTimeSeries: 4,
httpResponseCode: http.StatusAccepted,
},
{
name: "histogram_no_sum_case",
metrics: &histogramNoSumBatch,
reqTestFunc: checkFunc,
expectedTimeSeries: 10,
httpResponseCode: http.StatusAccepted,
},
{
Expand Down
31 changes: 19 additions & 12 deletions exporter/prometheusremotewriteexporter/testutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ var (
label22 = "test_label22"
value22 = "test_value22"

intVal1 int64 = 1
intVal2 int64 = 2
floatVal1 = 1.0
floatVal2 = 2.0
floatVal3 = 3.0
intVal1 int64 = 1
intVal2 int64 = 2
floatValZero = 0.0
floatVal1 = 1.0
floatVal2 = 2.0
floatVal3 = 3.0

lbs1 = getAttributes(label11, value11, label12, value12)
lbs2 = getAttributes(label21, value21, label22, value22)
Expand All @@ -62,6 +63,7 @@ var (
validSum = "valid_Sum"
validHistogram = "valid_Histogram"
validEmptyHistogram = "valid_empty_Histogram"
validHistogramNoSum = "valid_Histogram_No_Sum"
validSummary = "valid_Summary"
suffixedCounter = "valid_IntSum_total"

Expand All @@ -76,7 +78,8 @@ var (
validIntSum: getIntSumMetric(validIntSum, lbs1, intVal1, time1),
suffixedCounter: getIntSumMetric(suffixedCounter, lbs1, intVal1, time1),
validSum: getSumMetric(validSum, lbs1, floatVal1, time1),
validHistogram: getHistogramMetric(validHistogram, lbs1, time1, floatVal1, uint64(intVal1), bounds, buckets),
validHistogram: getHistogramMetric(validHistogram, lbs1, time1, &floatVal1, uint64(intVal1), bounds, buckets),
validHistogramNoSum: getHistogramMetric(validHistogramNoSum, lbs1, time1, nil, uint64(intVal1), bounds, buckets),
validEmptyHistogram: getHistogramMetricEmptyDataPoint(validEmptyHistogram, lbs1, time1),
validSummary: getSummaryMetric(validSummary, lbs1, time1, floatVal1, uint64(intVal1), quantiles),
}
Expand All @@ -85,11 +88,12 @@ var (
validDoubleGauge: getDoubleGaugeMetric(validDoubleGauge, lbs2, floatVal2, time2),
validIntSum: getIntSumMetric(validIntSum, lbs2, intVal2, time2),
validSum: getSumMetric(validSum, lbs2, floatVal2, time2),
validHistogram: getHistogramMetric(validHistogram, lbs2, time2, floatVal2, uint64(intVal2), bounds, buckets),
validHistogram: getHistogramMetric(validHistogram, lbs2, time2, &floatVal2, uint64(intVal2), bounds, buckets),
validHistogramNoSum: getHistogramMetric(validHistogramNoSum, lbs2, time2, nil, uint64(intVal2), bounds, buckets),
validEmptyHistogram: getHistogramMetricEmptyDataPoint(validEmptyHistogram, lbs2, time2),
validSummary: getSummaryMetric(validSummary, lbs2, time2, floatVal2, uint64(intVal2), quantiles),
validIntGaugeDirty: getIntGaugeMetric(validIntGaugeDirty, lbs1, intVal1, time1),
unmatchedBoundBucketHist: getHistogramMetric(unmatchedBoundBucketHist, pcommon.NewMap(), 0, 0, 0,
unmatchedBoundBucketHist: getHistogramMetric(unmatchedBoundBucketHist, pcommon.NewMap(), 0, &floatValZero, 0,
pcommon.NewImmutableFloat64Slice([]float64{0.1, 0.2, 0.3}),
pcommon.NewImmutableUInt64Slice([]uint64{1, 2})),
}
Expand Down Expand Up @@ -130,8 +134,8 @@ var (
staleNaNDoubleGauge: getDoubleGaugeMetric(staleNaNDoubleGauge, lbs1, floatVal1, time1),
staleNaNIntSum: getIntSumMetric(staleNaNIntSum, lbs1, intVal1, time1),
staleNaNSum: getSumMetric(staleNaNSum, lbs1, floatVal1, time1),
staleNaNHistogram: getHistogramMetric(staleNaNHistogram, lbs1, time1, floatVal2, uint64(intVal2), bounds, buckets),
staleNaNEmptyHistogram: getHistogramMetric(staleNaNEmptyHistogram, lbs1, time1, floatVal2, uint64(intVal2),
staleNaNHistogram: getHistogramMetric(staleNaNHistogram, lbs1, time1, &floatVal2, uint64(intVal2), bounds, buckets),
staleNaNEmptyHistogram: getHistogramMetric(staleNaNEmptyHistogram, lbs1, time1, &floatVal2, uint64(intVal2),
pcommon.NewImmutableFloat64Slice([]float64{}),
pcommon.NewImmutableUInt64Slice([]uint64{})),
staleNaNSummary: getSummaryMetric(staleNaNSummary, lbs2, time2, floatVal2, uint64(intVal2), quantiles),
Expand Down Expand Up @@ -307,7 +311,7 @@ func getHistogramMetricEmptyDataPoint(name string, attributes pcommon.Map, ts ui
return metric
}

func getHistogramMetric(name string, attributes pcommon.Map, ts uint64, sum float64, count uint64, bounds pcommon.ImmutableFloat64Slice,
func getHistogramMetric(name string, attributes pcommon.Map, ts uint64, sum *float64, count uint64, bounds pcommon.ImmutableFloat64Slice,
buckets pcommon.ImmutableUInt64Slice) pmetric.Metric {
metric := pmetric.NewMetric()
metric.SetName(name)
Expand All @@ -318,7 +322,10 @@ func getHistogramMetric(name string, attributes pcommon.Map, ts uint64, sum floa
dp.SetFlagsImmutable(pmetric.DefaultMetricDataPointFlags.WithNoRecordedValue(true))
}
dp.SetCount(count)
dp.SetSum(sum)

if sum != nil {
dp.SetSum(*sum)
}
dp.SetBucketCounts(buckets)
dp.SetExplicitBounds(bounds)
attributes.CopyTo(dp.Attributes())
Expand Down
25 changes: 15 additions & 10 deletions pkg/translator/prometheusremotewrite/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,17 +284,22 @@ func addSingleHistogramDataPoint(pt pmetric.HistogramDataPoint, resource pcommon
time := convertTimeStamp(pt.Timestamp())
// sum, count, and buckets of the histogram should append suffix to baseName
baseName := prometheustranslator.BuildPromCompliantName(metric, settings.Namespace)
// treat sum as a sample in an individual TimeSeries
sum := &prompb.Sample{
Value: pt.Sum(),
Timestamp: time,
}
if pt.FlagsImmutable().NoRecordedValue() {
sum.Value = math.Float64frombits(value.StaleNaN)
}

sumlabels := createAttributes(resource, pt.Attributes(), settings.ExternalLabels, nameStr, baseName+sumStr)
addSample(tsMap, sum, sumlabels, metric.DataType().String())
// If the sum is unset, it indicates the _sum metric point should be
// omitted
if pt.HasSum() {
// treat sum as a sample in an individual TimeSeries
sum := &prompb.Sample{
Value: pt.Sum(),
Timestamp: time,
}
if pt.FlagsImmutable().NoRecordedValue() {
sum.Value = math.Float64frombits(value.StaleNaN)
}

sumlabels := createAttributes(resource, pt.Attributes(), settings.ExternalLabels, nameStr, baseName+sumStr)
addSample(tsMap, sum, sumlabels, metric.DataType().String())
}

// treat count as a sample in an individual TimeSeries
count := &prompb.Sample{
Expand Down
8 changes: 6 additions & 2 deletions receiver/prometheusreceiver/internal/metricfamily.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@ func (mg *metricGroup) toDistributionPoint(dest pmetric.HistogramDataPointSlice)
point.SetFlagsImmutable(pmetric.DefaultMetricDataPointFlags.WithNoRecordedValue(true))
} else {
point.SetCount(uint64(mg.count))
point.SetSum(mg.sum)
if mg.hasSum {
point.SetSum(mg.sum)
}
}

point.SetExplicitBounds(pcommon.NewImmutableFloat64Slice(bounds))
Expand Down Expand Up @@ -171,7 +173,9 @@ func (mg *metricGroup) toSummaryPoint(dest pmetric.SummaryDataPointSlice) {
if pointIsStale {
point.SetFlagsImmutable(pmetric.DefaultMetricDataPointFlags.WithNoRecordedValue(true))
} else {
point.SetSum(mg.sum)
if mg.hasSum {
point.SetSum(mg.sum)
}
point.SetCount(uint64(mg.count))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,6 @@ func Test_OTLPMetricBuilder_histogram(t *testing.T) {
hist0.SetAggregationTemporality(pmetric.MetricAggregationTemporalityCumulative)
pt0 := hist0.DataPoints().AppendEmpty()
pt0.SetCount(3)
pt0.SetSum(0)
pt0.SetExplicitBounds(pcommon.NewImmutableFloat64Slice([]float64{10, 20}))
pt0.SetBucketCounts(pcommon.NewImmutableUInt64Slice([]uint64{1, 1, 1}))
pt0.SetTimestamp(startTsNanos)
Expand Down
16 changes: 16 additions & 0 deletions unreleased/prom-unset-sum.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: prometheusreceiver/prometheusremotewriteexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Leave the sum unset in histograms without sums, and don't produce _sum metric points for histograms without sums

# One or more tracking issues related to the change
issues: [7546]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

0 comments on commit 213d9c6

Please sign in to comment.