From f5a7d3d20a3130d075acf2dab452f7521652943e Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Tue, 1 Aug 2023 21:08:44 -0700 Subject: [PATCH 01/10] Add support for metric overflow attribute --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 78 +++- ...ggregatorTest.cs => AggregatorTestBase.cs} | 59 ++- ...{MetricAPITest.cs => MetricApiTestBase.cs} | 385 ++++++++++++++---- .../Metrics/MetricOverflowAttributeTests.cs | 130 ++++++ ...hotTests.cs => MetricSnapshotTestsBase.cs} | 113 ++++- .../Metrics/MetricViewTests.cs | 2 +- 6 files changed, 661 insertions(+), 106 deletions(-) rename test/OpenTelemetry.Tests/Metrics/{AggregatorTest.cs => AggregatorTestBase.cs} (92%) rename test/OpenTelemetry.Tests/Metrics/{MetricAPITest.cs => MetricApiTestBase.cs} (85%) create mode 100644 test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs rename test/OpenTelemetry.Tests/Metrics/{MetricSnapshotTests.cs => MetricSnapshotTestsBase.cs} (80%) diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index b0c4f673114..a087e920008 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -24,6 +24,8 @@ internal sealed class AggregatorStore { private static readonly string MetricPointCapHitFixMessage = "Modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit."; private static readonly Comparison> DimensionComparisonDelegate = (x, y) => x.Key.CompareTo(y.Key); + private static bool emitOverflowAttribute; + private readonly object lockZeroTags = new(); private readonly HashSet tagKeysInteresting; private readonly int tagsKeysInterestingCount; @@ -44,6 +46,7 @@ internal sealed class AggregatorStore private readonly UpdateDoubleDelegate updateDoubleCallback; private readonly int maxMetricPoints; private readonly ExemplarFilter exemplarFilter; + private int metricPointIndex = 0; private int batchSize = 0; private int metricCapHitMessageLogged; @@ -81,6 +84,25 @@ internal AggregatorStore( this.tagKeysInteresting = hs; this.tagsKeysInterestingCount = hs.Count; } + + // If the switch is not set at all or if it's explicitly set to false + if (AppContext.TryGetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", out bool shouldEmitOverflowAttribute) == false || + shouldEmitOverflowAttribute == false) + { + emitOverflowAttribute = false; + } + else if (this.maxMetricPoints > 1) + { + // We need at least two metric points. One is reserved for zero tags and the other one for overflow attribute + + emitOverflowAttribute = true; + + // Setting this to as we would reserve the metricPoints[1] for overflow attribute. + // Newer attributes should be added starting at the index: 2 + this.metricPointIndex = 1; + + this.metricPoints[1] = new MetricPoint(this, this.aggType, new KeyValuePair[] { new("otel.metric.overflow", true) }, this.histogramBounds, this.exponentialHistogramMaxSize, this.exponentialHistogramMaxScale); + } } private delegate void UpdateLongDelegate(long value, ReadOnlySpan> tags); @@ -329,12 +351,20 @@ private void UpdateLong(long value, ReadOnlySpan> t var index = this.FindMetricAggregatorsDefault(tags); if (index < 0) { - if (Interlocked.CompareExchange(ref this.metricCapHitMessageLogged, 1, 0) == 0) + if (emitOverflowAttribute) { - OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, MetricPointCapHitFixMessage); + this.metricPoints[1].Update(value); + return; } + else + { + if (Interlocked.CompareExchange(ref this.metricCapHitMessageLogged, 1, 0) == 0) + { + OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, MetricPointCapHitFixMessage); + } - return; + return; + } } // TODO: can special case built-in filters to be bit faster. @@ -361,12 +391,20 @@ private void UpdateLongCustomTags(long value, ReadOnlySpan +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -19,13 +19,27 @@ namespace OpenTelemetry.Metrics.Tests; -public class AggregatorTest +#pragma warning disable SA1402 + +public abstract class AggregatorTestBase : IDisposable { private static readonly Meter Meter = new("testMeter"); private static readonly Instrument Instrument = Meter.CreateHistogram("testInstrument"); private static readonly ExplicitBucketHistogramConfiguration HistogramConfiguration = new() { Boundaries = Metric.DefaultHistogramBounds }; private static readonly MetricStreamIdentity MetricStreamIdentity = new(Instrument, HistogramConfiguration); + private readonly AggregatorStore aggregatorStore = new(MetricStreamIdentity, AggregationType.HistogramWithBuckets, AggregationTemporality.Cumulative, 1024); + private readonly bool emitOverflowAttribute; + + protected AggregatorTestBase(bool emitOverflowAttribute) + { + this.emitOverflowAttribute = emitOverflowAttribute; + + if (emitOverflowAttribute) + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + } + } [Fact] public void HistogramDistributeToAllBucketsDefault() @@ -224,6 +238,11 @@ public void MultiThreadedHistogramUpdateAndSnapShotTest() Assert.Equal(200, argsToThread.SumOfDelta + lastDelta); } + public void Dispose() + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + } + internal static void AssertExponentialBucketsAreCorrect(Base2ExponentialBucketHistogram expectedHistogram, ExponentialHistogramData data) { Assert.Equal(expectedHistogram.Scale, data.Scale); @@ -307,7 +326,15 @@ internal void ExponentialHistogramTests(AggregationType aggregationType, Aggrega metricPoints.Add(mp); } - Assert.Single(metricPoints); + if (this.emitOverflowAttribute && aggregationTemporality == AggregationTemporality.Cumulative) + { + Assert.Equal(2, metricPoints.Count); + } + else + { + Assert.Single(metricPoints); + } + var metricPoint = metricPoints[0]; var count = metricPoint.GetHistogramCount(); @@ -404,7 +431,15 @@ internal void ExponentialMaxScaleConfigWorks(int? maxScale) metricPoints.Add(mp); } - Assert.Single(metricPoints); + if (this.emitOverflowAttribute) + { + Assert.Equal(2, metricPoints.Count); + } + else + { + Assert.Single(metricPoints); + } + var metricPoint = metricPoints[0]; // After a single measurement there will not have been a scale down. @@ -463,3 +498,19 @@ private class ThreadArguments public double SumOfDelta; } } + +public class AggregatorTests : AggregatorTestBase +{ + public AggregatorTests() + : base(false) + { + } +} + +public class AggregatorTestsWithOverflowAttribute : AggregatorTestBase +{ + public AggregatorTestsWithOverflowAttribute() + : base(true) + { + } +} diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs similarity index 85% rename from test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs rename to test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs index 5f403456a05..c2d4ef0c778 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -16,6 +16,7 @@ using System.Diagnostics; using System.Diagnostics.Metrics; +using System.Drawing.Drawing2D; using OpenTelemetry.Exporter; using OpenTelemetry.Internal; using OpenTelemetry.Tests; @@ -24,7 +25,9 @@ namespace OpenTelemetry.Metrics.Tests; -public class MetricApiTest : MetricTestsBase +#pragma warning disable SA1402 + +public abstract class MetricApiTestBase : MetricTestsBase, IDisposable { private const int MaxTimeToAllowForFlush = 10000; private static readonly int NumberOfThreads = Environment.ProcessorCount; @@ -32,10 +35,17 @@ public class MetricApiTest : MetricTestsBase private static readonly double DeltaDoubleValueUpdatedByEachCall = 11.987; private static readonly int NumberOfMetricUpdateByEachThread = 100000; private readonly ITestOutputHelper output; + private readonly bool emitOverflowAttribute; - public MetricApiTest(ITestOutputHelper output) + protected MetricApiTestBase(ITestOutputHelper output, bool emitOverflowAttribute) { this.output = output; + this.emitOverflowAttribute = emitOverflowAttribute; + + if (emitOverflowAttribute) + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + } } [Fact] @@ -61,14 +71,36 @@ public void MeasurementWithNullValuedTag() metricPoints.Add(mp); } - Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; - Assert.Equal(100, metricPoint.GetSumLong()); - Assert.Equal(1, metricPoint.Tags.Count); - var tagEnumerator = metricPoint.Tags.GetEnumerator(); - tagEnumerator.MoveNext(); - Assert.Equal("tagWithNullValue", tagEnumerator.Current.Key); - Assert.Null(tagEnumerator.Current.Value); + if (this.emitOverflowAttribute) + { + // Cumulative metric export would always emit the metric overflow attribute first + Assert.Equal(2, metricPoints.Count); + + var metricPoint = metricPoints[0]; + Assert.Equal(0, metricPoint.GetSumLong()); + Assert.Equal(1, metricPoint.Tags.Count); + Assert.Equal("otel.metric.overflow", metricPoint.Tags.KeyAndValues[0].Key); + Assert.Equal(true, metricPoint.Tags.KeyAndValues[0].Value); + + metricPoint = metricPoints[1]; + Assert.Equal(100, metricPoint.GetSumLong()); + Assert.Equal(1, metricPoint.Tags.Count); + var tagEnumerator = metricPoint.Tags.GetEnumerator(); + tagEnumerator.MoveNext(); + Assert.Equal("tagWithNullValue", tagEnumerator.Current.Key); + Assert.Null(tagEnumerator.Current.Value); + } + else + { + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; + Assert.Equal(100, metricPoint.GetSumLong()); + Assert.Equal(1, metricPoint.Tags.Count); + var tagEnumerator = metricPoint.Tags.GetEnumerator(); + tagEnumerator.MoveNext(); + Assert.Equal("tagWithNullValue", tagEnumerator.Current.Key); + Assert.Null(tagEnumerator.Current.Value); + } } [Fact] @@ -94,10 +126,29 @@ public void ObserverCallbackTest() metricPoints.Add(mp); } - Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; - Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); - Assert.True(metricPoint.Tags.Count > 0); + if (this.emitOverflowAttribute) + { + // Cumulative metric export would always emit the metric overflow attribute first + Assert.Equal(2, metricPoints.Count); + + var metricPoint = metricPoints[0]; + Assert.Equal(0, metricPoint.GetGaugeLastValueLong()); + Assert.Equal(1, metricPoint.Tags.Count); + Assert.Equal("otel.metric.overflow", metricPoint.Tags.KeyAndValues[0].Key); + Assert.Equal(true, metricPoint.Tags.KeyAndValues[0].Value); + + metricPoint = metricPoints[1]; + Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); + Assert.True(metricPoint.Tags.Count > 0); + } + else + { + Assert.Single(metricPoints); + + var metricPoint = metricPoints[0]; + Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); + Assert.True(metricPoint.Tags.Count > 0); + } } [Fact] @@ -115,7 +166,17 @@ public void ObserverCallbackExceptionTest() meter.CreateObservableGauge("myBadGauge", observeValues: () => throw new Exception("gauge read error")); meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Single(exportedItems); + + if (this.emitOverflowAttribute) + { + // The metric point for overflow attribute would be created even though `observeValues` throws an exception + Assert.Equal(2, exportedItems.Count); + } + else + { + Assert.Single(exportedItems); + } + var metric = exportedItems[0]; Assert.Equal("myGauge", metric.Name); List metricPoints = new List(); @@ -124,10 +185,29 @@ public void ObserverCallbackExceptionTest() metricPoints.Add(mp); } - Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; - Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); - Assert.True(metricPoint.Tags.Count > 0); + if (this.emitOverflowAttribute) + { + // Cumulative metric export would always emit the metric overflow attribute first + Assert.Equal(2, metricPoints.Count); + + var metricPoint = metricPoints[0]; + Assert.Equal(0, metricPoint.GetGaugeLastValueLong()); + Assert.Equal(1, metricPoint.Tags.Count); + Assert.Equal("otel.metric.overflow", metricPoint.Tags.KeyAndValues[0].Key); + Assert.Equal(true, metricPoint.Tags.KeyAndValues[0].Value); + + metricPoint = metricPoints[1]; + Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); + Assert.True(metricPoint.Tags.Count > 0); + } + else + { + Assert.Single(metricPoints); + + var metricPoint = metricPoints[0]; + Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); + Assert.True(metricPoint.Tags.Count > 0); + } } [Theory] @@ -205,7 +285,16 @@ public void DuplicateInstrumentRegistration_NoViews_IdenticalInstruments() metricPoints.Add(mp); } - Assert.Single(metricPoints); + if (this.emitOverflowAttribute) + { + Assert.Equal(2, metricPoints.Count); + } + else + { + Assert.Single(metricPoints); + } + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. var metricPoint1 = metricPoints[0]; Assert.Equal(30, metricPoint1.GetSumLong()); } @@ -242,17 +331,27 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe metric1MetricPoints.Add(mp); } - Assert.Single(metric1MetricPoints); - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); - List metric2MetricPoints = new List(); foreach (ref readonly var mp in metric2.GetMetricPoints()) { metric2MetricPoints.Add(mp); } - Assert.Single(metric2MetricPoints); + if (this.emitOverflowAttribute) + { + Assert.Equal(2, metric1MetricPoints.Count); + Assert.Equal(2, metric2MetricPoints.Count); + } + else + { + Assert.Single(metric1MetricPoints); + Assert.Single(metric2MetricPoints); + } + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20, metricPoint2.GetSumLong()); } @@ -289,17 +388,27 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe metric1MetricPoints.Add(mp); } - Assert.Single(metric1MetricPoints); - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); - List metric2MetricPoints = new List(); foreach (ref readonly var mp in metric2.GetMetricPoints()) { metric2MetricPoints.Add(mp); } - Assert.Single(metric2MetricPoints); + if (this.emitOverflowAttribute) + { + Assert.Equal(2, metric1MetricPoints.Count); + Assert.Equal(2, metric2MetricPoints.Count); + } + else + { + Assert.Single(metric1MetricPoints); + Assert.Single(metric2MetricPoints); + } + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20, metricPoint2.GetSumLong()); } @@ -334,17 +443,27 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe metric1MetricPoints.Add(mp); } - Assert.Single(metric1MetricPoints); - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); - List metric2MetricPoints = new List(); foreach (ref readonly var mp in metric2.GetMetricPoints()) { metric2MetricPoints.Add(mp); } - Assert.Single(metric2MetricPoints); + if (this.emitOverflowAttribute) + { + Assert.Equal(2, metric1MetricPoints.Count); + Assert.Equal(2, metric2MetricPoints.Count); + } + else + { + Assert.Single(metric1MetricPoints); + Assert.Single(metric2MetricPoints); + } + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20D, metricPoint2.GetSumDouble()); } @@ -379,17 +498,27 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe metric1MetricPoints.Add(mp); } - Assert.Single(metric1MetricPoints); - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); - List metric2MetricPoints = new List(); foreach (ref readonly var mp in metric2.GetMetricPoints()) { metric2MetricPoints.Add(mp); } - Assert.Single(metric2MetricPoints); + if (this.emitOverflowAttribute) + { + Assert.Equal(2, metric1MetricPoints.Count); + Assert.Equal(2, metric2MetricPoints.Count); + } + else + { + Assert.Single(metric1MetricPoints); + Assert.Single(metric2MetricPoints); + } + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(1, metricPoint2.GetHistogramCount()); Assert.Equal(20D, metricPoint2.GetHistogramSum()); @@ -752,17 +881,27 @@ public void ObservableCounterWithTagsAggregationTest(bool exportDelta) metricPoints.Add(mp); } - Assert.Equal(3, metricPoints.Count); + int offsetForOverflowAttribute = 0; - var metricPoint1 = metricPoints[0]; + if (this.emitOverflowAttribute && exportDelta == false) + { + Assert.Equal(4, metricPoints.Count); + offsetForOverflowAttribute = 1; + } + else + { + Assert.Equal(3, metricPoints.Count); + } + + var metricPoint1 = metricPoints[offsetForOverflowAttribute + 0]; Assert.Equal(10, metricPoint1.GetSumLong()); ValidateMetricPointTags(tags1, metricPoint1.Tags); - var metricPoint2 = metricPoints[1]; + var metricPoint2 = metricPoints[offsetForOverflowAttribute + 1]; Assert.Equal(10, metricPoint2.GetSumLong()); ValidateMetricPointTags(tags2, metricPoint2.Tags); - var metricPoint3 = metricPoints[2]; + var metricPoint3 = metricPoints[offsetForOverflowAttribute + 2]; Assert.Equal(10, metricPoint3.GetSumLong()); ValidateMetricPointTags(tags3, metricPoint3.Tags); @@ -778,17 +917,25 @@ public void ObservableCounterWithTagsAggregationTest(bool exportDelta) metricPoints.Add(mp); } - Assert.Equal(3, metricPoints.Count); + if (this.emitOverflowAttribute && exportDelta == false) + { + Assert.Equal(4, metricPoints.Count); + offsetForOverflowAttribute = 1; + } + else + { + Assert.Equal(3, metricPoints.Count); + } - metricPoint1 = metricPoints[0]; + metricPoint1 = metricPoints[offsetForOverflowAttribute + 0]; Assert.Equal(exportDelta ? 0 : 10, metricPoint1.GetSumLong()); ValidateMetricPointTags(tags1, metricPoint1.Tags); - metricPoint2 = metricPoints[1]; + metricPoint2 = metricPoints[offsetForOverflowAttribute + 1]; Assert.Equal(exportDelta ? 0 : 10, metricPoint2.GetSumLong()); ValidateMetricPointTags(tags2, metricPoint2.Tags); - metricPoint3 = metricPoints[2]; + metricPoint3 = metricPoints[offsetForOverflowAttribute + 2]; Assert.Equal(exportDelta ? 0 : 10, metricPoint3.GetSumLong()); ValidateMetricPointTags(tags3, metricPoint3.Tags); } @@ -1036,17 +1183,29 @@ public void ObservableUpDownCounterWithTagsAggregationTest(bool exportDelta) metricPoints.Add(mp); } - Assert.Equal(3, metricPoints.Count); + int offsetForOverflowAttribute = 0; - var metricPoint1 = metricPoints[0]; + // No need to check specifically if the exporter temporality is Delta or Cumulative. + // ObservableUpDownCounter always uses Cumulative AggregationTemporality. + if (this.emitOverflowAttribute) + { + Assert.Equal(4, metricPoints.Count); + offsetForOverflowAttribute = 1; + } + else + { + Assert.Equal(3, metricPoints.Count); + } + + var metricPoint1 = metricPoints[offsetForOverflowAttribute + 0]; Assert.Equal(10, metricPoint1.GetSumLong()); ValidateMetricPointTags(tags1, metricPoint1.Tags); - var metricPoint2 = metricPoints[1]; + var metricPoint2 = metricPoints[offsetForOverflowAttribute + 1]; Assert.Equal(10, metricPoint2.GetSumLong()); ValidateMetricPointTags(tags2, metricPoint2.Tags); - var metricPoint3 = metricPoints[2]; + var metricPoint3 = metricPoints[offsetForOverflowAttribute + 2]; Assert.Equal(10, metricPoint3.GetSumLong()); ValidateMetricPointTags(tags3, metricPoint3.Tags); @@ -1062,18 +1221,26 @@ public void ObservableUpDownCounterWithTagsAggregationTest(bool exportDelta) metricPoints.Add(mp); } - Assert.Equal(3, metricPoints.Count); + if (this.emitOverflowAttribute) + { + Assert.Equal(4, metricPoints.Count); + offsetForOverflowAttribute = 1; + } + else + { + Assert.Equal(3, metricPoints.Count); + } // Same for both cumulative and delta. MetricReaderTemporalityPreference.Delta implies cumulative for UpDownCounters. - metricPoint1 = metricPoints[0]; + metricPoint1 = metricPoints[offsetForOverflowAttribute + 0]; Assert.Equal(10, metricPoint1.GetSumLong()); ValidateMetricPointTags(tags1, metricPoint1.Tags); - metricPoint2 = metricPoints[1]; + metricPoint2 = metricPoints[offsetForOverflowAttribute + 1]; Assert.Equal(10, metricPoint2.GetSumLong()); ValidateMetricPointTags(tags2, metricPoint2.Tags); - metricPoint3 = metricPoints[2]; + metricPoint3 = metricPoints[offsetForOverflowAttribute + 2]; Assert.Equal(10, metricPoint3.GetSumLong()); ValidateMetricPointTags(tags3, metricPoint3.Tags); } @@ -1134,11 +1301,22 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) new("Key6", "Value3"), }; - Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, 4); + int offsetForOverflowAttribute = 0; + + if (this.emitOverflowAttribute && exportDelta == false) + { + Assert.Equal(5, GetNumberOfMetricPoints(exportedItems)); + offsetForOverflowAttribute = 1; + } + else + { + Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); + } + + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, offsetForOverflowAttribute + 1); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, offsetForOverflowAttribute + 2); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, offsetForOverflowAttribute + 3); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, offsetForOverflowAttribute + 4); long sumReceived = GetLongSum(exportedItems); Assert.Equal(75, sumReceived); @@ -1153,11 +1331,20 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, 4); + if (this.emitOverflowAttribute && exportDelta == false) + { + Assert.Equal(5, GetNumberOfMetricPoints(exportedItems)); + offsetForOverflowAttribute = 1; + } + else + { + Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); + } + + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, offsetForOverflowAttribute + 1); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, offsetForOverflowAttribute + 2); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, offsetForOverflowAttribute + 3); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, offsetForOverflowAttribute + 4); sumReceived = GetLongSum(exportedItems); if (exportDelta) { @@ -1225,11 +1412,22 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) new("Key6", "Value3"), }; - Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, 4); + int offsetForOverflowAttribute = 0; + + if (this.emitOverflowAttribute && exportDelta == false) + { + Assert.Equal(5, GetNumberOfMetricPoints(exportedItems)); + offsetForOverflowAttribute = 1; + } + else + { + Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); + } + + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, offsetForOverflowAttribute + 1); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, offsetForOverflowAttribute + 2); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, offsetForOverflowAttribute + 3); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, offsetForOverflowAttribute + 4); long sumReceived = GetLongSum(exportedItems); Assert.Equal(75, sumReceived); @@ -1244,11 +1442,20 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, 4); + if (this.emitOverflowAttribute && exportDelta == false) + { + Assert.Equal(5, GetNumberOfMetricPoints(exportedItems)); + offsetForOverflowAttribute = 1; + } + else + { + Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); + } + + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, offsetForOverflowAttribute + 1); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, offsetForOverflowAttribute + 2); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, offsetForOverflowAttribute + 3); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, offsetForOverflowAttribute + 4); sumReceived = GetLongSum(exportedItems); if (exportDelta) { @@ -1518,6 +1725,11 @@ public void UnsupportedMetricInstrument() Assert.Empty(exportedItems); } + public void Dispose() + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + } + private static void CounterUpdateThread(object obj) where T : struct, IComparable { @@ -1668,11 +1880,14 @@ private void MultithreadedHistogramTest(long[] expected, T[] values) metricReader.Collect(); + List metricPoints = new List(); + foreach (var metric in metrics) { foreach (var metricPoint in metric.GetMetricPoints()) { bucketCounts = metricPoint.GetHistogramBuckets().RunningBucketCounts; + break; } } @@ -1689,3 +1904,19 @@ private class UpdateThreadArguments public T[] ValuesToRecord; } } + +public class MetricApiTest : MetricApiTestBase +{ + public MetricApiTest(ITestOutputHelper output) + : base(output, false) + { + } +} + +public class MetricApiTestWithOverflowAttribute : MetricApiTestBase +{ + public MetricApiTestWithOverflowAttribute(ITestOutputHelper output) + : base(output, true) + { + } +} diff --git a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs new file mode 100644 index 00000000000..15ec17cd834 --- /dev/null +++ b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs @@ -0,0 +1,130 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Diagnostics.Metrics; +using OpenTelemetry.Tests; +using Xunit; + +namespace OpenTelemetry.Metrics.Tests; + +public class MetricOverflowAttributeTests +{ + [Theory] + [InlineData(MetricReaderTemporalityPreference.Delta)] + [InlineData(MetricReaderTemporalityPreference.Cumulative)] + public void MetricOverflowAttributeIsRecordedCorrectly(MetricReaderTemporalityPreference temporalityPreference) + { + try + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + + var exportedItems = new List(); + + var meter = new Meter(Utils.GetCurrentMethodName()); + var counter = meter.CreateCounter("TestCounter"); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems, metricReaderOptions => metricReaderOptions.TemporalityPreference = temporalityPreference) + .Build(); + + // There are two reserved MetricPoints + // 1. For zero tags + // 2. For metric overflow attribute when user opts-in for this feature + + // Max number for MetricPoints available for use when emitted with tags + int maxMetricPointsForUse = MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault - 2; + + for (int i = 0; i < maxMetricPointsForUse; i++) + { + // Emit unique key-value pairs to use up the available MetricPoints + // Once this loop is run, we have used up all available MetricPoints for metrics emitted with tags + counter.Add(10, new KeyValuePair("Key", i)); + } + + meterProvider.ForceFlush(); + + Assert.Single(exportedItems); + var metric = exportedItems[0]; + + var metricPoints = new List(); + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + MetricPoint overflowMetricPoint; + + // We still have not exceeded the max MetricPoint limit + if (temporalityPreference == MetricReaderTemporalityPreference.Delta) + { + Assert.DoesNotContain(metricPoints, mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + } + else + { + overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + Assert.Equal(true, overflowMetricPoint.Tags.KeyAndValues[0].Value); + Assert.Equal(0, overflowMetricPoint.GetSumLong()); // No recording should have been made for the overflow attribute at this point + } + + exportedItems.Clear(); + metricPoints.Clear(); + + counter.Add(5, new KeyValuePair("Key", 9999)); // Emit a metric to exceed the max MetricPoint limit + + meterProvider.ForceFlush(); + metric = exportedItems[0]; + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + Assert.Equal(true, overflowMetricPoint.Tags.KeyAndValues[0].Value); + Assert.Equal(5, overflowMetricPoint.GetSumLong()); + + exportedItems.Clear(); + metricPoints.Clear(); + + // Emit 50 more newer MetricPoints with distinct dimension combinations + for (int i = 10000; i < 10050; i++) + { + counter.Add(5, new KeyValuePair("Key", i)); + } + + meterProvider.ForceFlush(); + metric = exportedItems[0]; + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + if (temporalityPreference == MetricReaderTemporalityPreference.Delta) + { + Assert.Equal(250, overflowMetricPoint.GetSumLong()); // 50 * 5 + } + else + { + Assert.Equal(255, overflowMetricPoint.GetSumLong()); // 5 + (50 * 5) + } + } + finally + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + } + } +} diff --git a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs similarity index 80% rename from test/OpenTelemetry.Tests/Metrics/MetricSnapshotTests.cs rename to test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs index 37043198aef..812ad67f786 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -22,8 +22,22 @@ namespace OpenTelemetry.Metrics.Tests; -public class MetricSnapshotTests +#pragma warning disable SA1402 + +public abstract class MetricSnapshotTestsBase : IDisposable { + private readonly bool emitOverflowAttribute; + + protected MetricSnapshotTestsBase(bool emitOverflowAttribute) + { + this.emitOverflowAttribute = emitOverflowAttribute; + + if (emitOverflowAttribute) + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + } + } + [Fact] public void VerifySnapshot_Counter() { @@ -53,7 +67,16 @@ public void VerifySnapshot_Counter() // Verify Snapshot 1 Assert.Single(exportedSnapshots); var snapshot1 = exportedSnapshots[0]; - Assert.Single(snapshot1.MetricPoints); + + if (this.emitOverflowAttribute) + { + Assert.Equal(2, snapshot1.MetricPoints.Count); + } + else + { + Assert.Single(snapshot1.MetricPoints); + } + Assert.Equal(10, snapshot1.MetricPoints[0].GetSumLong()); // Verify Metric == Snapshot @@ -87,7 +110,16 @@ public void VerifySnapshot_Counter() // Verify Snapshot 2 Assert.Equal(2, exportedSnapshots.Count); var snapshot2 = exportedSnapshots[1]; - Assert.Single(snapshot2.MetricPoints); + + if (this.emitOverflowAttribute) + { + Assert.Equal(2, snapshot2.MetricPoints.Count); + } + else + { + Assert.Single(snapshot2.MetricPoints); + } + Assert.Equal(15, snapshot2.MetricPoints[0].GetSumLong()); } @@ -124,7 +156,16 @@ public void VerifySnapshot_Histogram() // Verify Snapshot 1 Assert.Single(exportedSnapshots); var snapshot1 = exportedSnapshots[0]; - Assert.Single(snapshot1.MetricPoints); + + if (this.emitOverflowAttribute) + { + Assert.Equal(2, snapshot1.MetricPoints.Count); + } + else + { + 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); @@ -174,7 +215,16 @@ public void VerifySnapshot_Histogram() // Verify Snapshot 2 Assert.Equal(2, exportedSnapshots.Count); var snapshot2 = exportedSnapshots[1]; - Assert.Single(snapshot2.MetricPoints); + + if (this.emitOverflowAttribute) + { + Assert.Equal(2, snapshot2.MetricPoints.Count); + } + else + { + 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); @@ -214,18 +264,27 @@ public void VerifySnapshot_ExponentialHistogram() metricPoint1.TryGetHistogramMinMaxValues(out var min, out var max); Assert.Equal(10, min); Assert.Equal(10, max); - AggregatorTest.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint1.GetExponentialHistogramData()); + AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint1.GetExponentialHistogramData()); // Verify Snapshot 1 Assert.Single(exportedSnapshots); var snapshot1 = exportedSnapshots[0]; - Assert.Single(snapshot1.MetricPoints); + + if (this.emitOverflowAttribute) + { + Assert.Equal(2, snapshot1.MetricPoints.Count); + } + else + { + 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()); + AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, snapshot1.MetricPoints[0].GetExponentialHistogramData()); // Verify Metric == Snapshot Assert.Equal(metric1.Name, snapshot1.Name); @@ -259,7 +318,7 @@ public void VerifySnapshot_ExponentialHistogram() metricPoint1.TryGetHistogramMinMaxValues(out min, out max); Assert.Equal(5, min); Assert.Equal(10, max); - AggregatorTest.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint2.GetExponentialHistogramData()); + AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint2.GetExponentialHistogramData()); // Verify Snapshot 1 after second export // This value is expected to be unchanged. @@ -272,12 +331,42 @@ public void VerifySnapshot_ExponentialHistogram() // Verify Snapshot 2 Assert.Equal(2, exportedSnapshots.Count); var snapshot2 = exportedSnapshots[1]; - Assert.Single(snapshot2.MetricPoints); + + if (this.emitOverflowAttribute) + { + Assert.Equal(2, snapshot2.MetricPoints.Count); + } + else + { + 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()); + AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, snapshot2.MetricPoints[0].GetExponentialHistogramData()); + } + + public void Dispose() + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + } +} + +public class MetricSnapshotTests : MetricSnapshotTestsBase +{ + public MetricSnapshotTests() + : base(false) + { + } +} + +public class MetricSnapshotTestsWithOverflowAttribute : MetricSnapshotTestsBase +{ + public MetricSnapshotTestsWithOverflowAttribute() + : base(true) + { } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 82cc2c57aff..d430c9ed886 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -636,7 +636,7 @@ public void ViewToProduceExponentialHistogram() var count = metricPoint.GetHistogramCount(); var sum = metricPoint.GetHistogramSum(); - AggregatorTest.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint.GetExponentialHistogramData()); + AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint.GetExponentialHistogramData()); Assert.Equal(50, sum); Assert.Equal(6, count); } From 21b0e778b654ac25fcfc053740cab81259466273 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Wed, 2 Aug 2023 15:59:28 -0700 Subject: [PATCH 02/10] Add test for Histogram --- .../Metrics/MetricOverflowAttributeTests.cs | 112 +++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs index 15ec17cd834..5d4434ac337 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs @@ -25,7 +25,7 @@ public class MetricOverflowAttributeTests [Theory] [InlineData(MetricReaderTemporalityPreference.Delta)] [InlineData(MetricReaderTemporalityPreference.Cumulative)] - public void MetricOverflowAttributeIsRecordedCorrectly(MetricReaderTemporalityPreference temporalityPreference) + public void MetricOverflowAttributeIsRecordedCorrectlyForCounter(MetricReaderTemporalityPreference temporalityPreference) { try { @@ -127,4 +127,114 @@ public void MetricOverflowAttributeIsRecordedCorrectly(MetricReaderTemporalityPr AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); } } + + [Theory] + [InlineData(MetricReaderTemporalityPreference.Delta)] + [InlineData(MetricReaderTemporalityPreference.Cumulative)] + public void MetricOverflowAttributeIsRecordedCorrectlyForHistogram(MetricReaderTemporalityPreference temporalityPreference) + { + try + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + + var exportedItems = new List(); + + var meter = new Meter(Utils.GetCurrentMethodName()); + var histogram = meter.CreateHistogram("TestHistogram"); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems, metricReaderOptions => metricReaderOptions.TemporalityPreference = temporalityPreference) + .Build(); + + // There are two reserved MetricPoints + // 1. For zero tags + // 2. For metric overflow attribute when user opts-in for this feature + + // Max number for MetricPoints available for use when emitted with tags + int maxMetricPointsForUse = MeterProviderBuilderSdk.MaxMetricPointsPerMetricDefault - 2; + + for (int i = 0; i < maxMetricPointsForUse; i++) + { + // Emit unique key-value pairs to use up the available MetricPoints + // Once this loop is run, we have used up all available MetricPoints for metrics emitted with tags + histogram.Record(10, new KeyValuePair("Key", i)); + } + + meterProvider.ForceFlush(); + + Assert.Single(exportedItems); + var metric = exportedItems[0]; + + var metricPoints = new List(); + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + MetricPoint overflowMetricPoint; + + // We still have not exceeded the max MetricPoint limit + if (temporalityPreference == MetricReaderTemporalityPreference.Delta) + { + Assert.DoesNotContain(metricPoints, mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + } + else + { + overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + Assert.Equal(true, overflowMetricPoint.Tags.KeyAndValues[0].Value); + Assert.Equal(0, overflowMetricPoint.GetHistogramSum()); // No recording should have been made for the overflow attribute at this point + Assert.Equal(0, overflowMetricPoint.GetHistogramCount()); + } + + exportedItems.Clear(); + metricPoints.Clear(); + + histogram.Record(5, new KeyValuePair("Key", 9999)); // Emit a metric to exceed the max MetricPoint limit + + meterProvider.ForceFlush(); + metric = exportedItems[0]; + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + Assert.Equal(true, overflowMetricPoint.Tags.KeyAndValues[0].Value); + Assert.Equal(5, overflowMetricPoint.GetHistogramSum()); + Assert.Equal(1, overflowMetricPoint.GetHistogramCount()); + + exportedItems.Clear(); + metricPoints.Clear(); + + // Emit 50 more newer MetricPoints with distinct dimension combinations + for (int i = 10000; i < 10050; i++) + { + histogram.Record(5, new KeyValuePair("Key", i)); + } + + meterProvider.ForceFlush(); + metric = exportedItems[0]; + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + if (temporalityPreference == MetricReaderTemporalityPreference.Delta) + { + Assert.Equal(250, overflowMetricPoint.GetHistogramSum()); // 50 * 5 + Assert.Equal(50, overflowMetricPoint.GetHistogramCount()); + } + else + { + Assert.Equal(255, overflowMetricPoint.GetHistogramSum()); // 5 + (50 * 5) + Assert.Equal(51, overflowMetricPoint.GetHistogramCount()); + } + } + finally + { + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + } + } } From e029e377317a24c08a1bc7ebfe2264741ba3b64d Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Thu, 3 Aug 2023 18:47:44 -0700 Subject: [PATCH 03/10] Address PR comments --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 24 +- .../Metrics/AggregatorTestBase.cs | 21 +- .../Metrics/MetricApiTestBase.cs | 342 +++++------------- .../Metrics/MetricOverflowAttributeTests.cs | 23 +- .../Metrics/MetricSnapshotTestsBase.cs | 58 +-- 5 files changed, 114 insertions(+), 354 deletions(-) diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index a087e920008..cff19b5c54e 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -27,6 +27,7 @@ internal sealed class AggregatorStore private static bool emitOverflowAttribute; private readonly object lockZeroTags = new(); + private readonly object lockOverflowTag = new(); private readonly HashSet tagKeysInteresting; private readonly int tagsKeysInterestingCount; @@ -51,6 +52,7 @@ internal sealed class AggregatorStore private int batchSize = 0; private int metricCapHitMessageLogged; private bool zeroTagMetricPointInitialized; + private bool overflowTagMetricPointInitialized; internal AggregatorStore( MetricStreamIdentity metricStreamIdentity, @@ -100,8 +102,6 @@ internal AggregatorStore( // Setting this to as we would reserve the metricPoints[1] for overflow attribute. // Newer attributes should be added starting at the index: 2 this.metricPointIndex = 1; - - this.metricPoints[1] = new MetricPoint(this, this.aggType, new KeyValuePair[] { new("otel.metric.overflow", true) }, this.histogramBounds, this.exponentialHistogramMaxSize, this.exponentialHistogramMaxScale); } } @@ -219,6 +219,22 @@ private void InitializeZeroTagPointIfNotInitialized() } } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void InitializeOverflowTagPointIfNotInitialized() + { + if (!this.overflowTagMetricPointInitialized) + { + lock (this.lockOverflowTag) + { + if (!this.overflowTagMetricPointInitialized) + { + this.metricPoints[1] = new MetricPoint(this, this.aggType, new KeyValuePair[] { new("otel.metric.overflow", true) }, this.histogramBounds, this.exponentialHistogramMaxSize, this.exponentialHistogramMaxScale); + this.overflowTagMetricPointInitialized = true; + } + } + } + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private int LookupAggregatorStore(KeyValuePair[] tagKeysAndValues, int length) { @@ -353,6 +369,7 @@ private void UpdateLong(long value, ReadOnlySpan> t { if (emitOverflowAttribute) { + this.InitializeOverflowTagPointIfNotInitialized(); this.metricPoints[1].Update(value); return; } @@ -393,6 +410,7 @@ private void UpdateLongCustomTags(long value, ReadOnlySpan 0); - } - else - { - Assert.Single(metricPoints); + Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; - Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); - Assert.True(metricPoint.Tags.Count > 0); - } + var metricPoint = metricPoints[0]; + Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); + Assert.True(metricPoint.Tags.Count > 0); } [Fact] @@ -167,15 +124,7 @@ public void ObserverCallbackExceptionTest() meterProvider.ForceFlush(MaxTimeToAllowForFlush); - if (this.emitOverflowAttribute) - { - // The metric point for overflow attribute would be created even though `observeValues` throws an exception - Assert.Equal(2, exportedItems.Count); - } - else - { - Assert.Single(exportedItems); - } + Assert.Single(exportedItems); var metric = exportedItems[0]; Assert.Equal("myGauge", metric.Name); @@ -185,29 +134,11 @@ public void ObserverCallbackExceptionTest() metricPoints.Add(mp); } - if (this.emitOverflowAttribute) - { - // Cumulative metric export would always emit the metric overflow attribute first - Assert.Equal(2, metricPoints.Count); - - var metricPoint = metricPoints[0]; - Assert.Equal(0, metricPoint.GetGaugeLastValueLong()); - Assert.Equal(1, metricPoint.Tags.Count); - Assert.Equal("otel.metric.overflow", metricPoint.Tags.KeyAndValues[0].Key); - Assert.Equal(true, metricPoint.Tags.KeyAndValues[0].Value); - - metricPoint = metricPoints[1]; - Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); - Assert.True(metricPoint.Tags.Count > 0); - } - else - { - Assert.Single(metricPoints); + Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; - Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); - Assert.True(metricPoint.Tags.Count > 0); - } + var metricPoint = metricPoints[0]; + Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); + Assert.True(metricPoint.Tags.Count > 0); } [Theory] @@ -285,14 +216,7 @@ public void DuplicateInstrumentRegistration_NoViews_IdenticalInstruments() metricPoints.Add(mp); } - if (this.emitOverflowAttribute) - { - Assert.Equal(2, metricPoints.Count); - } - else - { - Assert.Single(metricPoints); - } + Assert.Single(metricPoints); // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. var metricPoint1 = metricPoints[0]; @@ -331,26 +255,19 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe metric1MetricPoints.Add(mp); } + Assert.Single(metric1MetricPoints); + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + List metric2MetricPoints = new List(); foreach (ref readonly var mp in metric2.GetMetricPoints()) { metric2MetricPoints.Add(mp); } - if (this.emitOverflowAttribute) - { - Assert.Equal(2, metric1MetricPoints.Count); - Assert.Equal(2, metric2MetricPoints.Count); - } - else - { - Assert.Single(metric1MetricPoints); - Assert.Single(metric2MetricPoints); - } - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); + Assert.Single(metric2MetricPoints); var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20, metricPoint2.GetSumLong()); @@ -388,26 +305,19 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe metric1MetricPoints.Add(mp); } + Assert.Single(metric1MetricPoints); + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + List metric2MetricPoints = new List(); foreach (ref readonly var mp in metric2.GetMetricPoints()) { metric2MetricPoints.Add(mp); } - if (this.emitOverflowAttribute) - { - Assert.Equal(2, metric1MetricPoints.Count); - Assert.Equal(2, metric2MetricPoints.Count); - } - else - { - Assert.Single(metric1MetricPoints); - Assert.Single(metric2MetricPoints); - } - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); + Assert.Single(metric2MetricPoints); var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20, metricPoint2.GetSumLong()); @@ -443,26 +353,19 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe metric1MetricPoints.Add(mp); } + Assert.Single(metric1MetricPoints); + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + List metric2MetricPoints = new List(); foreach (ref readonly var mp in metric2.GetMetricPoints()) { metric2MetricPoints.Add(mp); } - if (this.emitOverflowAttribute) - { - Assert.Equal(2, metric1MetricPoints.Count); - Assert.Equal(2, metric2MetricPoints.Count); - } - else - { - Assert.Single(metric1MetricPoints); - Assert.Single(metric2MetricPoints); - } - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); + Assert.Single(metric2MetricPoints); var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20D, metricPoint2.GetSumDouble()); @@ -498,26 +401,19 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe metric1MetricPoints.Add(mp); } + Assert.Single(metric1MetricPoints); + + // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + List metric2MetricPoints = new List(); foreach (ref readonly var mp in metric2.GetMetricPoints()) { metric2MetricPoints.Add(mp); } - if (this.emitOverflowAttribute) - { - Assert.Equal(2, metric1MetricPoints.Count); - Assert.Equal(2, metric2MetricPoints.Count); - } - else - { - Assert.Single(metric1MetricPoints); - Assert.Single(metric2MetricPoints); - } - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); + Assert.Single(metric2MetricPoints); var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(1, metricPoint2.GetHistogramCount()); @@ -881,27 +777,17 @@ public void ObservableCounterWithTagsAggregationTest(bool exportDelta) metricPoints.Add(mp); } - int offsetForOverflowAttribute = 0; + Assert.Equal(3, metricPoints.Count); - if (this.emitOverflowAttribute && exportDelta == false) - { - Assert.Equal(4, metricPoints.Count); - offsetForOverflowAttribute = 1; - } - else - { - Assert.Equal(3, metricPoints.Count); - } - - var metricPoint1 = metricPoints[offsetForOverflowAttribute + 0]; + var metricPoint1 = metricPoints[0]; Assert.Equal(10, metricPoint1.GetSumLong()); ValidateMetricPointTags(tags1, metricPoint1.Tags); - var metricPoint2 = metricPoints[offsetForOverflowAttribute + 1]; + var metricPoint2 = metricPoints[1]; Assert.Equal(10, metricPoint2.GetSumLong()); ValidateMetricPointTags(tags2, metricPoint2.Tags); - var metricPoint3 = metricPoints[offsetForOverflowAttribute + 2]; + var metricPoint3 = metricPoints[2]; Assert.Equal(10, metricPoint3.GetSumLong()); ValidateMetricPointTags(tags3, metricPoint3.Tags); @@ -917,25 +803,17 @@ public void ObservableCounterWithTagsAggregationTest(bool exportDelta) metricPoints.Add(mp); } - if (this.emitOverflowAttribute && exportDelta == false) - { - Assert.Equal(4, metricPoints.Count); - offsetForOverflowAttribute = 1; - } - else - { - Assert.Equal(3, metricPoints.Count); - } + Assert.Equal(3, metricPoints.Count); - metricPoint1 = metricPoints[offsetForOverflowAttribute + 0]; + metricPoint1 = metricPoints[0]; Assert.Equal(exportDelta ? 0 : 10, metricPoint1.GetSumLong()); ValidateMetricPointTags(tags1, metricPoint1.Tags); - metricPoint2 = metricPoints[offsetForOverflowAttribute + 1]; + metricPoint2 = metricPoints[1]; Assert.Equal(exportDelta ? 0 : 10, metricPoint2.GetSumLong()); ValidateMetricPointTags(tags2, metricPoint2.Tags); - metricPoint3 = metricPoints[offsetForOverflowAttribute + 2]; + metricPoint3 = metricPoints[2]; Assert.Equal(exportDelta ? 0 : 10, metricPoint3.GetSumLong()); ValidateMetricPointTags(tags3, metricPoint3.Tags); } @@ -1183,29 +1061,17 @@ public void ObservableUpDownCounterWithTagsAggregationTest(bool exportDelta) metricPoints.Add(mp); } - int offsetForOverflowAttribute = 0; + Assert.Equal(3, metricPoints.Count); - // No need to check specifically if the exporter temporality is Delta or Cumulative. - // ObservableUpDownCounter always uses Cumulative AggregationTemporality. - if (this.emitOverflowAttribute) - { - Assert.Equal(4, metricPoints.Count); - offsetForOverflowAttribute = 1; - } - else - { - Assert.Equal(3, metricPoints.Count); - } - - var metricPoint1 = metricPoints[offsetForOverflowAttribute + 0]; + var metricPoint1 = metricPoints[0]; Assert.Equal(10, metricPoint1.GetSumLong()); ValidateMetricPointTags(tags1, metricPoint1.Tags); - var metricPoint2 = metricPoints[offsetForOverflowAttribute + 1]; + var metricPoint2 = metricPoints[1]; Assert.Equal(10, metricPoint2.GetSumLong()); ValidateMetricPointTags(tags2, metricPoint2.Tags); - var metricPoint3 = metricPoints[offsetForOverflowAttribute + 2]; + var metricPoint3 = metricPoints[2]; Assert.Equal(10, metricPoint3.GetSumLong()); ValidateMetricPointTags(tags3, metricPoint3.Tags); @@ -1221,26 +1087,18 @@ public void ObservableUpDownCounterWithTagsAggregationTest(bool exportDelta) metricPoints.Add(mp); } - if (this.emitOverflowAttribute) - { - Assert.Equal(4, metricPoints.Count); - offsetForOverflowAttribute = 1; - } - else - { - Assert.Equal(3, metricPoints.Count); - } + Assert.Equal(3, metricPoints.Count); // Same for both cumulative and delta. MetricReaderTemporalityPreference.Delta implies cumulative for UpDownCounters. - metricPoint1 = metricPoints[offsetForOverflowAttribute + 0]; + metricPoint1 = metricPoints[0]; Assert.Equal(10, metricPoint1.GetSumLong()); ValidateMetricPointTags(tags1, metricPoint1.Tags); - metricPoint2 = metricPoints[offsetForOverflowAttribute + 1]; + metricPoint2 = metricPoints[1]; Assert.Equal(10, metricPoint2.GetSumLong()); ValidateMetricPointTags(tags2, metricPoint2.Tags); - metricPoint3 = metricPoints[offsetForOverflowAttribute + 2]; + metricPoint3 = metricPoints[2]; Assert.Equal(10, metricPoint3.GetSumLong()); ValidateMetricPointTags(tags3, metricPoint3.Tags); } @@ -1301,22 +1159,12 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) new("Key6", "Value3"), }; - int offsetForOverflowAttribute = 0; - - if (this.emitOverflowAttribute && exportDelta == false) - { - Assert.Equal(5, GetNumberOfMetricPoints(exportedItems)); - offsetForOverflowAttribute = 1; - } - else - { - Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - } + Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, offsetForOverflowAttribute + 1); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, offsetForOverflowAttribute + 2); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, offsetForOverflowAttribute + 3); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, offsetForOverflowAttribute + 4); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, 4); long sumReceived = GetLongSum(exportedItems); Assert.Equal(75, sumReceived); @@ -1331,20 +1179,12 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) meterProvider.ForceFlush(MaxTimeToAllowForFlush); - if (this.emitOverflowAttribute && exportDelta == false) - { - Assert.Equal(5, GetNumberOfMetricPoints(exportedItems)); - offsetForOverflowAttribute = 1; - } - else - { - Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - } + Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, offsetForOverflowAttribute + 1); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, offsetForOverflowAttribute + 2); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, offsetForOverflowAttribute + 3); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, offsetForOverflowAttribute + 4); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, 4); sumReceived = GetLongSum(exportedItems); if (exportDelta) { @@ -1412,22 +1252,12 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) new("Key6", "Value3"), }; - int offsetForOverflowAttribute = 0; + Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - if (this.emitOverflowAttribute && exportDelta == false) - { - Assert.Equal(5, GetNumberOfMetricPoints(exportedItems)); - offsetForOverflowAttribute = 1; - } - else - { - Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - } - - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, offsetForOverflowAttribute + 1); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, offsetForOverflowAttribute + 2); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, offsetForOverflowAttribute + 3); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, offsetForOverflowAttribute + 4); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, 4); long sumReceived = GetLongSum(exportedItems); Assert.Equal(75, sumReceived); @@ -1442,20 +1272,12 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) meterProvider.ForceFlush(MaxTimeToAllowForFlush); - if (this.emitOverflowAttribute && exportDelta == false) - { - Assert.Equal(5, GetNumberOfMetricPoints(exportedItems)); - offsetForOverflowAttribute = 1; - } - else - { - Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - } + Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, offsetForOverflowAttribute + 1); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, offsetForOverflowAttribute + 2); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, offsetForOverflowAttribute + 3); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, offsetForOverflowAttribute + 4); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); + CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFourthMetricPoint, 4); sumReceived = GetLongSum(exportedItems); if (exportDelta) { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs index 5d4434ac337..1a3957be7fb 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs @@ -69,16 +69,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForCounter(MetricReaderTem MetricPoint overflowMetricPoint; // We still have not exceeded the max MetricPoint limit - if (temporalityPreference == MetricReaderTemporalityPreference.Delta) - { - Assert.DoesNotContain(metricPoints, mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); - } - else - { - overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); - Assert.Equal(true, overflowMetricPoint.Tags.KeyAndValues[0].Value); - Assert.Equal(0, overflowMetricPoint.GetSumLong()); // No recording should have been made for the overflow attribute at this point - } + Assert.DoesNotContain(metricPoints, mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); exportedItems.Clear(); metricPoints.Clear(); @@ -175,17 +166,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForHistogram(MetricReaderT MetricPoint overflowMetricPoint; // We still have not exceeded the max MetricPoint limit - if (temporalityPreference == MetricReaderTemporalityPreference.Delta) - { - Assert.DoesNotContain(metricPoints, mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); - } - else - { - overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); - Assert.Equal(true, overflowMetricPoint.Tags.KeyAndValues[0].Value); - Assert.Equal(0, overflowMetricPoint.GetHistogramSum()); // No recording should have been made for the overflow attribute at this point - Assert.Equal(0, overflowMetricPoint.GetHistogramCount()); - } + Assert.DoesNotContain(metricPoints, mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); exportedItems.Clear(); metricPoints.Clear(); diff --git a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs index 812ad67f786..4854152ee9a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs @@ -26,12 +26,8 @@ namespace OpenTelemetry.Metrics.Tests; public abstract class MetricSnapshotTestsBase : IDisposable { - private readonly bool emitOverflowAttribute; - protected MetricSnapshotTestsBase(bool emitOverflowAttribute) { - this.emitOverflowAttribute = emitOverflowAttribute; - if (emitOverflowAttribute) { AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); @@ -68,14 +64,7 @@ public void VerifySnapshot_Counter() Assert.Single(exportedSnapshots); var snapshot1 = exportedSnapshots[0]; - if (this.emitOverflowAttribute) - { - Assert.Equal(2, snapshot1.MetricPoints.Count); - } - else - { - Assert.Single(snapshot1.MetricPoints); - } + Assert.Single(snapshot1.MetricPoints); Assert.Equal(10, snapshot1.MetricPoints[0].GetSumLong()); @@ -111,14 +100,7 @@ public void VerifySnapshot_Counter() Assert.Equal(2, exportedSnapshots.Count); var snapshot2 = exportedSnapshots[1]; - if (this.emitOverflowAttribute) - { - Assert.Equal(2, snapshot2.MetricPoints.Count); - } - else - { - Assert.Single(snapshot2.MetricPoints); - } + Assert.Single(snapshot2.MetricPoints); Assert.Equal(15, snapshot2.MetricPoints[0].GetSumLong()); } @@ -157,14 +139,7 @@ public void VerifySnapshot_Histogram() Assert.Single(exportedSnapshots); var snapshot1 = exportedSnapshots[0]; - if (this.emitOverflowAttribute) - { - Assert.Equal(2, snapshot1.MetricPoints.Count); - } - else - { - Assert.Single(snapshot1.MetricPoints); - } + Assert.Single(snapshot1.MetricPoints); Assert.Equal(1, snapshot1.MetricPoints[0].GetHistogramCount()); Assert.Equal(10, snapshot1.MetricPoints[0].GetHistogramSum()); @@ -216,14 +191,7 @@ public void VerifySnapshot_Histogram() Assert.Equal(2, exportedSnapshots.Count); var snapshot2 = exportedSnapshots[1]; - if (this.emitOverflowAttribute) - { - Assert.Equal(2, snapshot2.MetricPoints.Count); - } - else - { - Assert.Single(snapshot2.MetricPoints); - } + Assert.Single(snapshot2.MetricPoints); Assert.Equal(2, snapshot2.MetricPoints[0].GetHistogramCount()); Assert.Equal(15, snapshot2.MetricPoints[0].GetHistogramSum()); @@ -270,14 +238,7 @@ public void VerifySnapshot_ExponentialHistogram() Assert.Single(exportedSnapshots); var snapshot1 = exportedSnapshots[0]; - if (this.emitOverflowAttribute) - { - Assert.Equal(2, snapshot1.MetricPoints.Count); - } - else - { - Assert.Single(snapshot1.MetricPoints); - } + Assert.Single(snapshot1.MetricPoints); Assert.Equal(1, snapshot1.MetricPoints[0].GetHistogramCount()); Assert.Equal(10, snapshot1.MetricPoints[0].GetHistogramSum()); @@ -332,14 +293,7 @@ public void VerifySnapshot_ExponentialHistogram() Assert.Equal(2, exportedSnapshots.Count); var snapshot2 = exportedSnapshots[1]; - if (this.emitOverflowAttribute) - { - Assert.Equal(2, snapshot2.MetricPoints.Count); - } - else - { - Assert.Single(snapshot2.MetricPoints); - } + Assert.Single(snapshot2.MetricPoints); Assert.Equal(2, snapshot2.MetricPoints[0].GetHistogramCount()); Assert.Equal(15, snapshot2.MetricPoints[0].GetHistogramSum()); From 2b4d05365c9d159ccbe896c07af778c163c146a9 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Thu, 3 Aug 2023 20:46:55 -0700 Subject: [PATCH 04/10] Update tests --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 4 +-- .../Metrics/AggregatorTestBase.cs | 2 -- .../Metrics/MetricApiTestBase.cs | 25 ------------------- .../Metrics/MetricSnapshotTestsBase.cs | 10 -------- 4 files changed, 1 insertion(+), 40 deletions(-) diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index cff19b5c54e..b09b87635a0 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -95,11 +95,9 @@ internal AggregatorStore( } else if (this.maxMetricPoints > 1) { - // We need at least two metric points. One is reserved for zero tags and the other one for overflow attribute - emitOverflowAttribute = true; - // Setting this to as we would reserve the metricPoints[1] for overflow attribute. + // Setting metricPointIndex to 1 as we would reserve the metricPoints[1] for overflow attribute. // Newer attributes should be added starting at the index: 2 this.metricPointIndex = 1; } diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTestBase.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTestBase.cs index 3b26daa50b9..220f1425e77 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTestBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTestBase.cs @@ -324,7 +324,6 @@ internal void ExponentialHistogramTests(AggregationType aggregationType, Aggrega } Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; var count = metricPoint.GetHistogramCount(); @@ -422,7 +421,6 @@ internal void ExponentialMaxScaleConfigWorks(int? maxScale) } Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; // After a single measurement there will not have been a scale down. diff --git a/test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs index 36523f4be86..23ba185046a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs @@ -102,7 +102,6 @@ public void ObserverCallbackTest() } Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); Assert.True(metricPoint.Tags.Count > 0); @@ -123,9 +122,7 @@ public void ObserverCallbackExceptionTest() meter.CreateObservableGauge("myBadGauge", observeValues: () => throw new Exception("gauge read error")); meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Single(exportedItems); - var metric = exportedItems[0]; Assert.Equal("myGauge", metric.Name); List metricPoints = new List(); @@ -135,7 +132,6 @@ public void ObserverCallbackExceptionTest() } Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; Assert.Equal(100, metricPoint.GetGaugeLastValueLong()); Assert.True(metricPoint.Tags.Count > 0); @@ -217,8 +213,6 @@ public void DuplicateInstrumentRegistration_NoViews_IdenticalInstruments() } Assert.Single(metricPoints); - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. var metricPoint1 = metricPoints[0]; Assert.Equal(30, metricPoint1.GetSumLong()); } @@ -256,8 +250,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe } Assert.Single(metric1MetricPoints); - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. var metricPoint1 = metric1MetricPoints[0]; Assert.Equal(10, metricPoint1.GetSumLong()); @@ -268,7 +260,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe } Assert.Single(metric2MetricPoints); - var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20, metricPoint2.GetSumLong()); } @@ -306,8 +297,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe } Assert.Single(metric1MetricPoints); - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. var metricPoint1 = metric1MetricPoints[0]; Assert.Equal(10, metricPoint1.GetSumLong()); @@ -318,7 +307,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe } Assert.Single(metric2MetricPoints); - var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20, metricPoint2.GetSumLong()); } @@ -354,8 +342,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe } Assert.Single(metric1MetricPoints); - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. var metricPoint1 = metric1MetricPoints[0]; Assert.Equal(10, metricPoint1.GetSumLong()); @@ -366,7 +352,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe } Assert.Single(metric2MetricPoints); - var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(20D, metricPoint2.GetSumDouble()); } @@ -402,8 +387,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe } Assert.Single(metric1MetricPoints); - - // 1st MetricPoint is reserved for zero-tags. In this test, the metrics are emitted with zero tags. var metricPoint1 = metric1MetricPoints[0]; Assert.Equal(10, metricPoint1.GetSumLong()); @@ -414,7 +397,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe } Assert.Single(metric2MetricPoints); - var metricPoint2 = metric2MetricPoints[0]; Assert.Equal(1, metricPoint2.GetHistogramCount()); Assert.Equal(20D, metricPoint2.GetHistogramSum()); @@ -1160,7 +1142,6 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) }; Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); @@ -1180,7 +1161,6 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); @@ -1253,7 +1233,6 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) }; Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); @@ -1273,7 +1252,6 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(4, GetNumberOfMetricPoints(exportedItems)); - CheckTagsForNthMetricPoint(exportedItems, expectedTagsForFirstMetricPoint, 1); CheckTagsForNthMetricPoint(exportedItems, expectedTagsForSecondMetricPoint, 2); CheckTagsForNthMetricPoint(exportedItems, expectedTagsForThirdMetricPoint, 3); @@ -1702,14 +1680,11 @@ private void MultithreadedHistogramTest(long[] expected, T[] values) metricReader.Collect(); - List metricPoints = new List(); - foreach (var metric in metrics) { foreach (var metricPoint in metric.GetMetricPoints()) { bucketCounts = metricPoint.GetHistogramBuckets().RunningBucketCounts; - break; } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs index 4854152ee9a..d0fa7e945da 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs @@ -63,9 +63,7 @@ public void VerifySnapshot_Counter() // Verify Snapshot 1 Assert.Single(exportedSnapshots); var snapshot1 = exportedSnapshots[0]; - Assert.Single(snapshot1.MetricPoints); - Assert.Equal(10, snapshot1.MetricPoints[0].GetSumLong()); // Verify Metric == Snapshot @@ -138,9 +136,7 @@ public void VerifySnapshot_Histogram() // 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); @@ -190,9 +186,7 @@ public void VerifySnapshot_Histogram() // 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); @@ -237,9 +231,7 @@ public void VerifySnapshot_ExponentialHistogram() // 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); @@ -292,9 +284,7 @@ public void VerifySnapshot_ExponentialHistogram() // 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); From 0178bc9132a15c1e6c24162f83251bac7113f62c Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Thu, 3 Aug 2023 22:41:02 -0700 Subject: [PATCH 05/10] Update CHANGELOG --- src/OpenTelemetry/CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 396db7aee78..27ab35623fc 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,22 @@ ## Unreleased +* **Experimental Feature** Added an opt-in feature to aggregate any metric + measurements that were dropped due to reaching the max MetricPoints limit. + When this feature is enabled, SDK would aggregate such measurements using a + reserved MetricPoint with a single tag with key as `otel.metric.overflow` amd + value as `true`. The feature is turned-off by default. You can enable it by + setting the `AppContext` switch: `OTel.Dotnet.EmitMetricOverflowAttribute` + before setting up the `MeterProvider`. + + ```csharp + AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + + // Setup MeterProvider + ``` + + ([#4737](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4737)) + ## 1.6.0-alpha.1 Released 2023-Jul-12 From a73acd8b7ba47897898195c76ed411d3dfa4db80 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Thu, 3 Aug 2023 22:43:18 -0700 Subject: [PATCH 06/10] Fix whitespace --- src/OpenTelemetry/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 27ab35623fc..f0a4d908061 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -15,7 +15,7 @@ // Setup MeterProvider ``` - + ([#4737](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4737)) ## 1.6.0-alpha.1 From e6890a88eb2c8178797fe255f578083baac67356 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Sat, 5 Aug 2023 17:46:07 -0700 Subject: [PATCH 07/10] Address PR comments --- src/OpenTelemetry/CHANGELOG.md | 4 +- src/OpenTelemetry/Metrics/AggregatorStore.cs | 25 +-- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 8 +- src/OpenTelemetry/Metrics/Metric.cs | 3 +- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 16 +- ...atorTestBase.cs => AggregatorTestsBase.cs} | 26 +-- ...icApiTestBase.cs => MetricApiTestsBase.cs} | 14 +- .../Metrics/MetricOverflowAttributeTests.cs | 202 +++++++++++++++++- .../Metrics/MetricSnapshotTestsBase.cs | 12 +- .../Metrics/MetricTestsBase.cs | 2 + .../Metrics/MetricViewTests.cs | 2 +- 11 files changed, 258 insertions(+), 56 deletions(-) rename test/OpenTelemetry.Tests/Metrics/{AggregatorTestBase.cs => AggregatorTestsBase.cs} (96%) rename test/OpenTelemetry.Tests/Metrics/{MetricApiTestBase.cs => MetricApiTestsBase.cs} (99%) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index f0a4d908061..498d09a3901 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -3,9 +3,9 @@ ## Unreleased * **Experimental Feature** Added an opt-in feature to aggregate any metric - measurements that were dropped due to reaching the max MetricPoints limit. + measurements that were dropped due to reaching the [max MetricPoints limit](https://github.com/open-telemetry/opentelemetry-dotnet/tree/core-1.6.0-alpha.1/docs/metrics/customizing-the-sdk). When this feature is enabled, SDK would aggregate such measurements using a - reserved MetricPoint with a single tag with key as `otel.metric.overflow` amd + reserved MetricPoint with a single tag with key as `otel.metric.overflow` and value as `true`. The feature is turned-off by default. You can enable it by setting the `AppContext` switch: `OTel.Dotnet.EmitMetricOverflowAttribute` before setting up the `MeterProvider`. diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index b09b87635a0..8a1bbb5e4b2 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -22,9 +22,8 @@ namespace OpenTelemetry.Metrics; internal sealed class AggregatorStore { - private static readonly string MetricPointCapHitFixMessage = "Modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit."; + private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit."; private static readonly Comparison> DimensionComparisonDelegate = (x, y) => x.Key.CompareTo(y.Key); - private static bool emitOverflowAttribute; private readonly object lockZeroTags = new(); private readonly object lockOverflowTag = new(); @@ -46,6 +45,7 @@ internal sealed class AggregatorStore private readonly UpdateLongDelegate updateLongCallback; private readonly UpdateDoubleDelegate updateDoubleCallback; private readonly int maxMetricPoints; + private readonly bool emitOverflowAttribute; private readonly ExemplarFilter exemplarFilter; private int metricPointIndex = 0; @@ -59,6 +59,7 @@ internal AggregatorStore( AggregationType aggType, AggregationTemporality temporality, int maxMetricPoints, + bool emitOverflowAttribute, ExemplarFilter exemplarFilter = null) { this.name = metricStreamIdentity.InstrumentName; @@ -87,16 +88,10 @@ internal AggregatorStore( this.tagsKeysInterestingCount = hs.Count; } - // If the switch is not set at all or if it's explicitly set to false - if (AppContext.TryGetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", out bool shouldEmitOverflowAttribute) == false || - shouldEmitOverflowAttribute == false) - { - emitOverflowAttribute = false; - } - else if (this.maxMetricPoints > 1) - { - emitOverflowAttribute = true; + this.emitOverflowAttribute = emitOverflowAttribute; + if (emitOverflowAttribute) + { // Setting metricPointIndex to 1 as we would reserve the metricPoints[1] for overflow attribute. // Newer attributes should be added starting at the index: 2 this.metricPointIndex = 1; @@ -365,7 +360,7 @@ private void UpdateLong(long value, ReadOnlySpan> t var index = this.FindMetricAggregatorsDefault(tags); if (index < 0) { - if (emitOverflowAttribute) + if (this.emitOverflowAttribute) { this.InitializeOverflowTagPointIfNotInitialized(); this.metricPoints[1].Update(value); @@ -406,7 +401,7 @@ private void UpdateLongCustomTags(long value, ReadOnlySpan instrumentations = new(); private readonly List> viewConfigs; private readonly object collectLock = new(); @@ -48,6 +51,9 @@ internal MeterProviderSdk( var state = serviceProvider!.GetRequiredService(); state.RegisterProvider(this); + var config = serviceProvider!.GetRequiredService(); + bool isEmitOverflowAttributeKeySet = config.GetValue(EmitOverFlowAttributeConfigKey, defaultValue: false); + this.ServiceProvider = serviceProvider!; if (ownsServiceProvider) @@ -79,7 +85,7 @@ internal MeterProviderSdk( reader.SetParentProvider(this); reader.SetMaxMetricStreams(state.MaxMetricStreams); - reader.SetMaxMetricPointsPerMetricStream(state.MaxMetricPointsPerMetricStream); + reader.SetMaxMetricPointsPerMetricStream(state.MaxMetricPointsPerMetricStream, isEmitOverflowAttributeKeySet); reader.SetExemplarFilter(state.ExemplarFilter); if (this.reader == null) diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index bdfd73368e7..cc42f96ffc2 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -35,6 +35,7 @@ internal Metric( MetricStreamIdentity instrumentIdentity, AggregationTemporality temporality, int maxMetricPointsPerMetricStream, + bool emitOverflowAttribute, ExemplarFilter exemplarFilter = null) { this.InstrumentIdentity = instrumentIdentity; @@ -141,7 +142,7 @@ internal Metric( throw new NotSupportedException($"Unsupported Instrument Type: {instrumentIdentity.InstrumentType.FullName}"); } - this.aggStore = new AggregatorStore(instrumentIdentity, aggType, temporality, maxMetricPointsPerMetricStream, exemplarFilter); + this.aggStore = new AggregatorStore(instrumentIdentity, aggType, temporality, maxMetricPointsPerMetricStream, emitOverflowAttribute, exemplarFilter); this.Temporality = temporality; this.InstrumentDisposed = false; } diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index fdb2781f99a..f10119dd7cb 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -33,6 +33,7 @@ public abstract partial class MetricReader private Metric[] metrics; private Metric[] metricsCurrentBatch; private int metricIndex = -1; + private bool emitOverflowAttribute; private ExemplarFilter exemplarFilter; @@ -71,7 +72,7 @@ internal Metric AddMetricWithNoViews(Instrument instrument) Metric metric = null; try { - metric = new Metric(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), this.maxMetricPointsPerMetricStream, exemplarFilter: this.exemplarFilter); + metric = new Metric(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), this.maxMetricPointsPerMetricStream, this.emitOverflowAttribute, this.exemplarFilter); } catch (NotSupportedException nse) { @@ -156,7 +157,7 @@ internal List AddMetricsListWithViews(Instrument instrument, List 1) + { + this.emitOverflowAttribute = true; + } + } } private Batch GetMetricsBatch() diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTestBase.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs similarity index 96% rename from test/OpenTelemetry.Tests/Metrics/AggregatorTestBase.cs rename to test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs index 220f1425e77..969c6890a87 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTestBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -21,21 +21,24 @@ namespace OpenTelemetry.Metrics.Tests; #pragma warning disable SA1402 -public abstract class AggregatorTestBase : IDisposable +public abstract class AggregatorTestsBase { private static readonly Meter Meter = new("testMeter"); private static readonly Instrument Instrument = Meter.CreateHistogram("testInstrument"); private static readonly ExplicitBucketHistogramConfiguration HistogramConfiguration = new() { Boundaries = Metric.DefaultHistogramBounds }; private static readonly MetricStreamIdentity MetricStreamIdentity = new(Instrument, HistogramConfiguration); - private readonly AggregatorStore aggregatorStore = new(MetricStreamIdentity, AggregationType.HistogramWithBuckets, AggregationTemporality.Cumulative, 1024); + private readonly bool emitOverflowAttribute; + private readonly AggregatorStore aggregatorStore; - protected AggregatorTestBase(bool emitOverflowAttribute) + protected AggregatorTestsBase(bool emitOverflowAttribute) { if (emitOverflowAttribute) { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + this.emitOverflowAttribute = emitOverflowAttribute; } + + this.aggregatorStore = new(MetricStreamIdentity, AggregationType.HistogramWithBuckets, AggregationTemporality.Cumulative, 1024, emitOverflowAttribute); } [Fact] @@ -235,11 +238,6 @@ public void MultiThreadedHistogramUpdateAndSnapShotTest() Assert.Equal(200, argsToThread.SumOfDelta + lastDelta); } - public void Dispose() - { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); - } - internal static void AssertExponentialBucketsAreCorrect(Base2ExponentialBucketHistogram expectedHistogram, ExponentialHistogramData data) { Assert.Equal(expectedHistogram.Scale, data.Scale); @@ -300,6 +298,7 @@ internal void ExponentialHistogramTests(AggregationType aggregationType, Aggrega aggregationType, aggregationTemporality, maxMetricPoints: 1024, + this.emitOverflowAttribute, exemplarsEnabled ? new AlwaysOnExemplarFilter() : null); var expectedHistogram = new Base2ExponentialBucketHistogram(); @@ -407,7 +406,8 @@ internal void ExponentialMaxScaleConfigWorks(int? maxScale) metricStreamIdentity, AggregationType.Base2ExponentialHistogram, AggregationTemporality.Cumulative, - maxMetricPoints: 1024); + maxMetricPoints: 1024, + this.emitOverflowAttribute); aggregatorStore.Update(10, Array.Empty>()); @@ -480,7 +480,7 @@ private class ThreadArguments } } -public class AggregatorTests : AggregatorTestBase +public class AggregatorTests : AggregatorTestsBase { public AggregatorTests() : base(false) @@ -488,7 +488,7 @@ public AggregatorTests() } } -public class AggregatorTestsWithOverflowAttribute : AggregatorTestBase +public class AggregatorTestsWithOverflowAttribute : AggregatorTestsBase { public AggregatorTestsWithOverflowAttribute() : base(true) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs similarity index 99% rename from test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs rename to test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs index 23ba185046a..bfe2229dcf5 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricApiTestBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -26,7 +26,7 @@ namespace OpenTelemetry.Metrics.Tests; #pragma warning disable SA1402 -public abstract class MetricApiTestBase : MetricTestsBase, IDisposable +public abstract class MetricApiTestsBase : MetricTestsBase, IDisposable { private const int MaxTimeToAllowForFlush = 10000; private static readonly int NumberOfThreads = Environment.ProcessorCount; @@ -35,13 +35,13 @@ public abstract class MetricApiTestBase : MetricTestsBase, IDisposable private static readonly int NumberOfMetricUpdateByEachThread = 100000; private readonly ITestOutputHelper output; - protected MetricApiTestBase(ITestOutputHelper output, bool emitOverflowAttribute) + protected MetricApiTestsBase(ITestOutputHelper output, bool emitOverflowAttribute) { this.output = output; if (emitOverflowAttribute) { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + Environment.SetEnvironmentVariable(EmitOverFlowAttributeConfigKey, "true"); } } @@ -1527,7 +1527,7 @@ public void UnsupportedMetricInstrument() public void Dispose() { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + Environment.SetEnvironmentVariable(EmitOverFlowAttributeConfigKey, null); } private static void CounterUpdateThread(object obj) @@ -1702,7 +1702,7 @@ private class UpdateThreadArguments } } -public class MetricApiTest : MetricApiTestBase +public class MetricApiTest : MetricApiTestsBase { public MetricApiTest(ITestOutputHelper output) : base(output, false) @@ -1710,7 +1710,7 @@ public MetricApiTest(ITestOutputHelper output) } } -public class MetricApiTestWithOverflowAttribute : MetricApiTestBase +public class MetricApiTestWithOverflowAttribute : MetricApiTestsBase { public MetricApiTestWithOverflowAttribute(ITestOutputHelper output) : base(output, true) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs index 1a3957be7fb..98f7fa05392 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs @@ -15,6 +15,9 @@ // using System.Diagnostics.Metrics; +using System.Reflection; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Tests; using Xunit; @@ -22,6 +25,133 @@ namespace OpenTelemetry.Metrics.Tests; public class MetricOverflowAttributeTests { + [Theory] + [InlineData("false", false)] + [InlineData("False", false)] + [InlineData("FALSE", false)] + [InlineData("true", true)] + [InlineData("True", true)] + [InlineData("TRUE", true)] + public void TestEmitOverflowAttributeConfigWithEnvVar(string value, bool isEmitOverflowAttributeKeySet) + { + try + { + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, value); + + var exportedItems = new List(); + + var meter = new Meter(Utils.GetCurrentMethodName()); + var counter = meter.CreateCounter("TestCounter"); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + counter.Add(10); + + meterProvider.ForceFlush(); + + Assert.Single(exportedItems); + var metric = exportedItems[0]; + + var aggregatorStore = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metric) as AggregatorStore; + var emitOverflowAttribute = (bool)typeof(AggregatorStore).GetField("emitOverflowAttribute", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStore); + + Assert.Equal(isEmitOverflowAttributeKeySet, emitOverflowAttribute); + } + finally + { + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, null); + } + } + + [Theory] + [InlineData("false", false)] + [InlineData("False", false)] + [InlineData("FALSE", false)] + [InlineData("true", true)] + [InlineData("True", true)] + [InlineData("TRUE", true)] + public void TestEmitOverflowAttributeConfigWithOtherConfigProvider(string value, bool isEmitOverflowAttributeKeySet) + { + try + { + var exportedItems = new List(); + + var meter = new Meter(Utils.GetCurrentMethodName()); + var counter = meter.CreateCounter("TestCounter"); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .ConfigureServices(services => + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { [MetricTestsBase.EmitOverFlowAttributeConfigKey] = value }) + .Build(); + + services.AddSingleton(configuration); + }) + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + counter.Add(10); + + meterProvider.ForceFlush(); + + Assert.Single(exportedItems); + var metric = exportedItems[0]; + + var aggregatorStore = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metric) as AggregatorStore; + var emitOverflowAttribute = (bool)typeof(AggregatorStore).GetField("emitOverflowAttribute", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStore); + + Assert.Equal(isEmitOverflowAttributeKeySet, emitOverflowAttribute); + } + finally + { + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, null); + } + } + + [Theory] + [InlineData(1, false)] + [InlineData(2, true)] + [InlineData(10, true)] + public void EmitOverflowAttributeIsOnlySetWhenMaxMetricPointsIsGreaterThanOne(int maxMetricPoints, bool isEmitOverflowAttributeKeySet) + { + try + { + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, "true"); + + var exportedItems = new List(); + + var meter = new Meter(Utils.GetCurrentMethodName()); + var counter = meter.CreateCounter("TestCounter"); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .SetMaxMetricPointsPerMetricStream(maxMetricPoints) + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + counter.Add(10); + + meterProvider.ForceFlush(); + + Assert.Single(exportedItems); + var metric = exportedItems[0]; + + var aggregatorStore = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metric) as AggregatorStore; + var emitOverflowAttribute = (bool)typeof(AggregatorStore).GetField("emitOverflowAttribute", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStore); + + Assert.Equal(isEmitOverflowAttributeKeySet, emitOverflowAttribute); + } + finally + { + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, null); + } + } + [Theory] [InlineData(MetricReaderTemporalityPreference.Delta)] [InlineData(MetricReaderTemporalityPreference.Cumulative)] @@ -29,7 +159,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForCounter(MetricReaderTem { try { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, "true"); var exportedItems = new List(); @@ -85,6 +215,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForCounter(MetricReaderTem overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); Assert.Equal(true, overflowMetricPoint.Tags.KeyAndValues[0].Value); + Assert.Equal(1, overflowMetricPoint.Tags.Count); Assert.Equal(5, overflowMetricPoint.GetSumLong()); exportedItems.Clear(); @@ -112,10 +243,37 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForCounter(MetricReaderTem { Assert.Equal(255, overflowMetricPoint.GetSumLong()); // 5 + (50 * 5) } + + exportedItems.Clear(); + metricPoints.Clear(); + + // Test that the SDK continues to correctly aggregate the previously registered measurements even after overflow has occurred + counter.Add(15, new KeyValuePair("Key", 0)); + + meterProvider.ForceFlush(); + metric = exportedItems[0]; + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + var metricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "Key" && (int)mp.Tags.KeyAndValues[0].Value == 0); + + if (temporalityPreference == MetricReaderTemporalityPreference.Delta) + { + Assert.Equal(15, metricPoint.GetSumLong()); + } + else + { + overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + + Assert.Equal(25, metricPoint.GetSumLong()); // 10 + 15 + Assert.Equal(255, overflowMetricPoint.GetSumLong()); + } } finally { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, null); } } @@ -126,7 +284,7 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForHistogram(MetricReaderT { try { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, "true"); var exportedItems = new List(); @@ -182,8 +340,8 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForHistogram(MetricReaderT overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); Assert.Equal(true, overflowMetricPoint.Tags.KeyAndValues[0].Value); - Assert.Equal(5, overflowMetricPoint.GetHistogramSum()); Assert.Equal(1, overflowMetricPoint.GetHistogramCount()); + Assert.Equal(5, overflowMetricPoint.GetHistogramSum()); exportedItems.Clear(); metricPoints.Clear(); @@ -204,18 +362,48 @@ public void MetricOverflowAttributeIsRecordedCorrectlyForHistogram(MetricReaderT overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); if (temporalityPreference == MetricReaderTemporalityPreference.Delta) { - Assert.Equal(250, overflowMetricPoint.GetHistogramSum()); // 50 * 5 Assert.Equal(50, overflowMetricPoint.GetHistogramCount()); + Assert.Equal(250, overflowMetricPoint.GetHistogramSum()); // 50 * 5 } else { - Assert.Equal(255, overflowMetricPoint.GetHistogramSum()); // 5 + (50 * 5) Assert.Equal(51, overflowMetricPoint.GetHistogramCount()); + Assert.Equal(255, overflowMetricPoint.GetHistogramSum()); // 5 + (50 * 5) + } + + exportedItems.Clear(); + metricPoints.Clear(); + + // Test that the SDK continues to correctly aggregate the previously registered measurements even after overflow has occurred + histogram.Record(15, new KeyValuePair("Key", 0)); + + meterProvider.ForceFlush(); + metric = exportedItems[0]; + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + var metricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "Key" && (int)mp.Tags.KeyAndValues[0].Value == 0); + + if (temporalityPreference == MetricReaderTemporalityPreference.Delta) + { + Assert.Equal(1, metricPoint.GetHistogramCount()); + Assert.Equal(15, metricPoint.GetHistogramSum()); + } + else + { + overflowMetricPoint = metricPoints.Single(mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + + Assert.Equal(2, metricPoint.GetHistogramCount()); + Assert.Equal(25, metricPoint.GetHistogramSum()); // 10 + 15 + + Assert.Equal(255, overflowMetricPoint.GetHistogramSum()); } } finally { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, null); } } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs index d0fa7e945da..dc0810fd338 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs @@ -30,7 +30,7 @@ protected MetricSnapshotTestsBase(bool emitOverflowAttribute) { if (emitOverflowAttribute) { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, "true"); } } @@ -226,7 +226,7 @@ public void VerifySnapshot_ExponentialHistogram() metricPoint1.TryGetHistogramMinMaxValues(out var min, out var max); Assert.Equal(10, min); Assert.Equal(10, max); - AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint1.GetExponentialHistogramData()); + AggregatorTestsBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint1.GetExponentialHistogramData()); // Verify Snapshot 1 Assert.Single(exportedSnapshots); @@ -237,7 +237,7 @@ public void VerifySnapshot_ExponentialHistogram() snapshot1.MetricPoints[0].TryGetHistogramMinMaxValues(out min, out max); Assert.Equal(10, min); Assert.Equal(10, max); - AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, snapshot1.MetricPoints[0].GetExponentialHistogramData()); + AggregatorTestsBase.AssertExponentialBucketsAreCorrect(expectedHistogram, snapshot1.MetricPoints[0].GetExponentialHistogramData()); // Verify Metric == Snapshot Assert.Equal(metric1.Name, snapshot1.Name); @@ -271,7 +271,7 @@ public void VerifySnapshot_ExponentialHistogram() metricPoint1.TryGetHistogramMinMaxValues(out min, out max); Assert.Equal(5, min); Assert.Equal(10, max); - AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint2.GetExponentialHistogramData()); + AggregatorTestsBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint2.GetExponentialHistogramData()); // Verify Snapshot 1 after second export // This value is expected to be unchanged. @@ -290,12 +290,12 @@ public void VerifySnapshot_ExponentialHistogram() snapshot2.MetricPoints[0].TryGetHistogramMinMaxValues(out min, out max); Assert.Equal(5, min); Assert.Equal(10, max); - AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, snapshot2.MetricPoints[0].GetExponentialHistogramData()); + AggregatorTestsBase.AssertExponentialBucketsAreCorrect(expectedHistogram, snapshot2.MetricPoints[0].GetExponentialHistogramData()); } public void Dispose() { - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", false); + Environment.SetEnvironmentVariable(MetricTestsBase.EmitOverFlowAttributeConfigKey, null); } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs index 181027e37a2..85c1b2c0817 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs @@ -20,6 +20,8 @@ namespace OpenTelemetry.Metrics.Tests; public class MetricTestsBase { + public const string EmitOverFlowAttributeConfigKey = "OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE"; + public static void ValidateMetricPointTags(List> expectedTags, ReadOnlyTagCollection actualTags) { int tagIndex = 0; diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index d430c9ed886..df98fc6cc2a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -636,7 +636,7 @@ public void ViewToProduceExponentialHistogram() var count = metricPoint.GetHistogramCount(); var sum = metricPoint.GetHistogramSum(); - AggregatorTestBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint.GetExponentialHistogramData()); + AggregatorTestsBase.AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint.GetExponentialHistogramData()); Assert.Equal(50, sum); Assert.Equal(6, count); } From 68aaee951b3d67918fca1724912307fc93509cd8 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Sat, 5 Aug 2023 18:02:15 -0700 Subject: [PATCH 08/10] Update CHANGELOG --- src/OpenTelemetry/CHANGELOG.md | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 498d09a3901..b5f8fb24a5f 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -3,19 +3,14 @@ ## Unreleased * **Experimental Feature** Added an opt-in feature to aggregate any metric - measurements that were dropped due to reaching the [max MetricPoints limit](https://github.com/open-telemetry/opentelemetry-dotnet/tree/core-1.6.0-alpha.1/docs/metrics/customizing-the-sdk). + measurements that were dropped due to reaching the [max MetricPoints + limit](https://github.com/open-telemetry/opentelemetry-dotnet/tree/core-1.6.0-alpha.1/docs/metrics/customizing-the-sdk). When this feature is enabled, SDK would aggregate such measurements using a reserved MetricPoint with a single tag with key as `otel.metric.overflow` and value as `true`. The feature is turned-off by default. You can enable it by - setting the `AppContext` switch: `OTel.Dotnet.EmitMetricOverflowAttribute` - before setting up the `MeterProvider`. - - ```csharp - AppContext.SetSwitch("OTel.Dotnet.EmitMetricOverflowAttribute", true); - - // Setup MeterProvider - ``` - + setting the environment variable + `OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE` to `true` before + setting up the `MeterProvider`. ([#4737](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4737)) ## 1.6.0-alpha.1 From a58631221dad2a789fd25287f801a379270afe35 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Sat, 5 Aug 2023 18:21:16 -0700 Subject: [PATCH 09/10] Fix IConfig issue --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 2 +- src/Shared/Options/ConfigurationExtensions.cs | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index d73d3ba3134..e7cbe4c99f5 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -52,7 +52,7 @@ internal MeterProviderSdk( state.RegisterProvider(this); var config = serviceProvider!.GetRequiredService(); - bool isEmitOverflowAttributeKeySet = config.GetValue(EmitOverFlowAttributeConfigKey, defaultValue: false); + _ = config.TryGetBoolValue(EmitOverFlowAttributeConfigKey, out bool isEmitOverflowAttributeKeySet); this.ServiceProvider = serviceProvider!; diff --git a/src/Shared/Options/ConfigurationExtensions.cs b/src/Shared/Options/ConfigurationExtensions.cs index 7a00025a104..c99e5d8d7d6 100644 --- a/src/Shared/Options/ConfigurationExtensions.cs +++ b/src/Shared/Options/ConfigurationExtensions.cs @@ -95,6 +95,26 @@ public static bool TryGetIntValue( return true; } + public static bool TryGetBoolValue( + this IConfiguration configuration, + string key, + out bool value) + { + if (!configuration.TryGetStringValue(key, out var stringValue)) + { + value = default; + return false; + } + + if (!bool.TryParse(stringValue, out value)) + { + LogInvalidEnvironmentVariable?.Invoke(key, stringValue!); + return false; + } + + return true; + } + public static bool TryGetValue( this IConfiguration configuration, string key, From 4ff089a4caea86a6544e936f98f53738fea57d3f Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Mon, 7 Aug 2023 16:01:59 -0700 Subject: [PATCH 10/10] Update src/OpenTelemetry/Metrics/AggregatorStore.cs Co-authored-by: Cijo Thomas --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 8a1bbb5e4b2..489f3b63c29 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -22,7 +22,7 @@ namespace OpenTelemetry.Metrics; internal sealed class AggregatorStore { - private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit."; + private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute by setting env variable OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE = true. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit."; private static readonly Comparison> DimensionComparisonDelegate = (x, y) => x.Key.CompareTo(y.Key); private readonly object lockZeroTags = new();