From 89777cd455987c7f55758e12370f84733c026373 Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Thu, 22 Sep 2022 11:46:43 +0200 Subject: [PATCH 1/3] update bucket default bounds to match the specification --- exporters/prometheus/testdata/histogram.txt | 5 +++++ sdk/metric/meter_test.go | 8 ++++---- sdk/metric/reader.go | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/exporters/prometheus/testdata/histogram.txt b/exporters/prometheus/testdata/histogram.txt index 547599cf6d5..00b5b803c28 100644 --- a/exporters/prometheus/testdata/histogram.txt +++ b/exporters/prometheus/testdata/histogram.txt @@ -9,7 +9,12 @@ baz_bucket{A="B",C="D",le="75"} 0 baz_bucket{A="B",C="D",le="100"} 0 baz_bucket{A="B",C="D",le="250"} 2 baz_bucket{A="B",C="D",le="500"} 0 +baz_bucket{A="B",C="D",le="750"} 0 baz_bucket{A="B",C="D",le="1000"} 0 +baz_bucket{A="B",C="D",le="2500"} 0 +baz_bucket{A="B",C="D",le="5000"} 0 +baz_bucket{A="B",C="D",le="7500"} 0 +baz_bucket{A="B",C="D",le="10000"} 0 baz_bucket{A="B",C="D",le="+Inf"} 4 baz_sum{A="B",C="D"} 236 baz_count{A="B",C="D"} 4 diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 7d6923fdd58..a67aa507d06 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -333,8 +333,8 @@ func TestMeterCreatesInstruments(t *testing.T) { { Attributes: attribute.Set{}, Count: 1, - Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000}, - BucketCounts: []uint64{0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0}, + Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, + BucketCounts: []uint64{0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, Min: &seven, Max: &seven, Sum: 7.0, @@ -397,8 +397,8 @@ func TestMeterCreatesInstruments(t *testing.T) { { Attributes: attribute.Set{}, Count: 1, - Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000}, - BucketCounts: []uint64{0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0}, + Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, + BucketCounts: []uint64{0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, Min: &seven, Max: &seven, Sum: 7.0, diff --git a/sdk/metric/reader.go b/sdk/metric/reader.go index bf596c27887..e5a1282db6b 100644 --- a/sdk/metric/reader.go +++ b/sdk/metric/reader.go @@ -166,7 +166,7 @@ func DefaultAggregationSelector(ik view.InstrumentKind) aggregation.Aggregation return aggregation.LastValue{} case view.SyncHistogram: return aggregation.ExplicitBucketHistogram{ - Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000}, + Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, NoMinMax: false, } } From b3fa7d4cd881d555452e1fe403a4843377a67664 Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Thu, 22 Sep 2022 11:48:32 +0200 Subject: [PATCH 2/3] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf26a4ff104..cb99127ad4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Flush pending measurements with the `PeriodicReader` in the `go.opentelemetry.io/otel/sdk/metric` when `ForceFlush` or `Shutdown` are called. (#3220) - Upgrade `golang.org/x/sys/unix` from `v0.0.0-20210423185535-09eb48e85fd7` to `v0.0.0-20220919091848-fb04ddd9f9c8`. This addresses [GO-2022-0493](https://pkg.go.dev/vuln/GO-2022-0493). (#3235) +- Update histogram default bounds to match the requirements of the latest specification. (#3222) ## [0.32.1] Metric SDK (Alpha) - 2022-09-22 From bccaa84732df700937cf163086a5586b5e5529a9 Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Tue, 27 Sep 2022 12:22:39 +0200 Subject: [PATCH 3/3] test custom boundaries with valid histogram --- exporters/prometheus/exporter_test.go | 19 ++++++++-- exporters/prometheus/testdata/histogram.txt | 35 ++++++++----------- .../prometheus/testdata/sanitized_names.txt | 5 +++ 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go index 6d46c3be0db..55db838dcd2 100644 --- a/exporters/prometheus/exporter_test.go +++ b/exporters/prometheus/exporter_test.go @@ -27,6 +27,8 @@ import ( otelmetric "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/instrument" "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/aggregation" + "go.opentelemetry.io/otel/sdk/metric/view" ) func TestPrometheusExporter(t *testing.T) { @@ -74,7 +76,7 @@ func TestPrometheusExporter(t *testing.T) { attribute.Key("A").String("B"), attribute.Key("C").String("D"), } - histogram, err := meter.SyncFloat64().Histogram("baz", instrument.WithDescription("a very nice histogram")) + histogram, err := meter.SyncFloat64().Histogram("histogram_baz", instrument.WithDescription("a very nice histogram")) require.NoError(t, err) histogram.Record(ctx, 23, attrs...) histogram.Record(ctx, 7, attrs...) @@ -137,11 +139,22 @@ func TestPrometheusExporter(t *testing.T) { ctx := context.Background() exporter := New() - provider := metric.NewMeterProvider(metric.WithReader(exporter)) + + customBucketsView, err := view.New( + view.MatchInstrumentName("histogram_*"), + view.WithSetAggregation(aggregation.ExplicitBucketHistogram{ + Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000}, + }), + ) + require.NoError(t, err) + defaultView, err := view.New(view.MatchInstrumentName("*")) + require.NoError(t, err) + + provider := metric.NewMeterProvider(metric.WithReader(exporter, customBucketsView, defaultView)) meter := provider.Meter("testmeter") registry := prometheus.NewRegistry() - err := registry.Register(exporter.Collector) + err = registry.Register(exporter.Collector) require.NoError(t, err) tc.recordMetrics(ctx, meter) diff --git a/exporters/prometheus/testdata/histogram.txt b/exporters/prometheus/testdata/histogram.txt index 00b5b803c28..3a8422bb573 100644 --- a/exporters/prometheus/testdata/histogram.txt +++ b/exporters/prometheus/testdata/histogram.txt @@ -1,20 +1,15 @@ -# HELP baz a very nice histogram -# TYPE baz histogram -baz_bucket{A="B",C="D",le="0"} 0 -baz_bucket{A="B",C="D",le="5"} 0 -baz_bucket{A="B",C="D",le="10"} 1 -baz_bucket{A="B",C="D",le="25"} 1 -baz_bucket{A="B",C="D",le="50"} 0 -baz_bucket{A="B",C="D",le="75"} 0 -baz_bucket{A="B",C="D",le="100"} 0 -baz_bucket{A="B",C="D",le="250"} 2 -baz_bucket{A="B",C="D",le="500"} 0 -baz_bucket{A="B",C="D",le="750"} 0 -baz_bucket{A="B",C="D",le="1000"} 0 -baz_bucket{A="B",C="D",le="2500"} 0 -baz_bucket{A="B",C="D",le="5000"} 0 -baz_bucket{A="B",C="D",le="7500"} 0 -baz_bucket{A="B",C="D",le="10000"} 0 -baz_bucket{A="B",C="D",le="+Inf"} 4 -baz_sum{A="B",C="D"} 236 -baz_count{A="B",C="D"} 4 +# HELP histogram_baz a very nice histogram +# TYPE histogram_baz histogram +histogram_baz_bucket{A="B",C="D",le="0"} 0 +histogram_baz_bucket{A="B",C="D",le="5"} 0 +histogram_baz_bucket{A="B",C="D",le="10"} 1 +histogram_baz_bucket{A="B",C="D",le="25"} 1 +histogram_baz_bucket{A="B",C="D",le="50"} 0 +histogram_baz_bucket{A="B",C="D",le="75"} 0 +histogram_baz_bucket{A="B",C="D",le="100"} 0 +histogram_baz_bucket{A="B",C="D",le="250"} 2 +histogram_baz_bucket{A="B",C="D",le="500"} 0 +histogram_baz_bucket{A="B",C="D",le="1000"} 0 +histogram_baz_bucket{A="B",C="D",le="+Inf"} 4 +histogram_baz_sum{A="B",C="D"} 236 +histogram_baz_count{A="B",C="D"} 4 diff --git a/exporters/prometheus/testdata/sanitized_names.txt b/exporters/prometheus/testdata/sanitized_names.txt index 5e9516716ae..0158d17aeb6 100644 --- a/exporters/prometheus/testdata/sanitized_names.txt +++ b/exporters/prometheus/testdata/sanitized_names.txt @@ -18,7 +18,12 @@ invalid_hist_name_bucket{A="B",C="D",le="75"} 0 invalid_hist_name_bucket{A="B",C="D",le="100"} 0 invalid_hist_name_bucket{A="B",C="D",le="250"} 0 invalid_hist_name_bucket{A="B",C="D",le="500"} 0 +invalid_hist_name_bucket{A="B",C="D",le="750"} 0 invalid_hist_name_bucket{A="B",C="D",le="1000"} 0 +invalid_hist_name_bucket{A="B",C="D",le="2500"} 0 +invalid_hist_name_bucket{A="B",C="D",le="5000"} 0 +invalid_hist_name_bucket{A="B",C="D",le="7500"} 0 +invalid_hist_name_bucket{A="B",C="D",le="10000"} 0 invalid_hist_name_bucket{A="B",C="D",le="+Inf"} 1 invalid_hist_name_sum{A="B",C="D"} 23 invalid_hist_name_count{A="B",C="D"} 1