From 30e405431b2652f5496b5e82bddeb55e6e82fe57 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 15 Mar 2023 13:20:40 -0700 Subject: [PATCH] Add exponential histogram tests and in-memory exporter support (#4303) --- .../Base2ExponentialBucketHistogram.cs | 11 +++ ...ExponentialBucketHistogramConfiguration.cs | 24 ++++- .../Metrics/ExponentialHistogramBuckets.cs | 21 ++-- .../Metrics/ExponentialHistogramData.cs | 14 ++- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 2 +- .../Metrics/MetricPointOptionalComponents.cs | 4 +- .../Metrics/AggregatorTest.cs | 59 +++++++---- .../Metrics/MetricSnapshotTests.cs | 99 +++++++++++++++++++ .../Metrics/MetricTestData.cs | 2 + .../Metrics/MetricViewTests.cs | 59 ++++++++++- 10 files changed, 258 insertions(+), 37 deletions(-) diff --git a/src/OpenTelemetry/Metrics/Base2ExponentialBucketHistogram.cs b/src/OpenTelemetry/Metrics/Base2ExponentialBucketHistogram.cs index 2f90e9b460e..4df0d91fc6d 100644 --- a/src/OpenTelemetry/Metrics/Base2ExponentialBucketHistogram.cs +++ b/src/OpenTelemetry/Metrics/Base2ExponentialBucketHistogram.cs @@ -235,4 +235,15 @@ internal ExponentialHistogramData GetExponentialHistogramData() { return this.SnapshotExponentialHistogramData; } + + internal Base2ExponentialBucketHistogram Copy() + { + Debug.Assert(this.PositiveBuckets.Capacity == this.NegativeBuckets.Capacity, "Capacity of positive and negative buckets are not equal."); + var copy = new Base2ExponentialBucketHistogram(this.PositiveBuckets.Capacity, this.SnapshotExponentialHistogramData.Scale); + copy.SnapshotSum = this.SnapshotSum; + copy.SnapshotMin = this.SnapshotMin; + copy.SnapshotMax = this.SnapshotMax; + copy.SnapshotExponentialHistogramData = this.SnapshotExponentialHistogramData.Copy(); + return copy; + } } diff --git a/src/OpenTelemetry/Metrics/Base2ExponentialBucketHistogramConfiguration.cs b/src/OpenTelemetry/Metrics/Base2ExponentialBucketHistogramConfiguration.cs index dd12dd783a5..f6f7db9df19 100644 --- a/src/OpenTelemetry/Metrics/Base2ExponentialBucketHistogramConfiguration.cs +++ b/src/OpenTelemetry/Metrics/Base2ExponentialBucketHistogramConfiguration.cs @@ -19,13 +19,31 @@ namespace OpenTelemetry.Metrics; /// /// Stores configuration for a histogram metric stream with base-2 exponential bucket boundaries. /// -internal sealed class Base2ExponentialBucketHistogramConfiguration : MetricStreamConfiguration +internal sealed class Base2ExponentialBucketHistogramConfiguration : HistogramConfiguration { + private int maxSize = 160; + /// /// Gets or sets the maximum number of buckets in each of the positive and negative ranges, not counting the special zero bucket. /// /// - /// The default value is 160. + /// The default value is 160. The minimum size is 2. /// - public int MaxSize { get; set; } = 160; + public int MaxSize + { + get + { + return this.maxSize; + } + + set + { + if (value < 2) + { + throw new ArgumentException($"Histogram max size is invalid. Minimum size is 2.", nameof(value)); + } + + this.maxSize = value; + } + } } diff --git a/src/OpenTelemetry/Metrics/ExponentialHistogramBuckets.cs b/src/OpenTelemetry/Metrics/ExponentialHistogramBuckets.cs index 83e0fe605c8..44f628fe1c7 100644 --- a/src/OpenTelemetry/Metrics/ExponentialHistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/ExponentialHistogramBuckets.cs @@ -20,7 +20,6 @@ namespace OpenTelemetry.Metrics; public sealed class ExponentialHistogramBuckets { - private int size; private long[] buckets = Array.Empty(); internal ExponentialHistogramBuckets() @@ -29,7 +28,7 @@ internal ExponentialHistogramBuckets() public int Offset { get; private set; } - public Enumerator GetEnumerator() => new(this.size, this.buckets); + public Enumerator GetEnumerator() => new(this.buckets); internal void SnapshotBuckets(CircularBufferBuckets buckets) { @@ -39,24 +38,30 @@ internal void SnapshotBuckets(CircularBufferBuckets buckets) } this.Offset = buckets.Offset; - this.size = buckets.Size; buckets.Copy(this.buckets); } + internal ExponentialHistogramBuckets Copy() + { + var copy = new ExponentialHistogramBuckets(); + copy.Offset = this.Offset; + copy.buckets = new long[this.buckets.Length]; + Array.Copy(this.buckets, copy.buckets, this.buckets.Length); + return copy; + } + /// /// Enumerates the bucket counts of an exponential histogram. /// // Note: Does not implement IEnumerator<> to prevent accidental boxing. public struct Enumerator { - private readonly int size; private readonly long[] buckets; private int index; - internal Enumerator(int size, long[] buckets) + internal Enumerator(long[] buckets) { - this.index = size; - this.size = size; + this.index = 0; this.buckets = buckets; this.Current = default; } @@ -76,7 +81,7 @@ internal Enumerator(int size, long[] buckets) /// collection. public bool MoveNext() { - if (this.index < this.size) + if (this.index < this.buckets.Length) { this.Current = this.buckets[this.index++]; return true; diff --git a/src/OpenTelemetry/Metrics/ExponentialHistogramData.cs b/src/OpenTelemetry/Metrics/ExponentialHistogramData.cs index d24402d2079..6fe8d145775 100644 --- a/src/OpenTelemetry/Metrics/ExponentialHistogramData.cs +++ b/src/OpenTelemetry/Metrics/ExponentialHistogramData.cs @@ -30,7 +30,17 @@ internal ExponentialHistogramData() public long ZeroCount { get; internal set; } - public ExponentialHistogramBuckets PositiveBuckets { get; } + public ExponentialHistogramBuckets PositiveBuckets { get; private set; } - public ExponentialHistogramBuckets NegativeBuckets { get; } + public ExponentialHistogramBuckets NegativeBuckets { get; private set; } + + internal ExponentialHistogramData Copy() + { + var copy = new ExponentialHistogramData(); + copy.Scale = this.Scale; + copy.ZeroCount = this.ZeroCount; + copy.PositiveBuckets = this.PositiveBuckets.Copy(); + copy.NegativeBuckets = this.NegativeBuckets.Copy(); + return copy; + } } diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index cffcf80a436..7db97eb3a0d 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -193,7 +193,7 @@ internal MeterProviderSdk( metricStreamConfig.ViewId = i; } - if (metricStreamConfig is ExplicitBucketHistogramConfiguration + if (metricStreamConfig is HistogramConfiguration && instrument.GetType().GetGenericTypeDefinition() != typeof(Histogram<>)) { metricStreamConfig = null; diff --git a/src/OpenTelemetry/Metrics/MetricPointOptionalComponents.cs b/src/OpenTelemetry/Metrics/MetricPointOptionalComponents.cs index 48251fa2427..c9ec98ebbd7 100644 --- a/src/OpenTelemetry/Metrics/MetricPointOptionalComponents.cs +++ b/src/OpenTelemetry/Metrics/MetricPointOptionalComponents.cs @@ -38,13 +38,13 @@ internal sealed class MetricPointOptionalComponents internal MetricPointOptionalComponents Copy() { MetricPointOptionalComponents copy = new MetricPointOptionalComponents(); - copy.HistogramBuckets = this.HistogramBuckets.Copy(); + copy.HistogramBuckets = this.HistogramBuckets?.Copy(); + copy.Base2ExponentialBucketHistogram = this.Base2ExponentialBucketHistogram?.Copy(); if (this.Exemplars != null) { Array.Copy(this.Exemplars, copy.Exemplars, this.Exemplars.Length); } - // TODO: Copy Base2ExponentialBucketHistogram return copy; } } diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs index 7983465aa9b..3fb3d1ee62e 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs @@ -188,6 +188,45 @@ public void HistogramWithOnlySumCount() Assert.False(enumerator.MoveNext()); } + internal static void AssertExponentialBucketsAreCorrect(Base2ExponentialBucketHistogram expectedHistogram, ExponentialHistogramData data) + { + Assert.Equal(expectedHistogram.Scale, data.Scale); + Assert.Equal(expectedHistogram.ZeroCount, data.ZeroCount); + Assert.Equal(expectedHistogram.PositiveBuckets.Offset, data.PositiveBuckets.Offset); + Assert.Equal(expectedHistogram.NegativeBuckets.Offset, data.NegativeBuckets.Offset); + + expectedHistogram.Snapshot(); + var expectedData = expectedHistogram.GetExponentialHistogramData(); + + var actual = new List(); + foreach (var bucketCount in data.PositiveBuckets) + { + actual.Add(bucketCount); + } + + var expected = new List(); + foreach (var bucketCount in expectedData.PositiveBuckets) + { + expected.Add(bucketCount); + } + + Assert.Equal(expected, actual); + + actual = new List(); + foreach (var bucketCount in data.NegativeBuckets) + { + actual.Add(bucketCount); + } + + expected = new List(); + foreach (var bucketCount in expectedData.NegativeBuckets) + { + expected.Add(bucketCount); + } + + Assert.Equal(expected, actual); + } + [Theory] [InlineData(AggregationType.Base2ExponentialHistogram, AggregationTemporality.Cumulative)] [InlineData(AggregationType.Base2ExponentialHistogram, AggregationTemporality.Delta)] @@ -284,25 +323,5 @@ internal void ExponentialHistogramTests(AggregationType aggregationType, Aggrega } } } - - private static void AssertExponentialBucketsAreCorrect(Base2ExponentialBucketHistogram expectedHistogram, ExponentialHistogramData data) - { - Assert.Equal(expectedHistogram.Scale, data.Scale); - Assert.Equal(expectedHistogram.ZeroCount, data.ZeroCount); - Assert.Equal(expectedHistogram.PositiveBuckets.Offset, data.PositiveBuckets.Offset); - Assert.Equal(expectedHistogram.NegativeBuckets.Offset, data.NegativeBuckets.Offset); - - var index = expectedHistogram.PositiveBuckets.Offset; - foreach (var bucketCount in data.PositiveBuckets) - { - Assert.Equal(expectedHistogram.PositiveBuckets[index++], bucketCount); - } - - index = expectedHistogram.NegativeBuckets.Offset; - foreach (var bucketCount in data.NegativeBuckets) - { - Assert.Equal(expectedHistogram.PositiveBuckets[index++], bucketCount); - } - } } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTests.cs index 9ea34769685..7c9f4ad58af 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTests.cs @@ -163,5 +163,104 @@ public void VerifySnapshot_Histogram() Assert.Equal(2, snapshot2.MetricPoints[0].GetHistogramCount()); Assert.Equal(15, snapshot2.MetricPoints[0].GetHistogramSum()); } + + [Fact] + public void VerifySnapshot_ExponentialHistogram() + { + var expectedHistogram = new Base2ExponentialBucketHistogram(); + var exportedMetrics = new List(); + var exportedSnapshots = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + var histogram = meter.CreateHistogram("histogram"); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView("histogram", new Base2ExponentialBucketHistogramConfiguration()) + .AddInMemoryExporter(exportedMetrics) + .AddInMemoryExporter(exportedSnapshots) + .Build(); + + // FIRST EXPORT + expectedHistogram.Record(10); + histogram.Record(10); + meterProvider.ForceFlush(); + + // Verify Metric 1 + Assert.Single(exportedMetrics); + var metric1 = exportedMetrics[0]; + var metricPoints1Enumerator = metric1.GetMetricPoints().GetEnumerator(); + Assert.True(metricPoints1Enumerator.MoveNext()); + ref readonly var metricPoint1 = ref metricPoints1Enumerator.Current; + Assert.Equal(1, metricPoint1.GetHistogramCount()); + Assert.Equal(10, metricPoint1.GetHistogramSum()); + metricPoint1.TryGetHistogramMinMaxValues(out var min, out var max); + Assert.Equal(10, min); + Assert.Equal(10, max); + AggregatorTest.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint1.GetExponentialHistogramData()); + + // Verify Snapshot 1 + Assert.Single(exportedSnapshots); + var snapshot1 = exportedSnapshots[0]; + Assert.Single(snapshot1.MetricPoints); + Assert.Equal(1, snapshot1.MetricPoints[0].GetHistogramCount()); + Assert.Equal(10, snapshot1.MetricPoints[0].GetHistogramSum()); + snapshot1.MetricPoints[0].TryGetHistogramMinMaxValues(out min, out max); + Assert.Equal(10, min); + Assert.Equal(10, max); + AggregatorTest.AssertExponentialBucketsAreCorrect(expectedHistogram, snapshot1.MetricPoints[0].GetExponentialHistogramData()); + + // Verify Metric == Snapshot + Assert.Equal(metric1.Name, snapshot1.Name); + Assert.Equal(metric1.Description, snapshot1.Description); + Assert.Equal(metric1.Unit, snapshot1.Unit); + Assert.Equal(metric1.MeterName, snapshot1.MeterName); + Assert.Equal(metric1.MetricType, snapshot1.MetricType); + Assert.Equal(metric1.MeterVersion, snapshot1.MeterVersion); + + // SECOND EXPORT + expectedHistogram.Record(5); + histogram.Record(5); + meterProvider.ForceFlush(); + + // Verify Metric 1 after second export + // This value is expected to be updated. + Assert.Equal(2, metricPoint1.GetHistogramCount()); + Assert.Equal(15, metricPoint1.GetHistogramSum()); + metricPoint1.TryGetHistogramMinMaxValues(out min, out max); + Assert.Equal(5, min); + Assert.Equal(10, max); + + // Verify Metric 2 + Assert.Equal(2, exportedMetrics.Count); + var metric2 = exportedMetrics[1]; + var metricPoints2Enumerator = metric2.GetMetricPoints().GetEnumerator(); + Assert.True(metricPoints2Enumerator.MoveNext()); + ref readonly var metricPoint2 = ref metricPoints2Enumerator.Current; + Assert.Equal(2, metricPoint2.GetHistogramCount()); + Assert.Equal(15, metricPoint2.GetHistogramSum()); + metricPoint1.TryGetHistogramMinMaxValues(out min, out max); + Assert.Equal(5, min); + Assert.Equal(10, max); + AggregatorTest.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint2.GetExponentialHistogramData()); + + // Verify Snapshot 1 after second export + // This value is expected to be unchanged. + Assert.Equal(1, snapshot1.MetricPoints[0].GetHistogramCount()); + Assert.Equal(10, snapshot1.MetricPoints[0].GetHistogramSum()); + snapshot1.MetricPoints[0].TryGetHistogramMinMaxValues(out min, out max); + Assert.Equal(10, min); + Assert.Equal(10, max); + + // Verify Snapshot 2 + Assert.Equal(2, exportedSnapshots.Count); + var snapshot2 = exportedSnapshots[1]; + Assert.Single(snapshot2.MetricPoints); + Assert.Equal(2, snapshot2.MetricPoints[0].GetHistogramCount()); + Assert.Equal(15, snapshot2.MetricPoints[0].GetHistogramSum()); + snapshot2.MetricPoints[0].TryGetHistogramMinMaxValues(out min, out max); + Assert.Equal(5, min); + Assert.Equal(10, max); + AggregatorTest.AssertExponentialBucketsAreCorrect(expectedHistogram, snapshot2.MetricPoints[0].GetExponentialHistogramData()); + } } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs index 75e80032a68..ed5361eb71e 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs @@ -58,6 +58,7 @@ public static IEnumerable ValidHistogramMinMax new object[] { new double[] { double.NegativeInfinity, 0, double.PositiveInfinity }, new HistogramConfiguration(), double.NegativeInfinity, double.PositiveInfinity }, new object[] { new double[] { 1 }, new HistogramConfiguration(), 1, 1 }, new object[] { new double[] { 5, 100, 4, 101, -2, 97 }, new ExplicitBucketHistogramConfiguration() { Boundaries = new double[] { 10, 20 } }, -2, 101 }, + new object[] { new double[] { 5, 100, 4, 101, -2, 97 }, new Base2ExponentialBucketHistogramConfiguration(), -2, 101 }, }; public static IEnumerable InvalidHistogramMinMax @@ -65,6 +66,7 @@ public static IEnumerable InvalidHistogramMinMax { new object[] { new double[] { 1 }, new HistogramConfiguration() { RecordMinMax = false } }, new object[] { new double[] { 1 }, new ExplicitBucketHistogramConfiguration() { Boundaries = new double[] { 10, 20 }, RecordMinMax = false } }, + new object[] { new double[] { 1 }, new Base2ExponentialBucketHistogramConfiguration() { RecordMinMax = false } }, }; } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 28d661dec67..82ef72b1246 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -204,6 +204,18 @@ public void AddViewWithInvalidHistogramBoundsThrowsArgumentException(double[] bo Assert.Contains("Histogram boundaries must be in ascending order with distinct values", ex.Message); } + [Theory] + [InlineData(-1)] + [InlineData(0)] + [InlineData(1)] + public void AddViewWithInvalidExponentialHistogramConfigThrowsArgumentException(int maxSize) + { + var ex = Assert.Throws(() => Sdk.CreateMeterProviderBuilder() + .AddView("name1", new Base2ExponentialBucketHistogramConfiguration { MaxSize = maxSize })); + + Assert.Contains("Histogram max size is invalid", ex.Message); + } + [Theory] [MemberData(nameof(MetricTestData.InvalidHistogramBoundaries), MemberType = typeof(MetricTestData))] public void AddViewWithInvalidHistogramBoundsIgnored(double[] boundaries) @@ -461,7 +473,8 @@ public void ViewWithHistogramConfigurationIgnoredWhenAppliedToNonHistogram() var exportedItems = new List(); using var meterProvider = Sdk.CreateMeterProviderBuilder() .AddMeter(meter.Name) - .AddView("NotAHistogram", new ExplicitBucketHistogramConfiguration() { Name = "ImAHistogram" }) + .AddView("NotAHistogram", new ExplicitBucketHistogramConfiguration() { Name = "ImAnExplicitBoundsHistogram" }) + .AddView("NotAHistogram", new Base2ExponentialBucketHistogramConfiguration() { Name = "ImAnExponentialHistogram" }) .AddInMemoryExporter(exportedItems) .Build(); @@ -569,6 +582,50 @@ public void ViewToProduceCustomHistogramBound() Assert.Equal(boundaries.Length + 1, actualCount); } + [Fact] + public void ViewToProduceExponentialHistogram() + { + var valuesToRecord = new[] { -10, 0, 1, 9, 10, 11, 19 }; + + using var meter = new Meter(Utils.GetCurrentMethodName()); + var exportedItems = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView("MyHistogram", new Base2ExponentialBucketHistogramConfiguration()) + .AddInMemoryExporter(exportedItems) + .Build(); + + var histogram = meter.CreateHistogram("MyHistogram"); + var expectedHistogram = new Base2ExponentialBucketHistogram(); + foreach (var value in valuesToRecord) + { + histogram.Record(value); + expectedHistogram.Record(value); + } + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + Assert.Single(exportedItems); + var metric = exportedItems[0]; + + Assert.Equal("MyHistogram", metric.Name); + + var metricPoints = new List(); + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; + + var count = metricPoint.GetHistogramCount(); + var sum = metricPoint.GetHistogramSum(); + + AggregatorTest.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint.GetExponentialHistogramData()); + Assert.Equal(40, sum); + Assert.Equal(7, count); + } + [Theory] [MemberData(nameof(MetricTestData.ValidHistogramMinMax), MemberType = typeof(MetricTestData))] public void HistogramMinMax(double[] values, HistogramConfiguration histogramConfiguration, double expectedMin, double expectedMax)