From 9dcb8393a5eff65b7e61852303f7d4cfdcc7f247 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Wed, 16 Feb 2022 15:21:10 -0800 Subject: [PATCH 01/24] poc for fix to InMemoryExporter --- .../InMemoryExporter.cs | 9 +++ src/OpenTelemetry/Metrics/AggregatorStore.cs | 11 +++ src/OpenTelemetry/Metrics/HistogramBuckets.cs | 4 + src/OpenTelemetry/Metrics/Metric.cs | 5 +- src/OpenTelemetry/Metrics/MetricPoint.cs | 81 +++++++++++++------ .../Metrics/InMemoryExporterTests.cs | 49 ++++++++++- 6 files changed, 133 insertions(+), 26 deletions(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index d76995f8fb6..da9777fe2bb 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -22,10 +22,12 @@ public class InMemoryExporter : BaseExporter where T : class { private readonly ICollection exportedItems; + private readonly bool isMetric; public InMemoryExporter(ICollection exportedItems) { this.exportedItems = exportedItems; + this.isMetric = typeof(T) == typeof(Metrics.Metric); } public override ExportResult Export(in Batch batch) @@ -35,6 +37,13 @@ public override ExportResult Export(in Batch batch) return ExportResult.Failure; } + // Note: this mitigates the growing ExportedItems issue. + // Need to discuss if this is an acceptible change. + if (this.isMetric) + { + this.exportedItems.Clear(); + } + foreach (var data in batch) { this.exportedItems.Add(data); diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index c61f1a59273..09ef496272b 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -156,6 +156,17 @@ internal MetricPointsAccessor GetMetricPoints() return new MetricPointsAccessor(this.metricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive); } + internal MetricPointsAccessor GetDeepCloneMetricPoints() + { + var deepClonedMetricPoints = new MetricPoint[this.metricPoints.Length]; + for (int i = 0; i < this.metricPoints.Length; i++) + { + deepClonedMetricPoints[i] = this.metricPoints[i].DeepCopy(); + } + + return new MetricPointsAccessor(deepClonedMetricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive); + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void InitializeZeroTagPointIfNotInitialized() { diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs index a68033d4300..d58300ba864 100644 --- a/src/OpenTelemetry/Metrics/HistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -43,6 +43,10 @@ internal HistogramBuckets(double[] explicitBounds) public Enumerator GetEnumerator() => new(this); + // This works because all private fields are value types. + // If this class changes significantly, this may need to change. + internal HistogramBuckets DeepCopy() => (HistogramBuckets)this.MemberwiseClone(); + /// /// Enumerates the elements of a . /// diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 983976d5e9e..2abd0b6e6d5 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -128,7 +128,10 @@ internal Metric( public MetricPointsAccessor GetMetricPoints() { - return this.aggStore.GetMetricPoints(); + // Note: This appears to be safe for all existing unit tests. + // This is safe because the enumerator only moves forward. + // This may not be desirable for all use cases, in which we would need a new public method. + return this.aggStore.GetDeepCloneMetricPoints(); } internal void UpdateLong(long value, ReadOnlySpan> tags) diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index e91c1253ace..b416d5a2382 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -28,7 +28,8 @@ public struct MetricPoint { private readonly AggregationType aggType; - private readonly HistogramBuckets histogramBuckets; + // Note: not necessary to remove this, but makes DeepCopy easier. + //private readonly HistogramBuckets histogramBuckets; // Represents temporality adjusted "value" for double/long metric types or "count" when histogram private MetricPointValueStorage runningValue; @@ -59,18 +60,35 @@ internal MetricPoint( if (this.aggType == AggregationType.Histogram) { - this.histogramBuckets = new HistogramBuckets(histogramExplicitBounds); + this.HistogramBuckets = new HistogramBuckets(histogramExplicitBounds); } else if (this.aggType == AggregationType.HistogramSumCount) { - this.histogramBuckets = new HistogramBuckets(null); + this.HistogramBuckets = new HistogramBuckets(null); } else { - this.histogramBuckets = null; + this.HistogramBuckets = null; } } + //internal MetricPoint(MetricPoint other) + //{ + // Note: This is an alternative to the DeepCopy method below. + // I'm not a fan of this because it creates extra maintenance. + + // this.aggType = other.aggType; + // this.HistogramBuckets = other.HistogramBuckets?.DeepClone(); + // this.runningValue = other.runningValue; + // this.snapshotValue = other.snapshotValue; + // this.deltaLastValue = other.deltaLastValue; + // this.Tags = other.Tags; + // this.MetricPointStatus = other.MetricPointStatus; + // this.Tags = other.Tags; + // this.StartTime = other.StartTime; + // this.EndTime = other.EndTime; + //} + /// /// Gets the tags associated with the metric point. /// @@ -113,6 +131,13 @@ internal MetricPointStatus MetricPointStatus private set; } + // Note: changing this to a Property loses the "readonly", + // but enables DeepCopy using the MemberwiseClone pattern below. + private HistogramBuckets HistogramBuckets + { + get; set; + } + /// /// Gets the sum long value associated with the metric point. /// @@ -218,7 +243,7 @@ public double GetHistogramSum() this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramSum)); } - return this.histogramBuckets.SnapshotSum; + return this.HistogramBuckets.SnapshotSum; } /// @@ -227,7 +252,7 @@ public double GetHistogramSum() /// /// Applies to metric type. /// - /// . + /// . [MethodImpl(MethodImplOptions.AggressiveInlining)] public HistogramBuckets GetHistogramBuckets() { @@ -236,7 +261,17 @@ public HistogramBuckets GetHistogramBuckets() this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramBuckets)); } - return this.histogramBuckets; + return this.HistogramBuckets; + } + + // this would be easier to maintain. + // Currently this doesn't work because histogramBuckets is readonly, but could hack using Reflection. + internal MetricPoint DeepCopy() + { + var other = (MetricPoint)this.MemberwiseClone(); + other.HistogramBuckets = this.HistogramBuckets?.DeepCopy(); + + return other; } internal void Update(long number) @@ -314,20 +349,20 @@ internal void Update(double number) case AggregationType.Histogram: { int i; - for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++) + for (i = 0; i < this.HistogramBuckets.ExplicitBounds.Length; i++) { // Upper bound is inclusive - if (number <= this.histogramBuckets.ExplicitBounds[i]) + if (number <= this.HistogramBuckets.ExplicitBounds[i]) { break; } } - lock (this.histogramBuckets.LockObject) + lock (this.HistogramBuckets.LockObject) { this.runningValue.AsLong++; - this.histogramBuckets.RunningSum += number; - this.histogramBuckets.RunningBucketCounts[i]++; + this.HistogramBuckets.RunningSum += number; + this.HistogramBuckets.RunningBucketCounts[i]++; } break; @@ -335,10 +370,10 @@ internal void Update(double number) case AggregationType.HistogramSumCount: { - lock (this.histogramBuckets.LockObject) + lock (this.HistogramBuckets.LockObject) { this.runningValue.AsLong++; - this.histogramBuckets.RunningSum += number; + this.HistogramBuckets.RunningSum += number; } break; @@ -460,22 +495,22 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.Histogram: { - lock (this.histogramBuckets.LockObject) + lock (this.HistogramBuckets.LockObject) { this.snapshotValue.AsLong = this.runningValue.AsLong; - this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; + this.HistogramBuckets.SnapshotSum = this.HistogramBuckets.RunningSum; if (outputDelta) { this.runningValue.AsLong = 0; - this.histogramBuckets.RunningSum = 0; + this.HistogramBuckets.RunningSum = 0; } - for (int i = 0; i < this.histogramBuckets.RunningBucketCounts.Length; i++) + for (int i = 0; i < this.HistogramBuckets.RunningBucketCounts.Length; i++) { - this.histogramBuckets.SnapshotBucketCounts[i] = this.histogramBuckets.RunningBucketCounts[i]; + this.HistogramBuckets.SnapshotBucketCounts[i] = this.HistogramBuckets.RunningBucketCounts[i]; if (outputDelta) { - this.histogramBuckets.RunningBucketCounts[i] = 0; + this.HistogramBuckets.RunningBucketCounts[i] = 0; } } @@ -487,14 +522,14 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.HistogramSumCount: { - lock (this.histogramBuckets.LockObject) + lock (this.HistogramBuckets.LockObject) { this.snapshotValue.AsLong = this.runningValue.AsLong; - this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; + this.HistogramBuckets.SnapshotSum = this.HistogramBuckets.RunningSum; if (outputDelta) { this.runningValue.AsLong = 0; - this.histogramBuckets.RunningSum = 0; + this.HistogramBuckets.RunningSum = 0; } this.MetricPointStatus = MetricPointStatus.NoCollectPending; diff --git a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs index 5f5914be28e..926097728d8 100644 --- a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs @@ -16,15 +16,17 @@ using System.Collections.Generic; using System.Diagnostics.Metrics; + using OpenTelemetry.Exporter; using OpenTelemetry.Tests; + using Xunit; namespace OpenTelemetry.Metrics.Tests { public class InMemoryExporterTests { - [Fact(Skip = "To be run after https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 is fixed")] + [Fact] public void InMemoryExporterShouldDeepCopyMetricPoints() { var exportedItems = new List(); @@ -56,7 +58,7 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() meterProvider.ForceFlush(); - metric = exportedItems[1]; // Second Metric object is added to the collection at this point + metric = exportedItems[0]; // Second Metric object is added to the collection at this point metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric var metricPointForSecondExport = metricPointsEnumerator.Current; @@ -65,5 +67,48 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() // MetricPoint.LongValue for the first exporter metric should still be 10 Assert.Equal(10, metricPointForFirstExport.GetSumLong()); } + + [Fact] + public void Investigate_2361() + { + // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 + + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + int i = 0; + var counterLong = meter.CreateObservableCounter( + "observable-counter", + () => ++i * 10); + + meterProvider.ForceFlush(); + Assert.Equal(1, i); // verify that callback is invoked when calling Flush + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(10, metricPoint1.GetSumLong()); + + meterProvider.ForceFlush(); + Assert.Equal(2, i); // verify that callback is invoked when calling Flush + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(20, metricPoint2.GetSumLong()); + + // Retest 1st item, this is expected to be unchanged. + Assert.Equal(10, metricPoint1.GetSumLong()); + } + + private static MetricPoint GetSingleMetricPoint(Metric metric) + { + var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); + Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric + ref readonly var metricPoint = ref metricPointsEnumerator.Current; + Assert.False(metricPointsEnumerator.MoveNext()); + return metricPoint; + } } } From 34d15ecf5bcd7d5e418f8c6d2de32f320a04a6f2 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 22 Feb 2022 13:39:07 -0800 Subject: [PATCH 02/24] Revert "poc for fix to InMemoryExporter" This reverts commit 9dcb8393a5eff65b7e61852303f7d4cfdcc7f247. --- .../InMemoryExporter.cs | 9 --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 11 --- src/OpenTelemetry/Metrics/HistogramBuckets.cs | 4 - src/OpenTelemetry/Metrics/Metric.cs | 5 +- src/OpenTelemetry/Metrics/MetricPoint.cs | 81 ++++++------------- .../Metrics/InMemoryExporterTests.cs | 49 +---------- 6 files changed, 26 insertions(+), 133 deletions(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index da9777fe2bb..d76995f8fb6 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -22,12 +22,10 @@ public class InMemoryExporter : BaseExporter where T : class { private readonly ICollection exportedItems; - private readonly bool isMetric; public InMemoryExporter(ICollection exportedItems) { this.exportedItems = exportedItems; - this.isMetric = typeof(T) == typeof(Metrics.Metric); } public override ExportResult Export(in Batch batch) @@ -37,13 +35,6 @@ public override ExportResult Export(in Batch batch) return ExportResult.Failure; } - // Note: this mitigates the growing ExportedItems issue. - // Need to discuss if this is an acceptible change. - if (this.isMetric) - { - this.exportedItems.Clear(); - } - foreach (var data in batch) { this.exportedItems.Add(data); diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 09ef496272b..c61f1a59273 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -156,17 +156,6 @@ internal MetricPointsAccessor GetMetricPoints() return new MetricPointsAccessor(this.metricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive); } - internal MetricPointsAccessor GetDeepCloneMetricPoints() - { - var deepClonedMetricPoints = new MetricPoint[this.metricPoints.Length]; - for (int i = 0; i < this.metricPoints.Length; i++) - { - deepClonedMetricPoints[i] = this.metricPoints[i].DeepCopy(); - } - - return new MetricPointsAccessor(deepClonedMetricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive); - } - [MethodImpl(MethodImplOptions.AggressiveInlining)] private void InitializeZeroTagPointIfNotInitialized() { diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs index d58300ba864..a68033d4300 100644 --- a/src/OpenTelemetry/Metrics/HistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -43,10 +43,6 @@ internal HistogramBuckets(double[] explicitBounds) public Enumerator GetEnumerator() => new(this); - // This works because all private fields are value types. - // If this class changes significantly, this may need to change. - internal HistogramBuckets DeepCopy() => (HistogramBuckets)this.MemberwiseClone(); - /// /// Enumerates the elements of a . /// diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 2abd0b6e6d5..983976d5e9e 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -128,10 +128,7 @@ internal Metric( public MetricPointsAccessor GetMetricPoints() { - // Note: This appears to be safe for all existing unit tests. - // This is safe because the enumerator only moves forward. - // This may not be desirable for all use cases, in which we would need a new public method. - return this.aggStore.GetDeepCloneMetricPoints(); + return this.aggStore.GetMetricPoints(); } internal void UpdateLong(long value, ReadOnlySpan> tags) diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index b416d5a2382..e91c1253ace 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -28,8 +28,7 @@ public struct MetricPoint { private readonly AggregationType aggType; - // Note: not necessary to remove this, but makes DeepCopy easier. - //private readonly HistogramBuckets histogramBuckets; + private readonly HistogramBuckets histogramBuckets; // Represents temporality adjusted "value" for double/long metric types or "count" when histogram private MetricPointValueStorage runningValue; @@ -60,35 +59,18 @@ internal MetricPoint( if (this.aggType == AggregationType.Histogram) { - this.HistogramBuckets = new HistogramBuckets(histogramExplicitBounds); + this.histogramBuckets = new HistogramBuckets(histogramExplicitBounds); } else if (this.aggType == AggregationType.HistogramSumCount) { - this.HistogramBuckets = new HistogramBuckets(null); + this.histogramBuckets = new HistogramBuckets(null); } else { - this.HistogramBuckets = null; + this.histogramBuckets = null; } } - //internal MetricPoint(MetricPoint other) - //{ - // Note: This is an alternative to the DeepCopy method below. - // I'm not a fan of this because it creates extra maintenance. - - // this.aggType = other.aggType; - // this.HistogramBuckets = other.HistogramBuckets?.DeepClone(); - // this.runningValue = other.runningValue; - // this.snapshotValue = other.snapshotValue; - // this.deltaLastValue = other.deltaLastValue; - // this.Tags = other.Tags; - // this.MetricPointStatus = other.MetricPointStatus; - // this.Tags = other.Tags; - // this.StartTime = other.StartTime; - // this.EndTime = other.EndTime; - //} - /// /// Gets the tags associated with the metric point. /// @@ -131,13 +113,6 @@ internal MetricPointStatus MetricPointStatus private set; } - // Note: changing this to a Property loses the "readonly", - // but enables DeepCopy using the MemberwiseClone pattern below. - private HistogramBuckets HistogramBuckets - { - get; set; - } - /// /// Gets the sum long value associated with the metric point. /// @@ -243,7 +218,7 @@ public double GetHistogramSum() this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramSum)); } - return this.HistogramBuckets.SnapshotSum; + return this.histogramBuckets.SnapshotSum; } /// @@ -252,7 +227,7 @@ public double GetHistogramSum() /// /// Applies to metric type. /// - /// . + /// . [MethodImpl(MethodImplOptions.AggressiveInlining)] public HistogramBuckets GetHistogramBuckets() { @@ -261,17 +236,7 @@ public HistogramBuckets GetHistogramBuckets() this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramBuckets)); } - return this.HistogramBuckets; - } - - // this would be easier to maintain. - // Currently this doesn't work because histogramBuckets is readonly, but could hack using Reflection. - internal MetricPoint DeepCopy() - { - var other = (MetricPoint)this.MemberwiseClone(); - other.HistogramBuckets = this.HistogramBuckets?.DeepCopy(); - - return other; + return this.histogramBuckets; } internal void Update(long number) @@ -349,20 +314,20 @@ internal void Update(double number) case AggregationType.Histogram: { int i; - for (i = 0; i < this.HistogramBuckets.ExplicitBounds.Length; i++) + for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++) { // Upper bound is inclusive - if (number <= this.HistogramBuckets.ExplicitBounds[i]) + if (number <= this.histogramBuckets.ExplicitBounds[i]) { break; } } - lock (this.HistogramBuckets.LockObject) + lock (this.histogramBuckets.LockObject) { this.runningValue.AsLong++; - this.HistogramBuckets.RunningSum += number; - this.HistogramBuckets.RunningBucketCounts[i]++; + this.histogramBuckets.RunningSum += number; + this.histogramBuckets.RunningBucketCounts[i]++; } break; @@ -370,10 +335,10 @@ internal void Update(double number) case AggregationType.HistogramSumCount: { - lock (this.HistogramBuckets.LockObject) + lock (this.histogramBuckets.LockObject) { this.runningValue.AsLong++; - this.HistogramBuckets.RunningSum += number; + this.histogramBuckets.RunningSum += number; } break; @@ -495,22 +460,22 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.Histogram: { - lock (this.HistogramBuckets.LockObject) + lock (this.histogramBuckets.LockObject) { this.snapshotValue.AsLong = this.runningValue.AsLong; - this.HistogramBuckets.SnapshotSum = this.HistogramBuckets.RunningSum; + this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; if (outputDelta) { this.runningValue.AsLong = 0; - this.HistogramBuckets.RunningSum = 0; + this.histogramBuckets.RunningSum = 0; } - for (int i = 0; i < this.HistogramBuckets.RunningBucketCounts.Length; i++) + for (int i = 0; i < this.histogramBuckets.RunningBucketCounts.Length; i++) { - this.HistogramBuckets.SnapshotBucketCounts[i] = this.HistogramBuckets.RunningBucketCounts[i]; + this.histogramBuckets.SnapshotBucketCounts[i] = this.histogramBuckets.RunningBucketCounts[i]; if (outputDelta) { - this.HistogramBuckets.RunningBucketCounts[i] = 0; + this.histogramBuckets.RunningBucketCounts[i] = 0; } } @@ -522,14 +487,14 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.HistogramSumCount: { - lock (this.HistogramBuckets.LockObject) + lock (this.histogramBuckets.LockObject) { this.snapshotValue.AsLong = this.runningValue.AsLong; - this.HistogramBuckets.SnapshotSum = this.HistogramBuckets.RunningSum; + this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; if (outputDelta) { this.runningValue.AsLong = 0; - this.HistogramBuckets.RunningSum = 0; + this.histogramBuckets.RunningSum = 0; } this.MetricPointStatus = MetricPointStatus.NoCollectPending; diff --git a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs index 926097728d8..5f5914be28e 100644 --- a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs @@ -16,17 +16,15 @@ using System.Collections.Generic; using System.Diagnostics.Metrics; - using OpenTelemetry.Exporter; using OpenTelemetry.Tests; - using Xunit; namespace OpenTelemetry.Metrics.Tests { public class InMemoryExporterTests { - [Fact] + [Fact(Skip = "To be run after https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 is fixed")] public void InMemoryExporterShouldDeepCopyMetricPoints() { var exportedItems = new List(); @@ -58,7 +56,7 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() meterProvider.ForceFlush(); - metric = exportedItems[0]; // Second Metric object is added to the collection at this point + metric = exportedItems[1]; // Second Metric object is added to the collection at this point metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric var metricPointForSecondExport = metricPointsEnumerator.Current; @@ -67,48 +65,5 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() // MetricPoint.LongValue for the first exporter metric should still be 10 Assert.Equal(10, metricPointForFirstExport.GetSumLong()); } - - [Fact] - public void Investigate_2361() - { - // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 - - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - int i = 0; - var counterLong = meter.CreateObservableCounter( - "observable-counter", - () => ++i * 10); - - meterProvider.ForceFlush(); - Assert.Equal(1, i); // verify that callback is invoked when calling Flush - Assert.Single(exportedItems); // verify that List contains 1 item - var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(10, metricPoint1.GetSumLong()); - - meterProvider.ForceFlush(); - Assert.Equal(2, i); // verify that callback is invoked when calling Flush - Assert.Single(exportedItems); // verify that List contains 1 item - var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(20, metricPoint2.GetSumLong()); - - // Retest 1st item, this is expected to be unchanged. - Assert.Equal(10, metricPoint1.GetSumLong()); - } - - private static MetricPoint GetSingleMetricPoint(Metric metric) - { - var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); - Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric - ref readonly var metricPoint = ref metricPointsEnumerator.Current; - Assert.False(metricPointsEnumerator.MoveNext()); - return metricPoint; - } } } From 48bf1e9e1f46cb638aa471e2228ea48d91e5a228 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Wed, 23 Feb 2022 15:23:32 -0800 Subject: [PATCH 03/24] change InMemoryExporter. update tests. add more tests. --- docs/metrics/extending-the-sdk/README.md | 5 +- .../InMemoryExporter.cs | 7 + .../Metrics/InMemoryExporterTests.cs | 259 +++++++++++++++++- .../Metrics/MemoryEfficiencyTests.cs | 1 - .../Metrics/MetricAPITest.cs | 30 +- .../Metrics/MultipleReadersTests.cs | 4 +- 6 files changed, 273 insertions(+), 33 deletions(-) diff --git a/docs/metrics/extending-the-sdk/README.md b/docs/metrics/extending-the-sdk/README.md index b1eb86eb3bd..a3d52afeb17 100644 --- a/docs/metrics/extending-the-sdk/README.md +++ b/docs/metrics/extending-the-sdk/README.md @@ -28,7 +28,10 @@ not covered by the built-in exporters: * Exporters should avoid generating telemetry and causing live-loop, this can be done via `OpenTelemetry.SuppressInstrumentationScope`. * Exporters receives a batch of `Metric`, and each `Metric` - can contain 1 or more `MetricPoint`s. + can contain 1 or more `MetricPoint`s. After export, users are expected to + perform all actions with the exported Metrics and MetricPoints. Once control + is returned, exporter can no longer assume the state of the Batch. + (TODO: I need help with the statement here.) * Exporters should use `Activity.TagObjects` collection instead of `Activity.Tags` to obtain the full set of attributes (tags). * Exporters should use `ParentProvider.GetResource()` to get the `Resource` diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index d76995f8fb6..4a198e7abae 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -22,10 +22,12 @@ public class InMemoryExporter : BaseExporter where T : class { private readonly ICollection exportedItems; + private readonly bool isMetric; public InMemoryExporter(ICollection exportedItems) { this.exportedItems = exportedItems; + this.isMetric = typeof(T) == typeof(Metrics.Metric); } public override ExportResult Export(in Batch batch) @@ -35,6 +37,11 @@ public override ExportResult Export(in Batch batch) return ExportResult.Failure; } + if (this.isMetric) + { + this.exportedItems.Clear(); + } + foreach (var data in batch) { this.exportedItems.Add(data); diff --git a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs index 5f5914be28e..06634406a8d 100644 --- a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs @@ -24,7 +24,7 @@ namespace OpenTelemetry.Metrics.Tests { public class InMemoryExporterTests { - [Fact(Skip = "To be run after https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 is fixed")] + [Fact] public void InMemoryExporterShouldDeepCopyMetricPoints() { var exportedItems = new List(); @@ -44,26 +44,257 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() counter.Add(10, new KeyValuePair("tag1", "value1")); meterProvider.ForceFlush(); - - var metric = exportedItems[0]; // Only one Metric object is added to the collection at this point - var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); - Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric - ref readonly var metricPointForFirstExport = ref metricPointsEnumerator.Current; - Assert.Equal(10, metricPointForFirstExport.GetSumLong()); + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(10, metricPoint1.GetSumLong()); // Emit 25 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") counter.Add(25, new KeyValuePair("tag1", "value1")); meterProvider.ForceFlush(); - - metric = exportedItems[1]; // Second Metric object is added to the collection at this point - metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); - Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric - var metricPointForSecondExport = metricPointsEnumerator.Current; - Assert.Equal(25, metricPointForSecondExport.GetSumLong()); + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(25, metricPoint2.GetSumLong()); // MetricPoint.LongValue for the first exporter metric should still be 10 - Assert.Equal(10, metricPointForFirstExport.GetSumLong()); + Assert.Equal(10, metricPoint1.GetSumLong()); + } + + [Fact] + public void Investigate_2361() + { + // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 + + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + int i = 0; + var counterLong = meter.CreateObservableCounter( + "observable-counter", + () => ++i * 10); + + meterProvider.ForceFlush(); + Assert.Equal(1, i); // verify that callback is invoked when calling Flush + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(10, metricPoint1.GetSumLong()); + + meterProvider.ForceFlush(); + Assert.Equal(2, i); // verify that callback is invoked when calling Flush + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(20, metricPoint2.GetSumLong()); + + // Retest 1st item, this is expected to be unchanged. + Assert.Equal(10, metricPoint1.GetSumLong()); + } + + [Fact] + public void InvestigateCounters() + { + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + var counter1 = meter.CreateCounter("counter1"); + + counter1.Add(10); + meterProvider.ForceFlush(); + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(10, metricPoint1.GetSumLong()); + + var counter2 = meter.CreateCounter("counter2"); + counter1.Add(20); + counter2.Add(35); + + meterProvider.ForceFlush(); + + Assert.Equal(2, exportedItems.Count); // verify that List contains 2 items + + var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(30, metricPoint2.GetSumLong()); + + var metricPoint3 = GetSingleMetricPoint(exportedItems[1]); + Assert.Equal(35, metricPoint3.GetSumLong()); + } + + [Fact] + public void InvestigateCounter_WithSecondFlush() + { + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + var counter = meter.CreateCounter("meter"); + + // Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") + counter.Add(10, new KeyValuePair("tag1", "value1")); + + meterProvider.ForceFlush(); + Assert.Single(exportedItems); // verify that List contains 1 item + var metric1 = exportedItems[0]; + + counter.Add(25, new KeyValuePair("tag1", "value1")); + + meterProvider.ForceFlush(); + Assert.Single(exportedItems); // verify that List contains 1 item + var metric2 = exportedItems[0]; + + Assert.Same(metric1, metric2); + + var metricPoint1 = GetSingleMetricPoint(metric1); + Assert.Equal(35, metricPoint1.GetSumLong()); + + var metricPoint2 = GetSingleMetricPoint(metric2); + Assert.Equal(35, metricPoint2.GetSumLong()); + } + + [Fact] + public void InvestigateCounter_WithoutSecondFlush() + { + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + var counter = meter.CreateCounter("meter"); + + // Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") + counter.Add(10, new KeyValuePair("tag1", "value1")); + + meterProvider.ForceFlush(); + Assert.Single(exportedItems); // verify that List contains 1 item + var metric1 = exportedItems[0]; + + counter.Add(25, new KeyValuePair("tag1", "value1")); + + var metricPoint1 = GetSingleMetricPoint(metric1); + Assert.Equal(10, metricPoint1.GetSumLong()); // Note that the second counter.Add doesn't affect the sum yet. + } + + [Fact] + public void InvestigateEnumerator_UsingCounter() + { + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + var counter = meter.CreateCounter("meter"); + + // Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") + counter.Add(10, new KeyValuePair("tag1", "value1")); + + meterProvider.ForceFlush(); + + var metricPointsAccessor = exportedItems[0].GetMetricPoints(); + + counter.Add(25, new KeyValuePair("tag1", "value1")); + + var metricPointsEnumerator1 = metricPointsAccessor.GetEnumerator(); + metricPointsEnumerator1.MoveNext(); + var metricPoint1 = metricPointsEnumerator1.Current; + Assert.Equal(10, metricPoint1.GetSumLong()); + + meterProvider.ForceFlush(); + + var metricPointsEnumerator2 = metricPointsAccessor.GetEnumerator(); + metricPointsEnumerator2.MoveNext(); + var metricPoint2 = metricPointsEnumerator2.Current; + Assert.Equal(35, metricPoint2.GetSumLong()); + + var metricPoint1again = metricPointsEnumerator1.Current; + Assert.Equal(35, metricPoint1again.GetSumLong()); + } + + [Fact] + public void InvestigateEnumerator_UsingObservable() + { + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + int i = 0; + var counterLong = meter.CreateObservableCounter( + "observable-counter", + () => ++i * 10); + + meterProvider.ForceFlush(); + + var metricPointsEnumerator = exportedItems[0].GetMetricPoints().GetEnumerator(); + + meterProvider.ForceFlush(); + + metricPointsEnumerator.MoveNext(); + var metricPoint = metricPointsEnumerator.Current; + Assert.Equal(20, metricPoint.GetSumLong()); + } + + [Fact] + public void TestHistograms() + { + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + var histogram = meter.CreateHistogram("histogram"); + + for (int i = 0; i < 5; i++) + { + histogram.Record(i); + } + + meterProvider.ForceFlush(); + + var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(5, metricPoint1.GetHistogramCount()); + Assert.Equal(10, metricPoint1.GetHistogramSum()); + + for (int i = 0; i < 5; i++) + { + histogram.Record(i); + } + + meterProvider.ForceFlush(); + Assert.Equal(5, metricPoint1.GetHistogramCount()); + Assert.Equal(20, metricPoint1.GetHistogramSum()); + } + + private static MetricPoint GetSingleMetricPoint(Metric metric) + { + var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); + Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric + ref readonly var metricPoint = ref metricPointsEnumerator.Current; + Assert.False(metricPointsEnumerator.MoveNext()); + return metricPoint; } } } diff --git a/test/OpenTelemetry.Tests/Metrics/MemoryEfficiencyTests.cs b/test/OpenTelemetry.Tests/Metrics/MemoryEfficiencyTests.cs index c241519b95e..eff168ba7ec 100644 --- a/test/OpenTelemetry.Tests/Metrics/MemoryEfficiencyTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MemoryEfficiencyTests.cs @@ -48,7 +48,6 @@ public void ExportOnlyWhenPointChanged(AggregationTemporality temporality) meterProvider.ForceFlush(); Assert.Single(exportedItems); - exportedItems.Clear(); meterProvider.ForceFlush(); if (temporality == AggregationTemporality.Cumulative) { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index cb5d1447fef..73bc9e6cf2a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -230,7 +230,7 @@ public void DuplicateInstrumentNamesFromDifferentMetersAreAllowed(AggregationTem var anotherCounterSameNameDiffMeter = meter2.CreateCounter("name1"); anotherCounterSameNameDiffMeter.Add(10); counterLong.Add(10); - exportedItems.Clear(); + meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(2, exportedItems.Count); } @@ -338,7 +338,6 @@ public void CounterAggregationTest(bool exportDelta) long sumReceived = GetLongSum(exportedItems); Assert.Equal(20, sumReceived); - exportedItems.Clear(); counterLong.Add(10); counterLong.Add(10); meterProvider.ForceFlush(MaxTimeToAllowForFlush); @@ -352,7 +351,6 @@ public void CounterAggregationTest(bool exportDelta) Assert.Equal(40, sumReceived); } - exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); sumReceived = GetLongSum(exportedItems); if (exportDelta) @@ -364,7 +362,6 @@ public void CounterAggregationTest(bool exportDelta) Assert.Equal(40, sumReceived); } - exportedItems.Clear(); counterLong.Add(40); counterLong.Add(20); meterProvider.ForceFlush(MaxTimeToAllowForFlush); @@ -410,7 +407,6 @@ public void ObservableCounterAggregationTest(bool exportDelta) long sumReceived = GetLongSum(exportedItems); Assert.Equal(10, sumReceived); - exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); sumReceived = GetLongSum(exportedItems); if (exportDelta) @@ -422,7 +418,6 @@ public void ObservableCounterAggregationTest(bool exportDelta) Assert.Equal(20, sumReceived); } - exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); sumReceived = GetLongSum(exportedItems); if (exportDelta) @@ -500,7 +495,7 @@ public void ObservableCounterWithTagsAggregationTest(bool exportDelta) ValidateMetricPointTags(tags3, metricPoint3.Tags); // Export 2 - exportedItems.Clear(); + // exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Single(exportedItems); metric = exportedItems[0]; @@ -651,7 +646,7 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) long sumReceived = GetLongSum(exportedItems); Assert.Equal(75, sumReceived); - exportedItems.Clear(); + // exportedItems.Clear(); counterLong.Add(5, new("Key2", "Value2"), new("Key1", "Value1"), new("Key3", "Value3")); counterLong.Add(5, new("Key2", "Value2"), new("Key1", "Value1"), new("Key3", "Value3")); @@ -742,7 +737,7 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) long sumReceived = GetLongSum(exportedItems); Assert.Equal(75, sumReceived); - exportedItems.Clear(); + // exportedItems.Clear(); counterLong.Add(5, new("Key2", "Value2"), new("Key1", "Value1"), new("Key3", "Value3")); counterLong.Add(5, new("Key2", "Value2"), new("Key1", "Value1"), new("Key3", "Value3")); @@ -794,7 +789,8 @@ public void TestInstrumentDisposal(AggregationTemporality temporality) meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(2, exportedItems.Count); - exportedItems.Clear(); + + // exportedItems.Clear(); counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); @@ -802,13 +798,15 @@ public void TestInstrumentDisposal(AggregationTemporality temporality) meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(2, exportedItems.Count); - exportedItems.Clear(); + + // exportedItems.Clear(); counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Single(exportedItems); - exportedItems.Clear(); + + // exportedItems.Clear(); counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); @@ -816,7 +814,8 @@ public void TestInstrumentDisposal(AggregationTemporality temporality) meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Single(exportedItems); - exportedItems.Clear(); + + // exportedItems.Clear(); counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); @@ -869,7 +868,7 @@ int MetricPointCount() meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault, MetricPointCount()); - exportedItems.Clear(); + // exportedItems.Clear(); counterLong.Add(10); for (int i = 0; i < MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault + 1; i++) { @@ -889,7 +888,8 @@ int MetricPointCount() counterLong.Add(10, new KeyValuePair("key", "valueA")); counterLong.Add(10, new KeyValuePair("key", "valueB")); counterLong.Add(10, new KeyValuePair("key", "valueC")); - exportedItems.Clear(); + + // exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault, MetricPointCount()); } diff --git a/test/OpenTelemetry.Tests/Metrics/MultipleReadersTests.cs b/test/OpenTelemetry.Tests/Metrics/MultipleReadersTests.cs index ba08b0644a5..e994b7a0c58 100644 --- a/test/OpenTelemetry.Tests/Metrics/MultipleReadersTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MultipleReadersTests.cs @@ -80,8 +80,8 @@ public void SdkSupportsMultipleReaders(AggregationTemporality aggregationTempora this.AssertLongSumValueForMetric(exportedItems1[1], 100); this.AssertLongSumValueForMetric(exportedItems2[1], 200); - exportedItems1.Clear(); - exportedItems2.Clear(); + // exportedItems1.Clear(); + // exportedItems2.Clear(); counter.Add(15, new KeyValuePair("key", "value")); From 2d06efc48714ca7201ce9c6114ebae3a45e51c7c Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Wed, 23 Feb 2022 15:32:45 -0800 Subject: [PATCH 04/24] cleanup --- .../OpenTelemetry.Tests/Metrics/MetricAPITest.cs | 16 ---------------- .../Metrics/MultipleReadersTests.cs | 3 --- 2 files changed, 19 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 73bc9e6cf2a..3f1b73e9576 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -230,7 +230,6 @@ public void DuplicateInstrumentNamesFromDifferentMetersAreAllowed(AggregationTem var anotherCounterSameNameDiffMeter = meter2.CreateCounter("name1"); anotherCounterSameNameDiffMeter.Add(10); counterLong.Add(10); - meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(2, exportedItems.Count); } @@ -495,7 +494,6 @@ public void ObservableCounterWithTagsAggregationTest(bool exportDelta) ValidateMetricPointTags(tags3, metricPoint3.Tags); // Export 2 - // exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Single(exportedItems); metric = exportedItems[0]; @@ -646,8 +644,6 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) long sumReceived = GetLongSum(exportedItems); Assert.Equal(75, sumReceived); - // exportedItems.Clear(); - counterLong.Add(5, new("Key2", "Value2"), new("Key1", "Value1"), new("Key3", "Value3")); counterLong.Add(5, new("Key2", "Value2"), new("Key1", "Value1"), new("Key3", "Value3")); counterLong.Add(10, new("Key2", "Value2"), new("Key3", "Value3"), new("Key1", "Value1")); @@ -737,8 +733,6 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) long sumReceived = GetLongSum(exportedItems); Assert.Equal(75, sumReceived); - // exportedItems.Clear(); - counterLong.Add(5, new("Key2", "Value2"), new("Key1", "Value1"), new("Key3", "Value3")); counterLong.Add(5, new("Key2", "Value2"), new("Key1", "Value1"), new("Key3", "Value3")); counterLong.Add(10, new("Key2", "Value2"), new("Key3", "Value3"), new("Key1", "Value1")); @@ -790,8 +784,6 @@ public void TestInstrumentDisposal(AggregationTemporality temporality) meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(2, exportedItems.Count); - // exportedItems.Clear(); - counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); meter1.Dispose(); @@ -799,15 +791,11 @@ public void TestInstrumentDisposal(AggregationTemporality temporality) meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(2, exportedItems.Count); - // exportedItems.Clear(); - counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Single(exportedItems); - // exportedItems.Clear(); - counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); meter2.Dispose(); @@ -815,8 +803,6 @@ public void TestInstrumentDisposal(AggregationTemporality temporality) meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Single(exportedItems); - // exportedItems.Clear(); - counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); meterProvider.ForceFlush(MaxTimeToAllowForFlush); @@ -868,7 +854,6 @@ int MetricPointCount() meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault, MetricPointCount()); - // exportedItems.Clear(); counterLong.Add(10); for (int i = 0; i < MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault + 1; i++) { @@ -889,7 +874,6 @@ int MetricPointCount() counterLong.Add(10, new KeyValuePair("key", "valueB")); counterLong.Add(10, new KeyValuePair("key", "valueC")); - // exportedItems.Clear(); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault, MetricPointCount()); } diff --git a/test/OpenTelemetry.Tests/Metrics/MultipleReadersTests.cs b/test/OpenTelemetry.Tests/Metrics/MultipleReadersTests.cs index e994b7a0c58..11868e963b3 100644 --- a/test/OpenTelemetry.Tests/Metrics/MultipleReadersTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MultipleReadersTests.cs @@ -80,9 +80,6 @@ public void SdkSupportsMultipleReaders(AggregationTemporality aggregationTempora this.AssertLongSumValueForMetric(exportedItems1[1], 100); this.AssertLongSumValueForMetric(exportedItems2[1], 200); - // exportedItems1.Clear(); - // exportedItems2.Clear(); - counter.Add(15, new KeyValuePair("key", "value")); meterProvider.ForceFlush(); From 1617248cc30caeb438eb3899a4aba75f491d777d Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Wed, 23 Feb 2022 15:33:37 -0800 Subject: [PATCH 05/24] cleanup --- test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 3f1b73e9576..48c53d46859 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -873,7 +873,6 @@ int MetricPointCount() counterLong.Add(10, new KeyValuePair("key", "valueA")); counterLong.Add(10, new KeyValuePair("key", "valueB")); counterLong.Add(10, new KeyValuePair("key", "valueC")); - meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(MeterProviderBuilderBase.MaxMetricPointsPerMetricDefault, MetricPointCount()); } From 4723675cfdf98f5c1c6405d433e374649a210c2b Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Wed, 23 Feb 2022 19:36:06 -0800 Subject: [PATCH 06/24] cleanup tests --- .../Metrics/InMemoryExporterTests.cs | 175 ++++-------------- 1 file changed, 41 insertions(+), 134 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs index 6c73729d469..96778396f88 100644 --- a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs @@ -24,7 +24,7 @@ namespace OpenTelemetry.Metrics.Tests public class InMemoryExporterTests { [Fact] - public void InMemoryExporterShouldDeepCopyMetricPoints() + public void Verify_MetricPoint_UsingDeltaAggregation() { var exportedItems = new List(); @@ -43,7 +43,7 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() counter.Add(10, new KeyValuePair("tag1", "value1")); meterProvider.ForceFlush(); - Assert.Single(exportedItems); // verify that List contains 1 item + Assert.Single(exportedItems); var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); Assert.Equal(10, metricPoint1.GetSumLong()); @@ -51,84 +51,16 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() counter.Add(25, new KeyValuePair("tag1", "value1")); meterProvider.ForceFlush(); - Assert.Single(exportedItems); // verify that List contains 1 item + Assert.Single(exportedItems); var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); Assert.Equal(25, metricPoint2.GetSumLong()); - // MetricPoint.LongValue for the first exporter metric should still be 10 - Assert.Equal(10, metricPoint1.GetSumLong()); - } - - [Fact] - public void Investigate_2361() - { - // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 - - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - int i = 0; - var counterLong = meter.CreateObservableCounter( - "observable-counter", - () => ++i * 10); - - meterProvider.ForceFlush(); - Assert.Equal(1, i); // verify that callback is invoked when calling Flush - Assert.Single(exportedItems); // verify that List contains 1 item - var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(10, metricPoint1.GetSumLong()); - - meterProvider.ForceFlush(); - Assert.Equal(2, i); // verify that callback is invoked when calling Flush - Assert.Single(exportedItems); // verify that List contains 1 item - var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(20, metricPoint2.GetSumLong()); - // Retest 1st item, this is expected to be unchanged. Assert.Equal(10, metricPoint1.GetSumLong()); } [Fact] - public void InvestigateCounters() - { - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - var counter1 = meter.CreateCounter("counter1"); - - counter1.Add(10); - meterProvider.ForceFlush(); - Assert.Single(exportedItems); // verify that List contains 1 item - var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(10, metricPoint1.GetSumLong()); - - var counter2 = meter.CreateCounter("counter2"); - counter1.Add(20); - counter2.Add(35); - - meterProvider.ForceFlush(); - - Assert.Equal(2, exportedItems.Count); // verify that List contains 2 items - - var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(30, metricPoint2.GetSumLong()); - - var metricPoint3 = GetSingleMetricPoint(exportedItems[1]); - Assert.Equal(35, metricPoint3.GetSumLong()); - } - - [Fact] - public void InvestigateCounter_WithSecondFlush() + public void Verify_Metric_UsingCounter() { var exportedItems = new List(); @@ -140,30 +72,25 @@ public void InvestigateCounter_WithSecondFlush() var counter = meter.CreateCounter("meter"); - // Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") - counter.Add(10, new KeyValuePair("tag1", "value1")); + counter.Add(10); meterProvider.ForceFlush(); - Assert.Single(exportedItems); // verify that List contains 1 item + Assert.Single(exportedItems); var metric1 = exportedItems[0]; - counter.Add(25, new KeyValuePair("tag1", "value1")); - + counter.Add(5); meterProvider.ForceFlush(); - Assert.Single(exportedItems); // verify that List contains 1 item + Assert.Single(exportedItems); var metric2 = exportedItems[0]; + // Note that although flush has been called twice + // the same metric has been exported. + // This is by design, because the MetricsApi reuses metrics. Assert.Same(metric1, metric2); - - var metricPoint1 = GetSingleMetricPoint(metric1); - Assert.Equal(35, metricPoint1.GetSumLong()); - - var metricPoint2 = GetSingleMetricPoint(metric2); - Assert.Equal(35, metricPoint2.GetSumLong()); } [Fact] - public void InvestigateCounter_WithoutSecondFlush() + public void Verify_MetricPoint_UsingCounter() { var exportedItems = new List(); @@ -175,60 +102,31 @@ public void InvestigateCounter_WithoutSecondFlush() var counter = meter.CreateCounter("meter"); - // Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") - counter.Add(10, new KeyValuePair("tag1", "value1")); + counter.Add(10); meterProvider.ForceFlush(); - Assert.Single(exportedItems); // verify that List contains 1 item - var metric1 = exportedItems[0]; - - counter.Add(25, new KeyValuePair("tag1", "value1")); + Assert.Single(exportedItems); - var metricPoint1 = GetSingleMetricPoint(metric1); - Assert.Equal(10, metricPoint1.GetSumLong()); // Note that the second counter.Add doesn't affect the sum yet. - } - - [Fact] - public void InvestigateEnumerator_UsingCounter() - { - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - var counter = meter.CreateCounter("meter"); + counter.Add(5); - // Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") - counter.Add(10, new KeyValuePair("tag1", "value1")); + var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(10, metricPoint1.GetSumLong()); // Note that the second counter.Add doesn't affect the sum until calling Flush. meterProvider.ForceFlush(); + Assert.Single(exportedItems); - var metricPointsAccessor = exportedItems[0].GetMetricPoints(); - - counter.Add(25, new KeyValuePair("tag1", "value1")); + var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(15, metricPoint2.GetSumLong()); // The second MetricPoint will have the updated value. - var metricPointsEnumerator1 = metricPointsAccessor.GetEnumerator(); - metricPointsEnumerator1.MoveNext(); - var metricPoint1 = metricPointsEnumerator1.Current; + // Retest 1st item, this is expected to be unchanged. Assert.Equal(10, metricPoint1.GetSumLong()); - - meterProvider.ForceFlush(); - - var metricPointsEnumerator2 = metricPointsAccessor.GetEnumerator(); - metricPointsEnumerator2.MoveNext(); - var metricPoint2 = metricPointsEnumerator2.Current; - Assert.Equal(35, metricPoint2.GetSumLong()); - - var metricPoint1again = metricPointsEnumerator1.Current; - Assert.Equal(35, metricPoint1again.GetSumLong()); } [Fact] - public void InvestigateEnumerator_UsingObservable() + public void Verify_MetricPoint_UsingObservableCounter() { + // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 + var exportedItems = new List(); using var meter = new Meter(Utils.GetCurrentMethodName()); @@ -243,18 +141,23 @@ public void InvestigateEnumerator_UsingObservable() () => ++i * 10); meterProvider.ForceFlush(); - - var metricPointsEnumerator = exportedItems[0].GetMetricPoints().GetEnumerator(); + Assert.Equal(1, i); // verify that callback is invoked when calling Flush + Assert.Single(exportedItems); + var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(10, metricPoint1.GetSumLong()); meterProvider.ForceFlush(); + Assert.Equal(2, i); // verify that callback is invoked when calling Flush + Assert.Single(exportedItems); + var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(20, metricPoint2.GetSumLong()); - metricPointsEnumerator.MoveNext(); - var metricPoint = metricPointsEnumerator.Current; - Assert.Equal(20, metricPoint.GetSumLong()); + // Retest 1st item, this is expected to be unchanged. + Assert.Equal(10, metricPoint1.GetSumLong()); } [Fact] - public void TestHistograms() + public void Verify_MetricPoint_UsingHistograms() { var exportedItems = new List(); @@ -283,6 +186,10 @@ public void TestHistograms() } meterProvider.ForceFlush(); + + // Note that although metricPoint1 represents the first flush, + // this struct holds a reference to HistogramBuckets and this value has updated. + // This is by design. Assert.Equal(5, metricPoint1.GetHistogramCount()); Assert.Equal(20, metricPoint1.GetHistogramSum()); } @@ -290,8 +197,8 @@ public void TestHistograms() private static MetricPoint GetSingleMetricPoint(Metric metric) { var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); - Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric - ref readonly var metricPoint = ref metricPointsEnumerator.Current; + Assert.True(metricPointsEnumerator.MoveNext()); // Only one MetricPoint is emitted for the Metric + var metricPoint = metricPointsEnumerator.Current; Assert.False(metricPointsEnumerator.MoveNext()); return metricPoint; } From f4b40fd6fd619d54ae4ae52e995779504b061365 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Wed, 23 Feb 2022 19:39:51 -0800 Subject: [PATCH 07/24] changelog --- src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md b/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md index 18449e8d085..f847c5f9cd3 100644 --- a/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md @@ -5,6 +5,9 @@ * Adds the ability to configure `MetricReaderOptions` via the `AddInMemoryExporter` extension method. ([#2931](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2931)) +* Changes `InMemoryExporter` to clear the exported collection before exporting `Metric`s. + This prevents the same `Metric` from being repeated in the exported collection. + ([#2937](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2937)) ## 1.2.0-rc2 From 99f3aa065d2c71dfe8839c9bc67a3e881683fe79 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Thu, 24 Feb 2022 09:16:38 -0800 Subject: [PATCH 08/24] Update README.md --- docs/metrics/extending-the-sdk/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/metrics/extending-the-sdk/README.md b/docs/metrics/extending-the-sdk/README.md index a3d52afeb17..4c597f9b641 100644 --- a/docs/metrics/extending-the-sdk/README.md +++ b/docs/metrics/extending-the-sdk/README.md @@ -28,10 +28,10 @@ not covered by the built-in exporters: * Exporters should avoid generating telemetry and causing live-loop, this can be done via `OpenTelemetry.SuppressInstrumentationScope`. * Exporters receives a batch of `Metric`, and each `Metric` - can contain 1 or more `MetricPoint`s. After export, users are expected to - perform all actions with the exported Metrics and MetricPoints. Once control - is returned, exporter can no longer assume the state of the Batch. - (TODO: I need help with the statement here.) + can contain 1 or more `MetricPoint`s. The exporter should perform all actions + with the `Metric`s and `MetricsPoint`s in the Batch before returning control from + `Export`, once the control is returned, the exporter can no longer assume the + state of the Batch or anything inside it. * Exporters should use `Activity.TagObjects` collection instead of `Activity.Tags` to obtain the full set of attributes (tags). * Exporters should use `ParentProvider.GetResource()` to get the `Resource` From 93db30d8ffd345a3091b51b66646675d534ed61a Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Thu, 24 Feb 2022 10:03:18 -0800 Subject: [PATCH 09/24] Update InMemoryExporter.cs --- src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index 4a198e7abae..f1e1403bece 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -39,6 +39,10 @@ public override ExportResult Export(in Batch batch) if (this.isMetric) { + // By design, The MetricApi reuses Metrics (MetricReader.metricsCurrentBatch). + // This means that exported Metrics will always reflect the the latest values. + // Here, we clear the exported collection to prevent populating + // this with duplicate instances of the same Metric. this.exportedItems.Clear(); } From d080e2757ab2b9d8c73d71959abb1e983d43adf7 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Thu, 24 Feb 2022 10:36:28 -0800 Subject: [PATCH 10/24] fix markdownlint --- docs/metrics/extending-the-sdk/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/metrics/extending-the-sdk/README.md b/docs/metrics/extending-the-sdk/README.md index 4c597f9b641..b0f8c58ca02 100644 --- a/docs/metrics/extending-the-sdk/README.md +++ b/docs/metrics/extending-the-sdk/README.md @@ -27,10 +27,10 @@ not covered by the built-in exporters: does not implement any retry logic. * Exporters should avoid generating telemetry and causing live-loop, this can be done via `OpenTelemetry.SuppressInstrumentationScope`. -* Exporters receives a batch of `Metric`, and each `Metric` - can contain 1 or more `MetricPoint`s. The exporter should perform all actions - with the `Metric`s and `MetricsPoint`s in the Batch before returning control from - `Export`, once the control is returned, the exporter can no longer assume the +* Exporters receives a batch of `Metric`, and each `Metric` can contain 1 or + more `MetricPoint`s. The exporter should perform all actions with the + `Metric`s and `MetricsPoint`s in the Batch before returning control from + `Export`, once the control is returned, the exporter can no longer assume the state of the Batch or anything inside it. * Exporters should use `Activity.TagObjects` collection instead of `Activity.Tags` to obtain the full set of attributes (tags). From 37aa2af17a5a4c3424fea0fb9574206d5ba2d9df Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Thu, 24 Feb 2022 13:06:12 -0800 Subject: [PATCH 11/24] Update README.md --- docs/metrics/extending-the-sdk/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/metrics/extending-the-sdk/README.md b/docs/metrics/extending-the-sdk/README.md index b0f8c58ca02..1512a2a3dee 100644 --- a/docs/metrics/extending-the-sdk/README.md +++ b/docs/metrics/extending-the-sdk/README.md @@ -30,8 +30,8 @@ not covered by the built-in exporters: * Exporters receives a batch of `Metric`, and each `Metric` can contain 1 or more `MetricPoint`s. The exporter should perform all actions with the `Metric`s and `MetricsPoint`s in the Batch before returning control from - `Export`, once the control is returned, the exporter can no longer assume the - state of the Batch or anything inside it. + `Export`, once the control is returned, the exporter can no longer make any + assumptions about the state of the Batch or anything inside it. * Exporters should use `Activity.TagObjects` collection instead of `Activity.Tags` to obtain the full set of attributes (tags). * Exporters should use `ParentProvider.GetResource()` to get the `Resource` From d8a8b6e7afd60a4c31c5328b82420062e4c4f69d Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Thu, 24 Feb 2022 13:28:03 -0800 Subject: [PATCH 12/24] cleanup --- src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index f1e1403bece..39952f5c47a 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -40,7 +40,7 @@ public override ExportResult Export(in Batch batch) if (this.isMetric) { // By design, The MetricApi reuses Metrics (MetricReader.metricsCurrentBatch). - // This means that exported Metrics will always reflect the the latest values. + // This means that exported Metrics will always reflect the latest values. // Here, we clear the exported collection to prevent populating // this with duplicate instances of the same Metric. this.exportedItems.Clear(); From 3078c97701bd6f7c9da619f04476fefe0000f96d Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Thu, 24 Feb 2022 14:41:11 -0800 Subject: [PATCH 13/24] Delete InMemoryExporterTests.cs --- .../Metrics/InMemoryExporterTests.cs | 206 ------------------ 1 file changed, 206 deletions(-) delete mode 100644 test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs diff --git a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs deleted file mode 100644 index 96778396f88..00000000000 --- a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs +++ /dev/null @@ -1,206 +0,0 @@ -// -// 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.Collections.Generic; -using System.Diagnostics.Metrics; -using OpenTelemetry.Tests; -using Xunit; - -namespace OpenTelemetry.Metrics.Tests -{ - public class InMemoryExporterTests - { - [Fact] - public void Verify_MetricPoint_UsingDeltaAggregation() - { - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems, metricReaderOptions => - { - metricReaderOptions.Temporality = AggregationTemporality.Delta; - }) - .Build(); - - var counter = meter.CreateCounter("meter"); - - // Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") - counter.Add(10, new KeyValuePair("tag1", "value1")); - - meterProvider.ForceFlush(); - Assert.Single(exportedItems); - var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(10, metricPoint1.GetSumLong()); - - // Emit 25 for the MetricPoint with a single key-vaue pair: ("tag1", "value1") - counter.Add(25, new KeyValuePair("tag1", "value1")); - - meterProvider.ForceFlush(); - Assert.Single(exportedItems); - var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(25, metricPoint2.GetSumLong()); - - // Retest 1st item, this is expected to be unchanged. - Assert.Equal(10, metricPoint1.GetSumLong()); - } - - [Fact] - public void Verify_Metric_UsingCounter() - { - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - var counter = meter.CreateCounter("meter"); - - counter.Add(10); - - meterProvider.ForceFlush(); - Assert.Single(exportedItems); - var metric1 = exportedItems[0]; - - counter.Add(5); - meterProvider.ForceFlush(); - Assert.Single(exportedItems); - var metric2 = exportedItems[0]; - - // Note that although flush has been called twice - // the same metric has been exported. - // This is by design, because the MetricsApi reuses metrics. - Assert.Same(metric1, metric2); - } - - [Fact] - public void Verify_MetricPoint_UsingCounter() - { - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - var counter = meter.CreateCounter("meter"); - - counter.Add(10); - - meterProvider.ForceFlush(); - Assert.Single(exportedItems); - - counter.Add(5); - - var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(10, metricPoint1.GetSumLong()); // Note that the second counter.Add doesn't affect the sum until calling Flush. - - meterProvider.ForceFlush(); - Assert.Single(exportedItems); - - var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(15, metricPoint2.GetSumLong()); // The second MetricPoint will have the updated value. - - // Retest 1st item, this is expected to be unchanged. - Assert.Equal(10, metricPoint1.GetSumLong()); - } - - [Fact] - public void Verify_MetricPoint_UsingObservableCounter() - { - // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 - - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - int i = 0; - var counterLong = meter.CreateObservableCounter( - "observable-counter", - () => ++i * 10); - - meterProvider.ForceFlush(); - Assert.Equal(1, i); // verify that callback is invoked when calling Flush - Assert.Single(exportedItems); - var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(10, metricPoint1.GetSumLong()); - - meterProvider.ForceFlush(); - Assert.Equal(2, i); // verify that callback is invoked when calling Flush - Assert.Single(exportedItems); - var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(20, metricPoint2.GetSumLong()); - - // Retest 1st item, this is expected to be unchanged. - Assert.Equal(10, metricPoint1.GetSumLong()); - } - - [Fact] - public void Verify_MetricPoint_UsingHistograms() - { - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - var histogram = meter.CreateHistogram("histogram"); - - for (int i = 0; i < 5; i++) - { - histogram.Record(i); - } - - meterProvider.ForceFlush(); - - var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(5, metricPoint1.GetHistogramCount()); - Assert.Equal(10, metricPoint1.GetHistogramSum()); - - for (int i = 0; i < 5; i++) - { - histogram.Record(i); - } - - meterProvider.ForceFlush(); - - // Note that although metricPoint1 represents the first flush, - // this struct holds a reference to HistogramBuckets and this value has updated. - // This is by design. - Assert.Equal(5, metricPoint1.GetHistogramCount()); - Assert.Equal(20, metricPoint1.GetHistogramSum()); - } - - private static MetricPoint GetSingleMetricPoint(Metric metric) - { - var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); - Assert.True(metricPointsEnumerator.MoveNext()); // Only one MetricPoint is emitted for the Metric - var metricPoint = metricPointsEnumerator.Current; - Assert.False(metricPointsEnumerator.MoveNext()); - return metricPoint; - } - } -} From 4f8de4d153e7bbed624ccf3e11e274b41941ec2b Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Tue, 8 Mar 2022 09:24:10 -0800 Subject: [PATCH 14/24] Update docs/metrics/extending-the-sdk/README.md Co-authored-by: Cijo Thomas --- docs/metrics/extending-the-sdk/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/metrics/extending-the-sdk/README.md b/docs/metrics/extending-the-sdk/README.md index 1512a2a3dee..afd4894cf03 100644 --- a/docs/metrics/extending-the-sdk/README.md +++ b/docs/metrics/extending-the-sdk/README.md @@ -27,7 +27,7 @@ not covered by the built-in exporters: does not implement any retry logic. * Exporters should avoid generating telemetry and causing live-loop, this can be done via `OpenTelemetry.SuppressInstrumentationScope`. -* Exporters receives a batch of `Metric`, and each `Metric` can contain 1 or +* Exporters receive a batch of `Metric`, and each `Metric` can contain one or more `MetricPoint`s. The exporter should perform all actions with the `Metric`s and `MetricsPoint`s in the Batch before returning control from `Export`, once the control is returned, the exporter can no longer make any From bffb8221cceaa3532beaf484f018207291d46413 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Tue, 8 Mar 2022 09:24:15 -0800 Subject: [PATCH 15/24] Update docs/metrics/extending-the-sdk/README.md Co-authored-by: Cijo Thomas --- docs/metrics/extending-the-sdk/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/metrics/extending-the-sdk/README.md b/docs/metrics/extending-the-sdk/README.md index afd4894cf03..d24984e508c 100644 --- a/docs/metrics/extending-the-sdk/README.md +++ b/docs/metrics/extending-the-sdk/README.md @@ -28,7 +28,7 @@ not covered by the built-in exporters: * Exporters should avoid generating telemetry and causing live-loop, this can be done via `OpenTelemetry.SuppressInstrumentationScope`. * Exporters receive a batch of `Metric`, and each `Metric` can contain one or - more `MetricPoint`s. The exporter should perform all actions with the + more `MetricPoint`s. The exporter should perform all actions (e.g. serializing etc.) with the `Metric`s and `MetricsPoint`s in the Batch before returning control from `Export`, once the control is returned, the exporter can no longer make any assumptions about the state of the Batch or anything inside it. From 1fa45bdfa0b2860c520f120c4be3def29f65b285 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Tue, 8 Mar 2022 09:24:26 -0800 Subject: [PATCH 16/24] Update src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs Co-authored-by: Cijo Thomas --- src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index 39952f5c47a..7f5818c0c72 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -39,7 +39,7 @@ public override ExportResult Export(in Batch batch) if (this.isMetric) { - // By design, The MetricApi reuses Metrics (MetricReader.metricsCurrentBatch). + // The Metrics SDK reuses Metrics (MetricReader.metricsCurrentBatch). // This means that exported Metrics will always reflect the latest values. // Here, we clear the exported collection to prevent populating // this with duplicate instances of the same Metric. From fc33bb20ff9b8a734f0470d0da51b6067c0dcb63 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Tue, 8 Mar 2022 09:29:37 -0800 Subject: [PATCH 17/24] Update docs/metrics/extending-the-sdk/README.md Co-authored-by: Cijo Thomas --- docs/metrics/extending-the-sdk/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/metrics/extending-the-sdk/README.md b/docs/metrics/extending-the-sdk/README.md index d24984e508c..abc1af8f099 100644 --- a/docs/metrics/extending-the-sdk/README.md +++ b/docs/metrics/extending-the-sdk/README.md @@ -29,7 +29,7 @@ not covered by the built-in exporters: done via `OpenTelemetry.SuppressInstrumentationScope`. * Exporters receive a batch of `Metric`, and each `Metric` can contain one or more `MetricPoint`s. The exporter should perform all actions (e.g. serializing etc.) with the - `Metric`s and `MetricsPoint`s in the Batch before returning control from + `Metric`s and `MetricsPoint`s in the batch before returning control from `Export`, once the control is returned, the exporter can no longer make any assumptions about the state of the Batch or anything inside it. * Exporters should use `Activity.TagObjects` collection instead of From 58dccbc815d13a2b1717095600cf6a7fc2cf61bc Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Tue, 8 Mar 2022 09:31:15 -0800 Subject: [PATCH 18/24] markdownlint --- docs/metrics/extending-the-sdk/README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/metrics/extending-the-sdk/README.md b/docs/metrics/extending-the-sdk/README.md index abc1af8f099..3342a395444 100644 --- a/docs/metrics/extending-the-sdk/README.md +++ b/docs/metrics/extending-the-sdk/README.md @@ -28,10 +28,11 @@ not covered by the built-in exporters: * Exporters should avoid generating telemetry and causing live-loop, this can be done via `OpenTelemetry.SuppressInstrumentationScope`. * Exporters receive a batch of `Metric`, and each `Metric` can contain one or - more `MetricPoint`s. The exporter should perform all actions (e.g. serializing etc.) with the - `Metric`s and `MetricsPoint`s in the batch before returning control from - `Export`, once the control is returned, the exporter can no longer make any - assumptions about the state of the Batch or anything inside it. + more `MetricPoint`s. The exporter should perform all actions + (e.g. serializing etc.) with the `Metric`s and `MetricsPoint`s in the batch + before returning control from `Export`, once the control is returned, the + exporter can no longer make any assumptions about the state of the Batch or + anything inside it. * Exporters should use `Activity.TagObjects` collection instead of `Activity.Tags` to obtain the full set of attributes (tags). * Exporters should use `ParentProvider.GetResource()` to get the `Resource` From 697fb9f199c13fe1012aded1bbda426c1ab637db Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Thu, 17 Mar 2022 14:36:27 -0700 Subject: [PATCH 19/24] discussing an alternate approach --- .../.publicApi/net461/PublicAPI.Unshipped.txt | 4 ++ .../netstandard2.0/PublicAPI.Unshipped.txt | 4 ++ .../InMemoryExporter.cs | 22 +++------- .../InMemoryExporterMetricsExtensions.cs | 2 +- .../InMemoryMetricExporter.cs | 41 +++++++++++++++++++ .../Trace/BatchExportActivityProcessorTest.cs | 2 +- 6 files changed, 56 insertions(+), 19 deletions(-) create mode 100644 src/OpenTelemetry.Exporter.InMemory/InMemoryMetricExporter.cs diff --git a/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt index ca21920895c..30bddaff4b5 100644 --- a/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt @@ -1,3 +1,7 @@ +OpenTelemetry.Exporter.InMemoryMetricExporter +OpenTelemetry.Exporter.InMemoryMetricExporter.InMemoryMetricExporter(System.Collections.Generic.ICollection exportedItems) -> void OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions +override OpenTelemetry.Exporter.InMemoryMetricExporter.Export(in OpenTelemetry.Batch batch) -> OpenTelemetry.ExportResult +readonly OpenTelemetry.Exporter.InMemoryExporter.exportedItems -> System.Collections.Generic.ICollection static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder \ No newline at end of file diff --git a/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index ca21920895c..30bddaff4b5 100644 --- a/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,3 +1,7 @@ +OpenTelemetry.Exporter.InMemoryMetricExporter +OpenTelemetry.Exporter.InMemoryMetricExporter.InMemoryMetricExporter(System.Collections.Generic.ICollection exportedItems) -> void OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions +override OpenTelemetry.Exporter.InMemoryMetricExporter.Export(in OpenTelemetry.Batch batch) -> OpenTelemetry.ExportResult +readonly OpenTelemetry.Exporter.InMemoryExporter.exportedItems -> System.Collections.Generic.ICollection static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder \ No newline at end of file diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index 39952f5c47a..cc96d5c13dc 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -16,36 +16,24 @@ using System.Collections.Generic; +using OpenTelemetry.Internal; + namespace OpenTelemetry.Exporter { public class InMemoryExporter : BaseExporter where T : class { - private readonly ICollection exportedItems; - private readonly bool isMetric; + protected readonly ICollection exportedItems; public InMemoryExporter(ICollection exportedItems) { + Guard.ThrowIfNull(exportedItems); + this.exportedItems = exportedItems; - this.isMetric = typeof(T) == typeof(Metrics.Metric); } public override ExportResult Export(in Batch batch) { - if (this.exportedItems == null) - { - return ExportResult.Failure; - } - - if (this.isMetric) - { - // By design, The MetricApi reuses Metrics (MetricReader.metricsCurrentBatch). - // This means that exported Metrics will always reflect the latest values. - // Here, we clear the exported collection to prevent populating - // this with duplicate instances of the same Metric. - this.exportedItems.Clear(); - } - foreach (var data in batch) { this.exportedItems.Add(data); diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporterMetricsExtensions.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporterMetricsExtensions.cs index b43d602df8d..e98c7d0796d 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporterMetricsExtensions.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporterMetricsExtensions.cs @@ -79,7 +79,7 @@ private static MeterProviderBuilder AddInMemoryExporter( { configureMetricReader?.Invoke(metricReaderOptions); - var metricExporter = new InMemoryExporter(exportedItems); + var metricExporter = new InMemoryMetricExporter(exportedItems); if (metricReaderOptions.MetricReaderType == (MetricReaderType)(-1)) { diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryMetricExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryMetricExporter.cs new file mode 100644 index 00000000000..69a4ec3df11 --- /dev/null +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryMetricExporter.cs @@ -0,0 +1,41 @@ +// +// 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.Collections.Generic; + +using OpenTelemetry.Metrics; + +namespace OpenTelemetry.Exporter +{ + public class InMemoryMetricExporter : InMemoryExporter + { + public InMemoryMetricExporter(ICollection exportedItems) + : base(exportedItems) + { + } + + public override ExportResult Export(in Batch batch) + { + // By design, The MetricApi reuses Metrics (MetricReader.metricsCurrentBatch). + // This means that exported Metrics will always reflect the latest values. + // Here, we clear the exported collection to prevent populating + // this with duplicate instances of the same Metric. + this.exportedItems.Clear(); + + return base.Export(batch); + } + } +} diff --git a/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorTest.cs b/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorTest.cs index 38cacff2a15..6af03bf8826 100644 --- a/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorTest.cs +++ b/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorTest.cs @@ -180,7 +180,7 @@ public void CheckExportForRecordingButNotSampledActivity() [Fact] public void CheckExportDrainsBatchOnFailure() { - using var exporter = new InMemoryExporter(null); + using var exporter = new InMemoryExporter(new List()); using var processor = new BatchActivityExportProcessor( exporter, maxQueueSize: 3, From 4f3f330cf7fcb3518540ce07127ada631f04e948 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Fri, 18 Mar 2022 15:32:13 -0700 Subject: [PATCH 20/24] clean up public api --- .../.publicApi/net461/PublicAPI.Unshipped.txt | 6 +----- .../.publicApi/netstandard2.0/PublicAPI.Unshipped.txt | 6 +----- src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs | 6 +++--- .../InMemoryMetricExporter.cs | 4 ++-- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt index 30bddaff4b5..3fd55d06d0a 100644 --- a/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt @@ -1,7 +1,3 @@ -OpenTelemetry.Exporter.InMemoryMetricExporter -OpenTelemetry.Exporter.InMemoryMetricExporter.InMemoryMetricExporter(System.Collections.Generic.ICollection exportedItems) -> void OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions -override OpenTelemetry.Exporter.InMemoryMetricExporter.Export(in OpenTelemetry.Batch batch) -> OpenTelemetry.ExportResult -readonly OpenTelemetry.Exporter.InMemoryExporter.exportedItems -> System.Collections.Generic.ICollection static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder -static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder \ No newline at end of file +static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder diff --git a/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index 30bddaff4b5..3fd55d06d0a 100644 --- a/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,7 +1,3 @@ -OpenTelemetry.Exporter.InMemoryMetricExporter -OpenTelemetry.Exporter.InMemoryMetricExporter.InMemoryMetricExporter(System.Collections.Generic.ICollection exportedItems) -> void OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions -override OpenTelemetry.Exporter.InMemoryMetricExporter.Export(in OpenTelemetry.Batch batch) -> OpenTelemetry.ExportResult -readonly OpenTelemetry.Exporter.InMemoryExporter.exportedItems -> System.Collections.Generic.ICollection static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder -static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder \ No newline at end of file +static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index cc96d5c13dc..97847411b0f 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -23,20 +23,20 @@ namespace OpenTelemetry.Exporter public class InMemoryExporter : BaseExporter where T : class { - protected readonly ICollection exportedItems; + internal readonly ICollection ExportedItems; public InMemoryExporter(ICollection exportedItems) { Guard.ThrowIfNull(exportedItems); - this.exportedItems = exportedItems; + this.ExportedItems = exportedItems; } public override ExportResult Export(in Batch batch) { foreach (var data in batch) { - this.exportedItems.Add(data); + this.ExportedItems.Add(data); } return ExportResult.Success; diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryMetricExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryMetricExporter.cs index 69a4ec3df11..759c6451122 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryMetricExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryMetricExporter.cs @@ -20,7 +20,7 @@ namespace OpenTelemetry.Exporter { - public class InMemoryMetricExporter : InMemoryExporter + internal class InMemoryMetricExporter : InMemoryExporter { public InMemoryMetricExporter(ICollection exportedItems) : base(exportedItems) @@ -33,7 +33,7 @@ public override ExportResult Export(in Batch batch) // This means that exported Metrics will always reflect the latest values. // Here, we clear the exported collection to prevent populating // this with duplicate instances of the same Metric. - this.exportedItems.Clear(); + this.ExportedItems.Clear(); return base.Export(batch); } From c91f522fcc3ec039f0262de1e313915204d22d69 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Fri, 18 Mar 2022 15:34:49 -0700 Subject: [PATCH 21/24] remove whitespace From 735b297554aafcbc9b2b5371eb8375610adb3e60 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Fri, 18 Mar 2022 15:35:03 -0700 Subject: [PATCH 22/24] remove whitespace From 5de8c9c47bcbd0292ee885f53389ed8b31205d2f Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Fri, 18 Mar 2022 15:39:03 -0700 Subject: [PATCH 23/24] fix whitespace --- .../.publicApi/net461/PublicAPI.Unshipped.txt | 2 +- .../.publicApi/netstandard2.0/PublicAPI.Unshipped.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt index 3fd55d06d0a..ca21920895c 100644 --- a/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.InMemory/.publicApi/net461/PublicAPI.Unshipped.txt @@ -1,3 +1,3 @@ OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder -static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder +static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder \ No newline at end of file diff --git a/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index 3fd55d06d0a..ca21920895c 100644 --- a/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.InMemory/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,3 +1,3 @@ OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder -static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder +static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection exportedItems, System.Action configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder \ No newline at end of file From 83af046c9dd97c922dcf4a379e8e36d1a1cb24bb Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Fri, 18 Mar 2022 15:40:57 -0700 Subject: [PATCH 24/24] update changelog --- src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md b/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md index 7766265037e..f65c898bcd3 100644 --- a/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md @@ -9,7 +9,8 @@ Released 2022-Mar-04 * Adds the ability to configure `MetricReaderOptions` via the `AddInMemoryExporter` extension method. ([#2931](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2931)) -* Changes `InMemoryExporter` to clear the exported collection before exporting `Metric`s. +* Add `InMemoryMetricExporter` which inherits `InMemoryExporter`. + This new class will clear the exported collection before exporting `Metric`s. This prevents the same `Metric` from being repeated in the exported collection. ([#2937](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2937))