From 47c54af3939fd2c0d640ecb9c14d63c56d198b4a Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Thu, 14 Apr 2022 19:46:54 -0700 Subject: [PATCH 1/2] alternate approach, adding selective copy to Metric --- .../InMemoryExporter.cs | 14 +++++++++++++- .../OpenTelemetry.Exporter.InMemory.csproj | 4 ++-- src/OpenTelemetry/AssemblyInfo.cs | 1 + src/OpenTelemetry/Metrics/AggregatorStore.cs | 10 ++++++++++ src/OpenTelemetry/Metrics/HistogramBuckets.cs | 1 + src/OpenTelemetry/Metrics/Metric.cs | 12 ++++++++++++ 6 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index d76995f8fb6..09d64a79a72 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -16,6 +16,8 @@ using System.Collections.Generic; +using OpenTelemetry.Metrics; + namespace OpenTelemetry.Exporter { public class InMemoryExporter : BaseExporter @@ -37,7 +39,17 @@ public override ExportResult Export(in Batch batch) foreach (var data in batch) { - this.exportedItems.Add(data); + if (data is Metric metric) + { + // By design, The MetricApi reuses Metrics (see: MetricReader.metricsCurrentBatch). + // Here we need to copy the metric to be exported to + // prevent the exported instance from being updated. + this.exportedItems.Add(metric.Copy() as T); + } + else + { + this.exportedItems.Add(data); + } } return ExportResult.Success; diff --git a/src/OpenTelemetry.Exporter.InMemory/OpenTelemetry.Exporter.InMemory.csproj b/src/OpenTelemetry.Exporter.InMemory/OpenTelemetry.Exporter.InMemory.csproj index 308038dc43a..7ab848e6cb4 100644 --- a/src/OpenTelemetry.Exporter.InMemory/OpenTelemetry.Exporter.InMemory.csproj +++ b/src/OpenTelemetry.Exporter.InMemory/OpenTelemetry.Exporter.InMemory.csproj @@ -17,8 +17,8 @@ - - + diff --git a/src/OpenTelemetry/AssemblyInfo.cs b/src/OpenTelemetry/AssemblyInfo.cs index 5b17a4357a6..545b35a13e8 100644 --- a/src/OpenTelemetry/AssemblyInfo.cs +++ b/src/OpenTelemetry/AssemblyInfo.cs @@ -17,6 +17,7 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("OpenTelemetry.Tests" + AssemblyInfo.PublicKey)] +[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.InMemory" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Prometheus" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Prometheus.Tests" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Extensions.Hosting.Tests" + AssemblyInfo.PublicKey)] diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 1daa4414ead..b720e9df11a 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -80,6 +80,14 @@ internal AggregatorStore( } } + private AggregatorStore(AggregatorStore other) + { + this.batchSize = other.batchSize; + + this.currentMetricPointBatch = (int[])other.currentMetricPointBatch.Clone(); + this.metricPoints = Array.ConvertAll(other.metricPoints, metricPoints => metricPoints.Copy()); + } + private delegate void UpdateLongDelegate(long value, ReadOnlySpan> tags); private delegate void UpdateDoubleDelegate(double value, ReadOnlySpan> tags); @@ -155,6 +163,8 @@ internal void SnapshotCumulative(int indexSnapshot) internal MetricPointsAccessor GetMetricPoints() => new(this.metricPoints, this.currentMetricPointBatch, this.batchSize); + internal AggregatorStore Copy() => new(this); + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void InitializeZeroTagPointIfNotInitialized() { diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs index 626779d4901..42db69f9972 100644 --- a/src/OpenTelemetry/Metrics/HistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -52,6 +52,7 @@ internal HistogramBuckets Copy() HistogramBuckets copy = new HistogramBuckets(this.ExplicitBounds); Array.Copy(this.SnapshotBucketCounts, copy.SnapshotBucketCounts, this.SnapshotBucketCounts.Length); + Array.Copy(this.RunningBucketCounts, copy.RunningBucketCounts, this.RunningBucketCounts.Length); copy.SnapshotSum = this.SnapshotSum; return copy; diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 61443173f70..1cbae4470b5 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -110,6 +110,13 @@ internal Metric( this.InstrumentDisposed = false; } + private Metric(Metric other) + { + this.MetricType = other.MetricType; + this.InstrumentIdentity = other.InstrumentIdentity; + this.aggStore = other.aggStore.Copy(); + } + public MetricType MetricType { get; private set; } public AggregationTemporality Temporality { get; private set; } @@ -147,5 +154,10 @@ internal int Snapshot() { return this.aggStore.Snapshot(); } + + internal Metric Copy() + { + return new Metric(this); + } } } From a5a6c47661e0f67ca0df3c9e7a07e799df1c7e31 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Fri, 15 Apr 2022 13:10:27 -0700 Subject: [PATCH 2/2] minor fix --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 1 + src/OpenTelemetry/Metrics/Metric.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index b720e9df11a..32feea8ace9 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -82,6 +82,7 @@ internal AggregatorStore( private AggregatorStore(AggregatorStore other) { + this.name = other.name; this.batchSize = other.batchSize; this.currentMetricPointBatch = (int[])other.currentMetricPointBatch.Clone(); diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 1cbae4470b5..6b44b2fadf2 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -115,6 +115,7 @@ private Metric(Metric other) this.MetricType = other.MetricType; this.InstrumentIdentity = other.InstrumentIdentity; this.aggStore = other.aggStore.Copy(); + this.Temporality = other.Temporality; } public MetricType MetricType { get; private set; }