From 808b49435e48b6ee1275782270e5971d699fc830 Mon Sep 17 00:00:00 2001 From: Sebastian Schoder Moreno Date: Tue, 4 Oct 2022 00:28:38 +0200 Subject: [PATCH 1/8] Update default buckets for Explicit Bucket Histogram from spec --- docs/metrics/customizing-the-sdk/README.md | 3 +-- src/OpenTelemetry/Metrics/Metric.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md index eeeb64ef819..c8b3c5cf582 100644 --- a/docs/metrics/customizing-the-sdk/README.md +++ b/docs/metrics/customizing-the-sdk/README.md @@ -235,8 +235,7 @@ with the metric are of interest to you. #### Specify custom boundaries for Histogram By default, the boundaries used for a Histogram are [`{ 0, 5, 10, 25, 50, 75, -100, 250, 500, -1000}`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#explicit-bucket-histogram-aggregation). +100, 250, 500,750, 1000, 2500, 5000, 7500, 10000}`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#explicit-bucket-histogram-aggregation). Views can be used to provide custom boundaries for a Histogram. The measurements are then aggregated using the custom boundaries provided instead of the the default boundaries. This requires the use of diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 51e8df9b7ac..c40e0f82ac7 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -25,7 +25,7 @@ namespace OpenTelemetry.Metrics /// public sealed class Metric { - internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 1000 }; + internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 }; private readonly AggregatorStore aggStore; From 325e4ba31137c029dfde2e66b1a9bf41d433e791 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Wed, 5 Oct 2022 11:31:40 -0400 Subject: [PATCH 2/8] Update docs/metrics/customizing-the-sdk/README.md --- docs/metrics/customizing-the-sdk/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md index c8b3c5cf582..607c4568daa 100644 --- a/docs/metrics/customizing-the-sdk/README.md +++ b/docs/metrics/customizing-the-sdk/README.md @@ -235,7 +235,7 @@ with the metric are of interest to you. #### Specify custom boundaries for Histogram By default, the boundaries used for a Histogram are [`{ 0, 5, 10, 25, 50, 75, -100, 250, 500,750, 1000, 2500, 5000, 7500, 10000}`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#explicit-bucket-histogram-aggregation). +100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#explicit-bucket-histogram-aggregation). Views can be used to provide custom boundaries for a Histogram. The measurements are then aggregated using the custom boundaries provided instead of the the default boundaries. This requires the use of From 5c25f64bb0f371899d33f08f99af77ed8273f299 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Wed, 5 Oct 2022 11:32:08 -0400 Subject: [PATCH 3/8] Update docs/metrics/customizing-the-sdk/README.md --- docs/metrics/customizing-the-sdk/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md index 607c4568daa..fbf5253a3a0 100644 --- a/docs/metrics/customizing-the-sdk/README.md +++ b/docs/metrics/customizing-the-sdk/README.md @@ -235,7 +235,7 @@ with the metric are of interest to you. #### Specify custom boundaries for Histogram By default, the boundaries used for a Histogram are [`{ 0, 5, 10, 25, 50, 75, -100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#explicit-bucket-histogram-aggregation). +100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}`](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.14.0/specification/metrics/sdk.md#explicit-bucket-histogram-aggregation). Views can be used to provide custom boundaries for a Histogram. The measurements are then aggregated using the custom boundaries provided instead of the the default boundaries. This requires the use of From f133b0038fd21e7b5c5bf5cc7adb08fbd1ac9520 Mon Sep 17 00:00:00 2001 From: Sebastian Schoder Moreno Date: Wed, 5 Oct 2022 23:22:47 +0200 Subject: [PATCH 4/8] Update changelog --- src/OpenTelemetry/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 58afdc44a34..fb4c54cce14 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -1,6 +1,8 @@ # Changelog ## Unreleased +* Updated default buckets for Explicit Bucket Histogram to align with the latest spec. + ([#3722](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3722)) ## 1.4.0-beta.1 From c03437bc4f56e5e38c1604d21df91ef2a24cdac2 Mon Sep 17 00:00:00 2001 From: Sebastian Schoder Moreno Date: Sun, 9 Oct 2022 21:01:25 +0200 Subject: [PATCH 5/8] Update changelog --- src/OpenTelemetry/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 93f1782cc9b..cef8b0ea973 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -1,7 +1,10 @@ # Changelog ## Unreleased -* Updated default buckets for Explicit Bucket Histogram to align with the latest spec. + +* Changed default bucket boundaries for Explicit Bucket Histogram from [0, 5, +10, 25, 50, 75, 100, 250, 500, 1000] to [0, 5, 10, 25, 50, 75, 100, 250, 500, +750, 1000, 2500, 5000, 7500, 10000]. ([#3722](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3722)) * Fixed an issue where `LogRecord.ForEachScope` may return scopes from a From cd9dbe5abdf831f36d8360391637ea0507385e91 Mon Sep 17 00:00:00 2001 From: Sebastian Schoder Moreno Date: Sun, 9 Oct 2022 21:01:40 +0200 Subject: [PATCH 6/8] Fix broken tests --- .../PrometheusSerializerTests.cs | 25 +++++++++++++++++++ .../Metrics/AggregatorTest.cs | 14 +++++++++-- .../Metrics/MetricAPITest.cs | 12 ++++----- .../Metrics/MetricViewTests.cs | 2 +- 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusSerializerTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusSerializerTests.cs index b9c8b3f0d3b..f6e91cbab50 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusSerializerTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusSerializerTests.cs @@ -273,7 +273,12 @@ public void HistogramZeroDimension() + "test_histogram_bucket{le='100'} 2 \\d+\n" + "test_histogram_bucket{le='250'} 2 \\d+\n" + "test_histogram_bucket{le='500'} 2 \\d+\n" + + "test_histogram_bucket{le='750'} 2 \\d+\n" + "test_histogram_bucket{le='1000'} 2 \\d+\n" + + "test_histogram_bucket{le='2500'} 2 \\d+\n" + + "test_histogram_bucket{le='5000'} 2 \\d+\n" + + "test_histogram_bucket{le='7500'} 2 \\d+\n" + + "test_histogram_bucket{le='10000'} 2 \\d+\n" + "test_histogram_bucket{le='\\+Inf'} 2 \\d+\n" + "test_histogram_sum 118 \\d+\n" + "test_histogram_count 2 \\d+\n" @@ -312,7 +317,12 @@ public void HistogramOneDimension() + "test_histogram_bucket{x='1',le='100'} 2 \\d+\n" + "test_histogram_bucket{x='1',le='250'} 2 \\d+\n" + "test_histogram_bucket{x='1',le='500'} 2 \\d+\n" + + "test_histogram_bucket{x='1',le='750'} 2 \\d+\n" + "test_histogram_bucket{x='1',le='1000'} 2 \\d+\n" + + "test_histogram_bucket{x='1',le='2500'} 2 \\d+\n" + + "test_histogram_bucket{x='1',le='5000'} 2 \\d+\n" + + "test_histogram_bucket{x='1',le='7500'} 2 \\d+\n" + + "test_histogram_bucket{x='1',le='10000'} 2 \\d+\n" + "test_histogram_bucket{x='1',le='\\+Inf'} 2 \\d+\n" + "test_histogram_sum{x='1'} 118 \\d+\n" + "test_histogram_count{x='1'} 2 \\d+\n" @@ -351,7 +361,12 @@ public void HistogramTwoDimensions() + "test_histogram_bucket{x='1',y='2',le='100'} 2 \\d+\n" + "test_histogram_bucket{x='1',y='2',le='250'} 2 \\d+\n" + "test_histogram_bucket{x='1',y='2',le='500'} 2 \\d+\n" + + "test_histogram_bucket{x='1',y='2',le='750'} 2 \\d+\n" + "test_histogram_bucket{x='1',y='2',le='1000'} 2 \\d+\n" + + "test_histogram_bucket{x='1',y='2',le='2500'} 2 \\d+\n" + + "test_histogram_bucket{x='1',y='2',le='5000'} 2 \\d+\n" + + "test_histogram_bucket{x='1',y='2',le='7500'} 2 \\d+\n" + + "test_histogram_bucket{x='1',y='2',le='10000'} 2 \\d+\n" + "test_histogram_bucket{x='1',y='2',le='\\+Inf'} 2 \\d+\n" + "test_histogram_sum{x='1',y='2'} 118 \\d+\n" + "test_histogram_count{x='1',y='2'} 2 \\d+\n" @@ -391,7 +406,12 @@ public void HistogramInfinities() + "test_histogram_bucket{le='100'} 1 \\d+\n" + "test_histogram_bucket{le='250'} 1 \\d+\n" + "test_histogram_bucket{le='500'} 1 \\d+\n" + + "test_histogram_bucket{le='750'} 1 \\d+\n" + "test_histogram_bucket{le='1000'} 1 \\d+\n" + + "test_histogram_bucket{le='2500'} 1 \\d+\n" + + "test_histogram_bucket{le='5000'} 1 \\d+\n" + + "test_histogram_bucket{le='7500'} 1 \\d+\n" + + "test_histogram_bucket{le='10000'} 1 \\d+\n" + "test_histogram_bucket{le='\\+Inf'} 3 \\d+\n" + "test_histogram_sum \\+Inf \\d+\n" + "test_histogram_count 3 \\d+\n" @@ -431,7 +451,12 @@ public void HistogramNaN() + "test_histogram_bucket{le='100'} 1 \\d+\n" + "test_histogram_bucket{le='250'} 1 \\d+\n" + "test_histogram_bucket{le='500'} 1 \\d+\n" + + "test_histogram_bucket{le='750'} 1 \\d+\n" + "test_histogram_bucket{le='1000'} 1 \\d+\n" + + "test_histogram_bucket{le='2500'} 1 \\d+\n" + + "test_histogram_bucket{le='5000'} 1 \\d+\n" + + "test_histogram_bucket{le='7500'} 1 \\d+\n" + + "test_histogram_bucket{le='10000'} 1 \\d+\n" + "test_histogram_bucket{le='\\+Inf'} 3 \\d+\n" + "test_histogram_sum Nan \\d+\n" + "test_histogram_count 3 \\d+\n" diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs index 0b598d4fa34..4876c9262b2 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs @@ -45,15 +45,25 @@ public void HistogramDistributeToAllBucketsDefault() histogramPoint.Update(250); histogramPoint.Update(499); histogramPoint.Update(500); - histogramPoint.Update(999); + histogramPoint.Update(501); + histogramPoint.Update(750); + histogramPoint.Update(751); histogramPoint.Update(1000); histogramPoint.Update(1001); + histogramPoint.Update(2500); + histogramPoint.Update(2501); + histogramPoint.Update(5000); + histogramPoint.Update(5001); + histogramPoint.Update(7500); + histogramPoint.Update(7501); + histogramPoint.Update(10000); + histogramPoint.Update(10001); histogramPoint.Update(10000000); histogramPoint.TakeSnapshot(true); var count = histogramPoint.GetHistogramCount(); - Assert.Equal(22, count); + Assert.Equal(32, count); int actualCount = 0; foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets()) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 83d64bd7932..33493edcb40 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -1404,14 +1404,14 @@ public void MultithreadedDoubleCounterTest() [Fact] public void MultithreadedLongHistogramTest() { - var expected = new long[11]; + var expected = new long[16]; for (var i = 0; i < expected.Length; i++) { expected[i] = NumberOfThreads * NumberOfMetricUpdateByEachThread; } - // Metric.DefaultHistogramBounds: 0, 5, 10, 25, 50, 75, 100, 250, 500, 1000 - var values = new long[] { -1, 1, 6, 20, 40, 60, 80, 200, 300, 600, 1001 }; + // Metric.DefaultHistogramBounds: 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 + var values = new long[] { -1, 1, 6, 20, 40, 60, 80, 200, 300, 600, 800, 1001, 3000, 6000, 8000, 10001 }; this.MultithreadedHistogramTest(expected, values); } @@ -1419,14 +1419,14 @@ public void MultithreadedLongHistogramTest() [Fact] public void MultithreadedDoubleHistogramTest() { - var expected = new long[11]; + var expected = new long[16]; for (var i = 0; i < expected.Length; i++) { expected[i] = NumberOfThreads * NumberOfMetricUpdateByEachThread; } - // Metric.DefaultHistogramBounds: 0, 5, 10, 25, 50, 75, 100, 250, 500, 1000 - var values = new double[] { -1.0, 1.0, 6.0, 20.0, 40.0, 60.0, 80.0, 200.0, 300.0, 600.0, 1001.0 }; + // Metric.DefaultHistogramBounds: 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 + var values = new double[] { -1.0, 1.0, 6.0, 20.0, 40.0, 60.0, 80.0, 200.0, 300.0, 600.0, 800.0, 1001.0, 3000.0, 6000.0, 8000.0, 10001.0 }; this.MultithreadedHistogramTest(expected, values); } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 3a12cd4ac05..125290c2b4c 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -534,7 +534,7 @@ public void ViewToProduceCustomHistogramBound() int index = 0; int actualCount = 0; - var expectedBucketCounts = new long[] { 2, 1, 2, 2, 0, 0, 0, 0, 0, 0, 0 }; + var expectedBucketCounts = new long[] { 2, 1, 2, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets()) { Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); From b3102d0af6a6e32e9e25be12b981bcba483d53be Mon Sep 17 00:00:00 2001 From: Sebastian Schoder Moreno Date: Sun, 9 Oct 2022 22:51:02 +0200 Subject: [PATCH 7/8] Fix format in MetricAPITest.cs --- test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 33493edcb40..342fffe7107 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -1426,7 +1426,7 @@ public void MultithreadedDoubleHistogramTest() } // Metric.DefaultHistogramBounds: 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 - var values = new double[] { -1.0, 1.0, 6.0, 20.0, 40.0, 60.0, 80.0, 200.0, 300.0, 600.0, 800.0, 1001.0, 3000.0, 6000.0, 8000.0, 10001.0 }; + var values = new double[] { -1.0, 1.0, 6.0, 20.0, 40.0, 60.0, 80.0, 200.0, 300.0, 600.0, 800.0, 1001.0, 3000.0, 6000.0, 8000.0, 10001.0 }; this.MultithreadedHistogramTest(expected, values); } From 12a9a9dc0013838aa94c34171492763a5c28e533 Mon Sep 17 00:00:00 2001 From: Sebastian Schoder Moreno Date: Sun, 9 Oct 2022 23:16:11 +0200 Subject: [PATCH 8/8] Fix CHANGELOG.md formatting --- src/OpenTelemetry/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index cef8b0ea973..a189a19f0c5 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -3,8 +3,8 @@ ## Unreleased * Changed default bucket boundaries for Explicit Bucket Histogram from [0, 5, -10, 25, 50, 75, 100, 250, 500, 1000] to [0, 5, 10, 25, 50, 75, 100, 250, 500, -750, 1000, 2500, 5000, 7500, 10000]. + 10, 25, 50, 75, 100, 250, 500, 1000] to [0, 5, 10, 25, 50, 75, 100, 250, 500, + 750, 1000, 2500, 5000, 7500, 10000]. ([#3722](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3722)) * Fixed an issue where `LogRecord.ForEachScope` may return scopes from a