From 2823545d66f726c8eeefacf5bc8775a92d34b16e Mon Sep 17 00:00:00 2001 From: Kevin Coulombe Date: Wed, 20 Nov 2024 23:19:41 -0500 Subject: [PATCH 1/5] Handle meter disposal cleanly --- .../Implementation/MetricItemExtensions.cs | 69 +++++++++++++++++++ src/OpenTelemetry/Metrics/Metric.cs | 2 + .../Metrics/Reader/MetricReaderExt.cs | 55 ++++++++------- 3 files changed, 98 insertions(+), 28 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index ea014ec5b29..0a58c103544 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -169,6 +169,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } sum.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + sum.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Sum = sum; @@ -206,6 +211,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } sum.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + sum.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Sum = sum; @@ -237,6 +247,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } gauge.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + gauge.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Gauge = gauge; @@ -268,6 +283,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } gauge.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + gauge.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Gauge = gauge; @@ -318,6 +338,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } histogram.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + histogram.DataPoints.Add(CreateNoRecordedValueHistogramDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Histogram = histogram; @@ -370,6 +395,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } histogram.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + histogram.DataPoints.Add(CreateNoRecordedValueExponentialHistogramDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.ExponentialHistogram = histogram; @@ -424,6 +454,45 @@ internal static OtlpMetrics.Exemplar ToOtlpExemplar(T value, in Metrics.Exemp return otlpExemplar; } + private static NumberDataPoint CreateNoRecordedValueNumberDataPoint(ulong timestamp, ReadOnlyTagCollection tags) + { + var lastDataPoint = new NumberDataPoint + { + StartTimeUnixNano = timestamp, + TimeUnixNano = timestamp, + Flags = (uint)DataPointFlags.NoRecordedValueMask, + }; + + AddAttributes(tags, lastDataPoint.Attributes); + return lastDataPoint; + } + + private static HistogramDataPoint CreateNoRecordedValueHistogramDataPoint(ulong timestamp, ReadOnlyTagCollection tags) + { + var lastDataPoint = new HistogramDataPoint + { + StartTimeUnixNano = timestamp, + TimeUnixNano = timestamp, + Flags = (uint)DataPointFlags.NoRecordedValueMask, + }; + + AddAttributes(tags, lastDataPoint.Attributes); + return lastDataPoint; + } + + private static ExponentialHistogramDataPoint CreateNoRecordedValueExponentialHistogramDataPoint(ulong timestamp, ReadOnlyTagCollection tags) + { + var lastDataPoint = new ExponentialHistogramDataPoint() + { + StartTimeUnixNano = timestamp, + TimeUnixNano = timestamp, + Flags = (uint)DataPointFlags.NoRecordedValueMask, + }; + + AddAttributes(tags, lastDataPoint.Attributes); + return lastDataPoint; + } + private static void AddAttributes(ReadOnlyTagCollection tags, RepeatedField attributes) { foreach (var tag in tags) diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index ac73955dda6..c4e6fe30fdd 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -243,6 +243,8 @@ internal Metric( internal bool Active { get; set; } = true; + internal bool NoRecordedValueNeeded { get; set; } + /// /// Get the metric points for the metric stream. /// diff --git a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs index 6074dd072f6..dc457839b29 100644 --- a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs @@ -197,8 +197,6 @@ private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metr { if (!this.metricStreamNames.Add(metricStreamIdentity.MetricStreamName)) { - // TODO: If a metric is deactivated and then reactivated we log the - // same warning as if it was a duplicate. OpenTelemetrySdkEventSource.Log.DuplicateMetricInstrument( metricStreamIdentity.InstrumentName, metricStreamIdentity.MeterName, @@ -231,7 +229,33 @@ private Batch GetMetricsBatch() if (!metric.Active) { - this.RemoveMetric(ref metric); + // Inactive metrics are sent one last time so the remaining data points and + // NoRecordedValue data points can be sent. The Active property might be + // set to false between collection cycles, so a separate property must be + // used to avoid duplicate staleness markers. + metric.NoRecordedValueNeeded = true; + + lock (this.instrumentCreationLock) + { + OpenTelemetrySdkEventSource.Log.MetricInstrumentRemoved(metric.Name, metric.MeterName); + + // Note: This is using TryUpdate and NOT TryRemove because there is a + // race condition. If a metric is deactivated and then reactivated in + // the same collection cycle + // instrumentIdentityToMetric[metric.InstrumentIdentity] may already + // point to the new activated metric and not the old deactivated one. + this.instrumentIdentityToMetric.TryUpdate(metric.InstrumentIdentity, null, metric); + + this.metricStreamNames.Remove(metric.InstrumentIdentity.MetricStreamName); + + // Defragment metrics list so storage can be reused on future metrics. + for (int j = i + 1; j < target; j++) + { + this.metrics[j - 1] = this.metrics[j]; + } + + this.metricIndex--; + } } } } @@ -244,29 +268,4 @@ private Batch GetMetricsBatch() return default; } } - - private void RemoveMetric(ref Metric? metric) - { - Debug.Assert(metric != null, "metric was null"); - - // TODO: This logic removes the metric. If the same - // metric is published again we will create a new metric - // for it. If this happens often we will run out of - // storage. Instead, should we keep the metric around - // and set a new start time + reset its data if it comes - // back? - - OpenTelemetrySdkEventSource.Log.MetricInstrumentRemoved(metric!.Name, metric.MeterName); - - // Note: This is using TryUpdate and NOT TryRemove because there is a - // race condition. If a metric is deactivated and then reactivated in - // the same collection cycle - // instrumentIdentityToMetric[metric.InstrumentIdentity] may already - // point to the new activated metric and not the old deactivated one. - this.instrumentIdentityToMetric.TryUpdate(metric.InstrumentIdentity, null, metric); - - // Note: metric is a reference to the array storage so - // this clears the metric out of the array. - metric = null; - } } From c9d547590b1079764ac975f37b6ada2d31105bcf Mon Sep 17 00:00:00 2001 From: Kevin Coulombe Date: Fri, 22 Nov 2024 09:31:39 -0500 Subject: [PATCH 2/5] Fix test and issue with list defrag --- src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs | 2 ++ .../OpenTelemetryMetricsBuilderExtensionsTests.cs | 5 ++--- .../Metrics/MeterProviderSdkTest.cs | 13 ++++++++++--- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs index dc457839b29..1d46646a550 100644 --- a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs @@ -254,7 +254,9 @@ private Batch GetMetricsBatch() this.metrics[j - 1] = this.metrics[j]; } + this.metrics[target - 1] = null; this.metricIndex--; + i--; } } } diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs index d34dc060f1e..9b5dc35a786 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs @@ -140,8 +140,7 @@ public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMe var duplicateMetricInstrumentEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 38); - // Note: We currently log a duplicate warning anytime a metric is reactivated. - Assert.Single(duplicateMetricInstrumentEvents); + Assert.Empty(duplicateMetricInstrumentEvents); var metricInstrumentDeactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 52); @@ -211,7 +210,7 @@ public void ReloadOfMetricsViaIConfigurationWithoutExportCleanupTest(bool useWit var duplicateMetricInstrumentEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 38); - // Note: We currently log a duplicate warning anytime a metric is reactivated. + // Note: The old metric is only removed after a flush, so both duplicates lived for one collection cycle. Assert.Single(duplicateMetricInstrumentEvents); var metricInstrumentDeactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 52); diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs index 83d5d4644cf..f688f03548d 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs @@ -41,7 +41,7 @@ public void BuilderTypeDoesNotChangeTest() [InlineData(true, true)] [InlineData(false, false)] [InlineData(true, false)] - public void TransientMeterExhaustsMetricStorageTest(bool withView, bool forceFlushAfterEachTest) + public void TransientMeterBetweenCollectionsExhaustsMetricStorageTest(bool withView, bool forceFlushAfterEachTest) { using var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log); @@ -74,7 +74,7 @@ public void TransientMeterExhaustsMetricStorageTest(bool withView, bool forceFlu if (forceFlushAfterEachTest) { - Assert.Empty(exportedItems); + Assert.Single(exportedItems); } else { @@ -85,7 +85,14 @@ public void TransientMeterExhaustsMetricStorageTest(bool withView, bool forceFlu var metricInstrumentIgnoredEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 33 && (e.Payload?.Count ?? 0) >= 2 && e.Payload![1] as string == meterName); - Assert.Single(metricInstrumentIgnoredEvents); + if (forceFlushAfterEachTest) + { + Assert.Empty(metricInstrumentIgnoredEvents); + } + else + { + Assert.Single(metricInstrumentIgnoredEvents); + } void RunTest() { From e4e377e37f73b16abd1c811091cc473ebcb7c56b Mon Sep 17 00:00:00 2001 From: Kevin Coulombe Date: Fri, 22 Nov 2024 15:06:00 -0500 Subject: [PATCH 3/5] Added some tests for no recorded value in OTLP serialization when disposing the meter --- .../OtlpMetricsExporterTests.cs | 435 +++++++++++++----- 1 file changed, 308 insertions(+), 127 deletions(-) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs index f34bc572126..1c1635376bf 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs @@ -247,7 +247,8 @@ public void ToOtlpResourceMetricsTest(bool useCustomSerializer, bool includeServ [InlineData(false, "test_gauge", null, null, 123L, null, true)] [InlineData(false, "test_gauge", null, null, null, 123.45, true)] [InlineData(false, "test_gauge", "description", "unit", 123L, null)] - public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, bool enableExemplars = false) + [InlineData(false, "test_gauge", "description", "unit", 123L, null, false, true)] + public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, bool enableExemplars = false, bool disposeMeterEarly = false) { var metrics = new List(); @@ -267,6 +268,14 @@ public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? meter.CreateObservableGauge(name, () => doubleValue!.Value, unit, description); } + if (disposeMeterEarly) + { + // For observable instruments, disposing the meter before the first collection leaves the metric with no data at all. + provider.ForceFlush(); + metrics.Clear(); + meter.Dispose(); + } + provider.ForceFlush(); var batch = new Batch(metrics.ToArray(), metrics.Count); @@ -298,25 +307,54 @@ public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? Assert.Null(actual.ExponentialHistogram); Assert.Null(actual.Summary); - Assert.Single(actual.Gauge.DataPoints); - var dataPoint = actual.Gauge.DataPoints.First(); - Assert.True(dataPoint.StartTimeUnixNano > 0); - Assert.True(dataPoint.TimeUnixNano > 0); - - if (longValue.HasValue) + if (!disposeMeterEarly) { - Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsInt, dataPoint.ValueCase); - Assert.Equal(longValue, dataPoint.AsInt); + Assert.Single(actual.Gauge.DataPoints); } else { - Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsDouble, dataPoint.ValueCase); - Assert.Equal(doubleValue, dataPoint.AsDouble); + Assert.Equal(2, actual.Gauge.DataPoints.Count); } - Assert.Empty(dataPoint.Attributes); + for (var index = 0; index < actual.Gauge.DataPoints.Count; index++) + { + var dataPoint = actual.Gauge.DataPoints[index]; + bool isNoRecordedValueDataPoint = index == 1; + + Assert.True(dataPoint.StartTimeUnixNano > 0); + Assert.True(dataPoint.TimeUnixNano > 0); + + if (!isNoRecordedValueDataPoint) + { + if (longValue.HasValue) + { + Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsInt, dataPoint.ValueCase); + Assert.Equal(longValue, dataPoint.AsInt); + } + else + { + Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsDouble, dataPoint.ValueCase); + Assert.Equal(doubleValue, dataPoint.AsDouble); + } - VerifyExemplars(longValue, doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + Assert.Equal((uint)OtlpMetrics.DataPointFlags.DoNotUse, dataPoint.Flags); + } + else + { + Assert.Equal((uint)OtlpMetrics.DataPointFlags.NoRecordedValueMask, dataPoint.Flags); + } + + Assert.Empty(dataPoint.Attributes); + + if (!isNoRecordedValueDataPoint) + { + VerifyExemplars(longValue, doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + } + else + { + Assert.Empty(dataPoint.Exemplars); + } + } } [Theory] @@ -341,7 +379,8 @@ public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? [InlineData(false, "test_counter", null, null, null, 123.45, MetricReaderTemporalityPreference.Delta, false, true)] [InlineData(false, "test_counter", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)] [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, true)] - public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false) + [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true)] + public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false) { var metrics = new List(); @@ -367,6 +406,11 @@ public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, strin counter.Add(doubleValue!.Value, attributes); } + if (disposeMeterEarly) + { + meter.Dispose(); + } + provider.ForceFlush(); var batch = new Batch(metrics.ToArray(), metrics.Count); @@ -405,32 +449,61 @@ public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, strin : OtlpMetrics.AggregationTemporality.Delta; Assert.Equal(otlpAggregationTemporality, actual.Sum.AggregationTemporality); - Assert.Single(actual.Sum.DataPoints); - var dataPoint = actual.Sum.DataPoints.First(); - Assert.True(dataPoint.StartTimeUnixNano > 0); - Assert.True(dataPoint.TimeUnixNano > 0); - - if (longValue.HasValue) + if (!disposeMeterEarly) { - Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsInt, dataPoint.ValueCase); - Assert.Equal(longValue, dataPoint.AsInt); + Assert.Single(actual.Sum.DataPoints); } else { - Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsDouble, dataPoint.ValueCase); - Assert.Equal(doubleValue, dataPoint.AsDouble); + Assert.Equal(2, actual.Sum.DataPoints.Count); } - if (attributes.Length > 0) - { - OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes); - } - else + for (var index = 0; index < actual.Sum.DataPoints.Count; index++) { - Assert.Empty(dataPoint.Attributes); - } + var dataPoint = actual.Sum.DataPoints[index]; + bool isNoRecordedValueDataPoint = index == 1; + + Assert.True(dataPoint.StartTimeUnixNano > 0); + Assert.True(dataPoint.TimeUnixNano > 0); + + if (!isNoRecordedValueDataPoint) + { + if (longValue.HasValue) + { + Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsInt, dataPoint.ValueCase); + Assert.Equal(longValue, dataPoint.AsInt); + } + else + { + Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsDouble, dataPoint.ValueCase); + Assert.Equal(doubleValue, dataPoint.AsDouble); + } + + Assert.Equal((uint)OtlpMetrics.DataPointFlags.DoNotUse, dataPoint.Flags); + } + else + { + Assert.Equal((uint)OtlpMetrics.DataPointFlags.NoRecordedValueMask, dataPoint.Flags); + } - VerifyExemplars(longValue, doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + if (attributes.Length > 0) + { + OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes); + } + else + { + Assert.Empty(dataPoint.Attributes); + } + + if (!isNoRecordedValueDataPoint) + { + VerifyExemplars(longValue, doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + } + else + { + Assert.Empty(dataPoint.Exemplars); + } + } } [Theory] @@ -459,7 +532,8 @@ public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, strin [InlineData(false, "test_counter", null, null, null, -123.45, MetricReaderTemporalityPreference.Delta, false, true)] [InlineData(false, "test_counter", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)] [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, true)] - public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false) + [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true)] + public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false) { var metrics = new List(); @@ -485,6 +559,11 @@ public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, counter.Add(doubleValue!.Value, attributes); } + if (disposeMeterEarly) + { + meter.Dispose(); + } + provider.ForceFlush(); var batch = new Batch(metrics.ToArray(), metrics.Count); @@ -522,32 +601,61 @@ public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, : OtlpMetrics.AggregationTemporality.Cumulative; Assert.Equal(otlpAggregationTemporality, actual.Sum.AggregationTemporality); - Assert.Single(actual.Sum.DataPoints); - var dataPoint = actual.Sum.DataPoints.First(); - Assert.True(dataPoint.StartTimeUnixNano > 0); - Assert.True(dataPoint.TimeUnixNano > 0); - - if (longValue.HasValue) + if (!disposeMeterEarly) { - Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsInt, dataPoint.ValueCase); - Assert.Equal(longValue, dataPoint.AsInt); + Assert.Single(actual.Sum.DataPoints); } else { - Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsDouble, dataPoint.ValueCase); - Assert.Equal(doubleValue, dataPoint.AsDouble); + Assert.Equal(2, actual.Sum.DataPoints.Count); } - if (attributes.Length > 0) + for (var index = 0; index < actual.Sum.DataPoints.Count; index++) { - OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes); - } - else - { - Assert.Empty(dataPoint.Attributes); - } + var dataPoint = actual.Sum.DataPoints[index]; + bool isNoRecordedValueDataPoint = index == 1; + + Assert.True(dataPoint.StartTimeUnixNano > 0); + Assert.True(dataPoint.TimeUnixNano > 0); + + if (!isNoRecordedValueDataPoint) + { + if (longValue.HasValue) + { + Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsInt, dataPoint.ValueCase); + Assert.Equal(longValue, dataPoint.AsInt); + } + else + { + Assert.Equal(OtlpMetrics.NumberDataPoint.ValueOneofCase.AsDouble, dataPoint.ValueCase); + Assert.Equal(doubleValue, dataPoint.AsDouble); + } + + Assert.Equal((uint)OtlpMetrics.DataPointFlags.DoNotUse, dataPoint.Flags); + } + else + { + Assert.Equal((uint)OtlpMetrics.DataPointFlags.NoRecordedValueMask, dataPoint.Flags); + } + + if (attributes.Length > 0) + { + OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes); + } + else + { + Assert.Empty(dataPoint.Attributes); + } - VerifyExemplars(longValue, doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + if (!isNoRecordedValueDataPoint) + { + VerifyExemplars(longValue, doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + } + else + { + Assert.Empty(dataPoint.Exemplars); + } + } } [Theory] @@ -576,7 +684,8 @@ public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, [InlineData(false, "test_histogram", null, null, null, 123.45, MetricReaderTemporalityPreference.Delta, false, true)] [InlineData(false, "test_histogram", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)] [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, true)] - public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false) + [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true)] + public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false) { var metrics = new List(); @@ -608,6 +717,11 @@ public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, strin histogram.Record(0, attributes); } + if (disposeMeterEarly) + { + meter.Dispose(); + } + provider.ForceFlush(); var batch = new Batch(metrics.ToArray(), metrics.Count); @@ -643,70 +757,100 @@ public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, strin : OtlpMetrics.AggregationTemporality.Delta; Assert.Equal(otlpAggregationTemporality, actual.ExponentialHistogram.AggregationTemporality); - Assert.Single(actual.ExponentialHistogram.DataPoints); - var dataPoint = actual.ExponentialHistogram.DataPoints.First(); - Assert.True(dataPoint.StartTimeUnixNano > 0); - Assert.True(dataPoint.TimeUnixNano > 0); - - Assert.Equal(20, dataPoint.Scale); - Assert.Equal(1UL, dataPoint.ZeroCount); - if (longValue > 0 || doubleValue > 0) + if (!disposeMeterEarly) { - Assert.Equal(2UL, dataPoint.Count); + Assert.Single(actual.ExponentialHistogram.DataPoints); } else { - Assert.Equal(1UL, dataPoint.Count); + Assert.Equal(2, actual.ExponentialHistogram.DataPoints.Count); } - if (longValue.HasValue) + for (var index = 0; index < actual.ExponentialHistogram.DataPoints.Count; index++) { - if (longValue > 0) + var dataPoint = actual.ExponentialHistogram.DataPoints[index]; + bool isNoRecordedValueDataPoint = index == 1; + + Assert.True(dataPoint.StartTimeUnixNano > 0); + Assert.True(dataPoint.TimeUnixNano > 0); + + if (!isNoRecordedValueDataPoint) { - Assert.Equal((double)longValue, dataPoint.Sum); - Assert.Null(dataPoint.Negative); - Assert.True(dataPoint.Positive.Offset > 0); - Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]); + Assert.Equal(20, dataPoint.Scale); + Assert.Equal(1UL, dataPoint.ZeroCount); + if (longValue > 0 || doubleValue > 0) + { + Assert.Equal(2UL, dataPoint.Count); + } + else + { + Assert.Equal(1UL, dataPoint.Count); + } + + if (longValue.HasValue) + { + if (longValue > 0) + { + Assert.Equal((double)longValue, dataPoint.Sum); + Assert.Null(dataPoint.Negative); + Assert.True(dataPoint.Positive.Offset > 0); + Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]); + } + else + { + Assert.Equal(0, dataPoint.Sum); + Assert.Null(dataPoint.Negative); + Assert.Equal(0, dataPoint.Positive.Offset); + Assert.Empty(dataPoint.Positive.BucketCounts); + } + } + else + { + if (doubleValue > 0) + { + Assert.Equal(doubleValue, dataPoint.Sum); + Assert.Null(dataPoint.Negative); + Assert.True(dataPoint.Positive.Offset > 0); + Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]); + } + else + { + Assert.Equal(0, dataPoint.Sum); + Assert.Null(dataPoint.Negative); + Assert.Equal(0, dataPoint.Positive.Offset); + Assert.Empty(dataPoint.Positive.BucketCounts); + } + } + + Assert.Equal((uint)OtlpMetrics.DataPointFlags.DoNotUse, dataPoint.Flags); } else { - Assert.Equal(0, dataPoint.Sum); - Assert.Null(dataPoint.Negative); - Assert.Equal(0, dataPoint.Positive.Offset); - Assert.Empty(dataPoint.Positive.BucketCounts); + Assert.Equal(0UL, dataPoint.Count); + Assert.Equal((uint)OtlpMetrics.DataPointFlags.NoRecordedValueMask, dataPoint.Flags); } - } - else - { - if (doubleValue > 0) + + if (attributes.Length > 0) { - Assert.Equal(doubleValue, dataPoint.Sum); - Assert.Null(dataPoint.Negative); - Assert.True(dataPoint.Positive.Offset > 0); - Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]); + OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes); } else { - Assert.Equal(0, dataPoint.Sum); - Assert.Null(dataPoint.Negative); - Assert.Equal(0, dataPoint.Positive.Offset); - Assert.Empty(dataPoint.Positive.BucketCounts); + Assert.Empty(dataPoint.Attributes); } - } - - if (attributes.Length > 0) - { - OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes); - } - else - { - Assert.Empty(dataPoint.Attributes); - } - VerifyExemplars(null, longValue ?? doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); - if (enableExemplars) - { - VerifyExemplars(null, 0, enableExemplars, d => d.Exemplars.Skip(1).FirstOrDefault(), dataPoint); + if (!isNoRecordedValueDataPoint) + { + VerifyExemplars(null, longValue ?? doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + if (enableExemplars) + { + VerifyExemplars(null, 0, enableExemplars, d => d.Exemplars.Skip(1).FirstOrDefault(), dataPoint); + } + } + else + { + Assert.Empty(dataPoint.Exemplars); + } } } @@ -736,11 +880,13 @@ public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, strin [InlineData(false, "test_histogram", null, null, null, 123.45, MetricReaderTemporalityPreference.Delta, false, true)] [InlineData(false, "test_histogram", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)] [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, true)] - public void TestHistogramToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false) + [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true)] + public void TestHistogramToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false) { var metrics = new List(); using var meter = new Meter(Utils.GetCurrentMethodName()); + using var provider = Sdk.CreateMeterProviderBuilder() .AddMeter(meter.Name) .SetExemplarFilter(enableExemplars ? ExemplarFilterType.AlwaysOn : ExemplarFilterType.AlwaysOff) @@ -762,6 +908,11 @@ public void TestHistogramToOtlpMetric(bool useCustomSerializer, string name, str histogram.Record(doubleValue!.Value, attributes); } + if (disposeMeterEarly) + { + meter.Dispose(); + } + provider.ForceFlush(); var batch = new Batch(metrics.ToArray(), metrics.Count); @@ -797,46 +948,76 @@ public void TestHistogramToOtlpMetric(bool useCustomSerializer, string name, str : OtlpMetrics.AggregationTemporality.Delta; Assert.Equal(otlpAggregationTemporality, actual.Histogram.AggregationTemporality); - Assert.Single(actual.Histogram.DataPoints); - var dataPoint = actual.Histogram.DataPoints.First(); - Assert.True(dataPoint.StartTimeUnixNano > 0); - Assert.True(dataPoint.TimeUnixNano > 0); - - Assert.Equal(1UL, dataPoint.Count); - - // Known issue: Negative measurements affect the Sum. Per the spec, they should not. - if (longValue.HasValue) + if (!disposeMeterEarly) { - Assert.Equal((double)longValue, dataPoint.Sum); + Assert.Single(actual.Histogram.DataPoints); } else { - Assert.Equal(doubleValue, dataPoint.Sum); + Assert.Equal(2, actual.Histogram.DataPoints.Count); } - int bucketIndex; - for (bucketIndex = 0; bucketIndex < dataPoint.ExplicitBounds.Count; ++bucketIndex) + for (var index = 0; index < actual.Histogram.DataPoints.Count; index++) { - if (dataPoint.Sum <= dataPoint.ExplicitBounds[bucketIndex]) + var dataPoint = actual.Histogram.DataPoints[index]; + bool isNoRecordedValueDataPoint = index == 1; + + Assert.True(dataPoint.StartTimeUnixNano > 0); + Assert.True(dataPoint.TimeUnixNano > 0); + + if (!isNoRecordedValueDataPoint) { - break; - } + Assert.Equal(1UL, dataPoint.Count); - Assert.Equal(0UL, dataPoint.BucketCounts[bucketIndex]); - } + // Known issue: Negative measurements affect the Sum. Per the spec, they should not. + if (longValue.HasValue) + { + Assert.Equal((double)longValue, dataPoint.Sum); + } + else + { + Assert.Equal(doubleValue, dataPoint.Sum); + } - Assert.Equal(1UL, dataPoint.BucketCounts[bucketIndex]); + int bucketIndex; + for (bucketIndex = 0; bucketIndex < dataPoint.ExplicitBounds.Count; ++bucketIndex) + { + if (dataPoint.Sum <= dataPoint.ExplicitBounds[bucketIndex]) + { + break; + } - if (attributes.Length > 0) - { - OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes); - } - else - { - Assert.Empty(dataPoint.Attributes); - } + Assert.Equal(0UL, dataPoint.BucketCounts[bucketIndex]); + } + + Assert.Equal(1UL, dataPoint.BucketCounts[bucketIndex]); + + Assert.Equal((uint)OtlpMetrics.DataPointFlags.DoNotUse, dataPoint.Flags); + } + else + { + Assert.Equal(0UL, dataPoint.Count); + Assert.Equal((uint)OtlpMetrics.DataPointFlags.NoRecordedValueMask, dataPoint.Flags); + } + + if (attributes.Length > 0) + { + OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes); + } + else + { + Assert.Empty(dataPoint.Attributes); + } - VerifyExemplars(null, longValue ?? doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + if (!isNoRecordedValueDataPoint) + { + VerifyExemplars(null, longValue ?? doubleValue, enableExemplars, d => d.Exemplars.FirstOrDefault(), dataPoint); + } + else + { + Assert.Empty(dataPoint.Exemplars); + } + } } [Theory] From 8ab84afe71e299fda0827e7f847a6b4d34a01c53 Mon Sep 17 00:00:00 2001 From: Kevin Coulombe Date: Fri, 29 Nov 2024 13:56:37 -0500 Subject: [PATCH 4/5] Added feature flag OTEL_DOTNET_EXPERIMENTAL_OTLP_METRICS_EMIT_NO_RECORDED_VALUE to only emil no recorded value data points as a opt-in feature --- .../Implementation/ExperimentalOptions.cs | 13 +++++++ .../Implementation/MetricItemExtensions.cs | 19 +++++----- .../OtlpMetricExporter.cs | 4 ++- .../OtlpMetricsExporterTests.cs | 35 +++++++++++-------- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExperimentalOptions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExperimentalOptions.cs index f86743325db..98ccbc00fb8 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExperimentalOptions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExperimentalOptions.cs @@ -19,6 +19,8 @@ internal sealed class ExperimentalOptions public const string OtlpUseCustomSerializer = "OTEL_DOTNET_EXPERIMENTAL_USE_CUSTOM_PROTOBUF_SERIALIZER"; + public const string EmitNoRecordedValueNeededDataPointsEnvVar = "OTEL_DOTNET_EXPERIMENTAL_OTLP_METRICS_EMIT_NO_RECORDED_VALUE"; + public ExperimentalOptions() : this(new ConfigurationBuilder().AddEnvironmentVariables().Build()) { @@ -36,6 +38,11 @@ public ExperimentalOptions(IConfiguration configuration) this.UseCustomProtobufSerializer = useCustomSerializer; } + if (configuration.TryGetBoolValue(OpenTelemetryProtocolExporterEventSource.Log, EmitNoRecordedValueNeededDataPointsEnvVar, out var emitNoRecordedValueNeededDataPoints)) + { + this.EmitNoRecordedValueNeededDataPoints = emitNoRecordedValueNeededDataPoints; + } + if (configuration.TryGetStringValue(OtlpRetryEnvVar, out var retryPolicy) && retryPolicy != null) { if (retryPolicy.Equals("in_memory", StringComparison.OrdinalIgnoreCase)) @@ -90,4 +97,10 @@ public ExperimentalOptions(IConfiguration configuration) /// Gets a value indicating whether custom serializer should be used for OTLP export. /// public bool UseCustomProtobufSerializer { get; } + + /// + /// Gets a value indicating whether the NoRecordedValue measurement should be sent when metrics are removed, + /// e.g when disposing a Meter. + /// + public bool EmitNoRecordedValueNeededDataPoints { get; } } diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index 0a58c103544..2759a244cae 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -24,7 +24,8 @@ internal static class MetricItemExtensions internal static void AddMetrics( this OtlpCollector.ExportMetricsServiceRequest request, OtlpResource.Resource processResource, - in Batch metrics) + in Batch metrics, + bool experimentalEmitNoRecordedValueNeededDataPoints = false) { var metricsByLibrary = new Dictionary(); var resourceMetrics = new ResourceMetrics @@ -35,7 +36,7 @@ internal static void AddMetrics( foreach (var metric in metrics) { - var otlpMetric = metric.ToOtlpMetric(); + var otlpMetric = metric.ToOtlpMetric(experimentalEmitNoRecordedValueNeededDataPoints); // TODO: Replace null check with exception handling. if (otlpMetric == null) @@ -109,7 +110,7 @@ internal static ScopeMetrics GetMetricListFromPool(string name, string version, } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) + internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric, bool experimentalEmitNoRecordedValueNeededDataPoints) { var otlpMetric = new OtlpMetrics.Metric { @@ -170,7 +171,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) sum.DataPoints.Add(dataPoint); - if (metric.NoRecordedValueNeeded) + if (experimentalEmitNoRecordedValueNeededDataPoints && metric.NoRecordedValueNeeded) { sum.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); } @@ -212,7 +213,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) sum.DataPoints.Add(dataPoint); - if (metric.NoRecordedValueNeeded) + if (experimentalEmitNoRecordedValueNeededDataPoints && metric.NoRecordedValueNeeded) { sum.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); } @@ -248,7 +249,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) gauge.DataPoints.Add(dataPoint); - if (metric.NoRecordedValueNeeded) + if (experimentalEmitNoRecordedValueNeededDataPoints && metric.NoRecordedValueNeeded) { gauge.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); } @@ -284,7 +285,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) gauge.DataPoints.Add(dataPoint); - if (metric.NoRecordedValueNeeded) + if (experimentalEmitNoRecordedValueNeededDataPoints && metric.NoRecordedValueNeeded) { gauge.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); } @@ -339,7 +340,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) histogram.DataPoints.Add(dataPoint); - if (metric.NoRecordedValueNeeded) + if (experimentalEmitNoRecordedValueNeededDataPoints && metric.NoRecordedValueNeeded) { histogram.DataPoints.Add(CreateNoRecordedValueHistogramDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); } @@ -396,7 +397,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) histogram.DataPoints.Add(dataPoint); - if (metric.NoRecordedValueNeeded) + if (experimentalEmitNoRecordedValueNeededDataPoints && metric.NoRecordedValueNeeded) { histogram.DataPoints.Add(CreateNoRecordedValueExponentialHistogramDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); } diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporter.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporter.cs index aef2bdc4fe3..6e88dd11987 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporter.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporter.cs @@ -18,6 +18,7 @@ public class OtlpMetricExporter : BaseExporter { private readonly OtlpExporterTransmissionHandler transmissionHandler; + private readonly bool emitNoRecordedValueNeededDataPoints; private OtlpResource.Resource? processResource; /// @@ -44,6 +45,7 @@ internal OtlpMetricExporter( Debug.Assert(experimentalOptions != null, "experimentalOptions was null"); this.transmissionHandler = transmissionHandler ?? exporterOptions!.GetMetricsExportTransmissionHandler(experimentalOptions!); + this.emitNoRecordedValueNeededDataPoints = experimentalOptions!.EmitNoRecordedValueNeededDataPoints; } internal OtlpResource.Resource ProcessResource => this.processResource ??= this.ParentProvider.GetResource().ToOtlpResource(); @@ -58,7 +60,7 @@ public override ExportResult Export(in Batch metrics) try { - request.AddMetrics(this.ProcessResource, metrics); + request.AddMetrics(this.ProcessResource, metrics, this.emitNoRecordedValueNeededDataPoints); if (!this.transmissionHandler.TrySubmitRequest(request)) { diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs index 1c1635376bf..6bb70e8e601 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs @@ -248,7 +248,8 @@ public void ToOtlpResourceMetricsTest(bool useCustomSerializer, bool includeServ [InlineData(false, "test_gauge", null, null, null, 123.45, true)] [InlineData(false, "test_gauge", "description", "unit", 123L, null)] [InlineData(false, "test_gauge", "description", "unit", 123L, null, false, true)] - public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, bool enableExemplars = false, bool disposeMeterEarly = false) + [InlineData(false, "test_gauge", "description", "unit", 123L, null, false, true, true)] + public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, bool enableExemplars = false, bool disposeMeterEarly = false, bool experimentalEmitNoRecordedValue = false) { var metrics = new List(); @@ -288,7 +289,7 @@ public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? } else { - request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch); + request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch, experimentalEmitNoRecordedValue); } var resourceMetric = request.ResourceMetrics.Single(); @@ -307,7 +308,7 @@ public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? Assert.Null(actual.ExponentialHistogram); Assert.Null(actual.Summary); - if (!disposeMeterEarly) + if (!disposeMeterEarly || !experimentalEmitNoRecordedValue) { Assert.Single(actual.Gauge.DataPoints); } @@ -380,7 +381,8 @@ public void TestGaugeToOtlpMetric(bool useCustomSerializer, string name, string? [InlineData(false, "test_counter", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)] [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, true)] [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true)] - public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false) + [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true, true)] + public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false, bool experimentalEmitNoRecordedValue = false) { var metrics = new List(); @@ -423,7 +425,7 @@ public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, strin } else { - request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch); + request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch, experimentalEmitNoRecordedValue); } var resourceMetric = request.ResourceMetrics.Single(); @@ -449,7 +451,7 @@ public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, strin : OtlpMetrics.AggregationTemporality.Delta; Assert.Equal(otlpAggregationTemporality, actual.Sum.AggregationTemporality); - if (!disposeMeterEarly) + if (!disposeMeterEarly || !experimentalEmitNoRecordedValue) { Assert.Single(actual.Sum.DataPoints); } @@ -533,7 +535,8 @@ public void TestCounterToOtlpMetric(bool useCustomSerializer, string name, strin [InlineData(false, "test_counter", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)] [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, true)] [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true)] - public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false) + [InlineData(false, "test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true, true)] + public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false, bool experimentalEmitNoRecordedValue = false) { var metrics = new List(); @@ -575,7 +578,7 @@ public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, } else { - request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch); + request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch, experimentalEmitNoRecordedValue); } var resourceMetric = request.ResourceMetrics.Single(); @@ -601,7 +604,7 @@ public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, : OtlpMetrics.AggregationTemporality.Cumulative; Assert.Equal(otlpAggregationTemporality, actual.Sum.AggregationTemporality); - if (!disposeMeterEarly) + if (!disposeMeterEarly || !experimentalEmitNoRecordedValue) { Assert.Single(actual.Sum.DataPoints); } @@ -685,7 +688,8 @@ public void TestUpDownCounterToOtlpMetric(bool useCustomSerializer, string name, [InlineData(false, "test_histogram", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)] [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, true)] [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true)] - public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false) + [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true, true)] + public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false, bool experimentalEmitNoRecordedValue = false) { var metrics = new List(); @@ -733,7 +737,7 @@ public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, strin } else { - request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch); + request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch, experimentalEmitNoRecordedValue); } var resourceMetric = request.ResourceMetrics.Single(); @@ -757,7 +761,7 @@ public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, strin : OtlpMetrics.AggregationTemporality.Delta; Assert.Equal(otlpAggregationTemporality, actual.ExponentialHistogram.AggregationTemporality); - if (!disposeMeterEarly) + if (!disposeMeterEarly || !experimentalEmitNoRecordedValue) { Assert.Single(actual.ExponentialHistogram.DataPoints); } @@ -881,7 +885,8 @@ public void TestExponentialHistogramToOtlpMetric(bool useCustomSerializer, strin [InlineData(false, "test_histogram", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)] [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, true)] [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true)] - public void TestHistogramToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false) + [InlineData(false, "test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, false, false, true, true)] + public void TestHistogramToOtlpMetric(bool useCustomSerializer, string name, string? description, string? unit, long? longValue, double? doubleValue, MetricReaderTemporalityPreference aggregationTemporality, bool enableKeyValues = false, bool enableExemplars = false, bool disposeMeterEarly = false, bool experimentalEmitNoRecordedValue = false) { var metrics = new List(); @@ -924,7 +929,7 @@ public void TestHistogramToOtlpMetric(bool useCustomSerializer, string name, str } else { - request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch); + request.AddMetrics(ResourceBuilder.CreateEmpty().Build().ToOtlpResource(), batch, experimentalEmitNoRecordedValue); } var resourceMetric = request.ResourceMetrics.Single(); @@ -948,7 +953,7 @@ public void TestHistogramToOtlpMetric(bool useCustomSerializer, string name, str : OtlpMetrics.AggregationTemporality.Delta; Assert.Equal(otlpAggregationTemporality, actual.Histogram.AggregationTemporality); - if (!disposeMeterEarly) + if (!disposeMeterEarly || !experimentalEmitNoRecordedValue) { Assert.Single(actual.Histogram.DataPoints); } From f4fa00bc2399a9f8bd432d5b57d1290d87614614 Mon Sep 17 00:00:00 2001 From: Kevin Coulombe Date: Fri, 29 Nov 2024 15:19:34 -0500 Subject: [PATCH 5/5] Added changelog --- src/OpenTelemetry/CHANGELOG.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index bb9a8c4cd97..d687f6d9bad 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -6,6 +6,25 @@ Notes](../../RELEASENOTES.md). ## Unreleased +* Fixed storage exhaustion when disposing and recreating Meters. + **Previous Behavior:** Disposing a meter set the associated metric to null + in an array of default size 1000 allocated at creation time. Disposing and + recreating meters could exhaust the storage available in that list, leading + to an inability to collect the data points from newly created meters. + + **New Behavior:** Disposing a meter now marks the associated metrics for + deletion and they are cleaned up after the next collection cycle. + + **Limitation:** This means that quickly recreating meters within the same + collection cycle will still exhaust the storage limit. + +* Added an experimental flag + `OTEL_DOTNET_EXPERIMENTAL_OTLP_METRICS_EMIT_NO_RECORDED_VALUE`. When set to + `true`, after disposing a meter, a DataPoint with the flag NoRecordedValue + will be sent on the next collection cycle for all associated metrics as per + the + [OTLP data model](https://opentelemetry.io/docs/specs/otel/metrics/data-model/#no-recorded-value). + ## 1.10.0 Released 2024-Nov-12