Minor refactor MetricReader and AggregationTemporality (#2357)

open-telemetry · Sep 17, 2021 · da54f8c · da54f8c
1 parent 34d8332
commit da54f8c
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 23 deletions.
diff --git a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs
@@ -23,7 +23,7 @@
 
 namespace OpenTelemetry.Exporter
 {
-    [AggregationTemporality(AggregationTemporality.Both, AggregationTemporality.Cumulative)]
+    [AggregationTemporality(AggregationTemporality.Cumulative | AggregationTemporality.Delta, AggregationTemporality.Cumulative)]
     public class ConsoleMetricExporter : ConsoleExporter<Metric>
     {
         private Resource resource;

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricsExporter.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricsExporter.cs
@@ -32,7 +32,7 @@ namespace OpenTelemetry.Exporter
     /// Exporter consuming <see cref="Metric"/> and exporting the data using
     /// the OpenTelemetry protocol (OTLP).
     /// </summary>
-    [AggregationTemporality(AggregationTemporality.Both, AggregationTemporality.Cumulative)]
+    [AggregationTemporality(AggregationTemporality.Cumulative | AggregationTemporality.Delta, AggregationTemporality.Cumulative)]
     public class OtlpMetricsExporter : BaseOtlpExporter<Metric>
     {
         private readonly OtlpCollector.MetricsService.IMetricsServiceClient metricsClient;

diff --git a/src/OpenTelemetry/Metrics/AggregationTemporality.cs b/src/OpenTelemetry/Metrics/AggregationTemporality.cs
@@ -30,15 +30,5 @@ public enum AggregationTemporality : byte
         /// Delta.
         /// </summary>
         Delta = 0b10,
-
-        /// <summary>
-        /// Neither.
-        /// </summary>
-        Neither = 0b0,
-
-        /// <summary>
-        /// Both.
-        /// </summary>
-        Both = 0b11,
     }
 }
diff --git a/src/OpenTelemetry/Metrics/AggregationTemporalityAttribute.cs b/src/OpenTelemetry/Metrics/AggregationTemporalityAttribute.cs
@@ -18,6 +18,7 @@
 
 namespace OpenTelemetry.Metrics
 {
+    [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
     public sealed class AggregationTemporalityAttribute : Attribute
     {
         private AggregationTemporality preferredAggregationTemporality;

diff --git a/src/OpenTelemetry/Metrics/BaseExportingMetricReader.cs b/src/OpenTelemetry/Metrics/BaseExportingMetricReader.cs
@@ -21,9 +21,8 @@ namespace OpenTelemetry.Metrics
     public class BaseExportingMetricReader : MetricReader
     {
         protected readonly BaseExporter<Metric> exporter;
-        protected bool disposed;
-
         private readonly ExportModes supportedExportModes = ExportModes.Push | ExportModes.Pull;
+        private bool disposed;
 
         public BaseExportingMetricReader(BaseExporter<Metric> exporter)
         {
@@ -62,9 +61,12 @@ internal override void SetParentProvider(BaseProvider parentProvider)
         /// <inheritdoc/>
         protected override void Dispose(bool disposing)
         {
-            base.Dispose(disposing);
+            if (this.disposed)
+            {
+                return;
+            }
 
-            if (disposing && !this.disposed)
+            if (disposing)
             {
                 try
                 {
@@ -74,9 +76,11 @@ protected override void Dispose(bool disposing)
                 {
                     // TODO: Log
                 }
-
-                this.disposed = true;
             }
+
+            this.disposed = true;
+
+            base.Dispose(disposing);
         }
     }
 }
diff --git a/src/OpenTelemetry/Metrics/MetricReader.cs b/src/OpenTelemetry/Metrics/MetricReader.cs
@@ -20,8 +20,9 @@ namespace OpenTelemetry.Metrics
 {
     public abstract class MetricReader : IDisposable
     {
-        private AggregationTemporality preferredAggregationTemporality = AggregationTemporality.Both;
-        private AggregationTemporality supportedAggregationTemporality = AggregationTemporality.Both;
+        private const AggregationTemporality CumulativeAndDelta = AggregationTemporality.Cumulative | AggregationTemporality.Delta;
+        private AggregationTemporality preferredAggregationTemporality = CumulativeAndDelta;
+        private AggregationTemporality supportedAggregationTemporality = CumulativeAndDelta;
 
         public BaseProvider ParentProvider { get; private set; }
 
@@ -74,12 +75,12 @@ protected virtual void Dispose(bool disposing)
 
         private static void ValidateAggregationTemporality(AggregationTemporality preferred, AggregationTemporality supported)
         {
-            if ((int)(preferred & AggregationTemporality.Both) == 0)
+            if ((int)(preferred & CumulativeAndDelta) == 0)
             {
                 throw new ArgumentException($"PreferredAggregationTemporality has an invalid value {preferred}.", nameof(preferred));
             }
 
-            if ((int)(supported & AggregationTemporality.Both) == 0)
+            if ((int)(supported & CumulativeAndDelta) == 0)
             {
                 throw new ArgumentException($"SupportedAggregationTemporality has an invalid value {supported}.", nameof(supported));
             }

diff --git a/src/OpenTelemetry/Metrics/PeriodicExportingMetricReader.cs b/src/OpenTelemetry/Metrics/PeriodicExportingMetricReader.cs
@@ -27,6 +27,7 @@ public class PeriodicExportingMetricReader : BaseExportingMetricReader
 
         private readonly Task exportTask;
         private readonly CancellationTokenSource token;
+        private bool disposed;
 
         public PeriodicExportingMetricReader(
             BaseExporter<Metric> exporter,
@@ -62,7 +63,12 @@ public override void OnCollect(Batch<Metric> metrics)
         /// <inheritdoc/>
         protected override void Dispose(bool disposing)
         {
-            if (disposing && !this.disposed)
+            if (this.disposed)
+            {
+                return;
+            }
+
+            if (disposing)
             {
                 try
                 {
@@ -76,6 +82,8 @@ protected override void Dispose(bool disposing)
                 }
             }
 
+            this.disposed = true;
+
             base.Dispose(disposing);
         }
     }