Clean up metric reader options (#3038)

open-telemetry · Mar 15, 2022 · a709cfd · a709cfd
1 parent bab645f
commit a709cfd
Show file tree

Hide file tree

Showing 19 changed files with 96 additions and 80 deletions.
diff --git a/examples/AspNet/Global.asax.cs b/examples/AspNet/Global.asax.cs
@@ -93,7 +93,6 @@ protected void Application_Start()
 
                         // The ConsoleMetricExporter defaults to a manual collect cycle.
                         // This configuration causes metrics to be exported to stdout on a 10s interval.
-                        metricReaderOptions.MetricReaderType = MetricReaderType.Periodic;
                         metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 10000;
                     });
                     break;

diff --git a/examples/AspNetCore/Program.cs b/examples/AspNetCore/Program.cs
@@ -135,7 +135,6 @@
             {
                 // The ConsoleMetricExporter defaults to a manual collect cycle.
                 // This configuration causes metrics to be exported to stdout on a 10s interval.
-                metricReaderOptions.MetricReaderType = MetricReaderType.Periodic;
                 metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 10000;
             });
             break;

diff --git a/examples/Console/TestMetrics.cs b/examples/Console/TestMetrics.cs
@@ -70,7 +70,6 @@ internal static object Run(MetricsOptions options)
                     {
                         exporterOptions.Protocol = options.UseGrpc ? OtlpExportProtocol.Grpc : OtlpExportProtocol.HttpProtobuf;
 
-                        metricReaderOptions.MetricReaderType = MetricReaderType.Periodic;
                         metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = options.DefaultCollectionPeriodMilliseconds;
                         metricReaderOptions.Temporality = options.IsDelta ? AggregationTemporality.Delta : AggregationTemporality.Cumulative;
                     });
@@ -82,7 +81,6 @@ internal static object Run(MetricsOptions options)
                     {
                         exporterOptions.Targets = ConsoleExporterOutputTargets.Console;
 
-                        metricReaderOptions.MetricReaderType = MetricReaderType.Periodic;
                         metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = options.DefaultCollectionPeriodMilliseconds;
                         metricReaderOptions.Temporality = options.IsDelta ? AggregationTemporality.Delta : AggregationTemporality.Cumulative;
                     });

diff --git a/src/OpenTelemetry.Exporter.Console/ConsoleExporterMetricsExtensions.cs b/src/OpenTelemetry.Exporter.Console/ConsoleExporterMetricsExtensions.cs
@@ -15,6 +15,7 @@
 // </copyright>
 
 using System;
+using System.Threading;
 using OpenTelemetry.Exporter;
 using OpenTelemetry.Internal;
 
@@ -25,6 +26,9 @@ namespace OpenTelemetry.Metrics
     /// </summary>
     public static class ConsoleExporterMetricsExtensions
     {
+        private const int DefaultExportIntervalMilliseconds = Timeout.Infinite;
+        private const int DefaultExportTimeoutMilliseconds = Timeout.Infinite;
+
         /// <summary>
         /// Adds <see cref="ConsoleMetricExporter"/> to the <see cref="MeterProviderBuilder"/> using default options.
         /// </summary>
@@ -97,13 +101,12 @@ private static MeterProviderBuilder AddConsoleExporter(
 
             var metricExporter = new ConsoleMetricExporter(exporterOptions);
 
-            var metricReader = metricReaderOptions.MetricReaderType == MetricReaderType.Manual
-                ? new BaseExportingMetricReader(metricExporter)
-                : new PeriodicExportingMetricReader(
-                    metricExporter,
-                    metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds ?? -1);
+            var metricReader = PeriodicExportingMetricReaderHelper.CreatePeriodicExportingMetricReader(
+                metricExporter,
+                metricReaderOptions,
+                DefaultExportIntervalMilliseconds,
+                DefaultExportTimeoutMilliseconds);
 
-            metricReader.Temporality = metricReaderOptions.Temporality;
             return builder.AddReader(metricReader);
         }
     }

diff --git a/src/OpenTelemetry.Exporter.Console/OpenTelemetry.Exporter.Console.csproj b/src/OpenTelemetry.Exporter.Console/OpenTelemetry.Exporter.Console.csproj
@@ -17,6 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PeriodicExportingMetricReaderHelper.cs" Link="Includes\PeriodicExportingMetricReaderHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\ServiceProviderExtensions.cs" Link="Includes\ServiceProviderExtensions.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\Guard.cs" Link="Includes\Guard.cs" />
   </ItemGroup>

diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporterMetricsExtensions.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporterMetricsExtensions.cs
@@ -16,6 +16,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Threading;
 using OpenTelemetry.Exporter;
 using OpenTelemetry.Internal;
 
@@ -26,6 +27,9 @@ namespace OpenTelemetry.Metrics
     /// </summary>
     public static class InMemoryExporterMetricsExtensions
     {
+        private const int DefaultExportIntervalMilliseconds = Timeout.Infinite;
+        private const int DefaultExportTimeoutMilliseconds = Timeout.Infinite;
+
         /// <summary>
         /// Adds InMemory metric exporter to the <see cref="MeterProviderBuilder"/> using default options.
         /// </summary>
@@ -81,13 +85,12 @@ private static MeterProviderBuilder AddInMemoryExporter(
 
             var metricExporter = new InMemoryExporter<Metric>(exportedItems);
 
-            var metricReader = metricReaderOptions.MetricReaderType == MetricReaderType.Manual
-                ? new BaseExportingMetricReader(metricExporter)
-                : new PeriodicExportingMetricReader(
-                    metricExporter,
-                    metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds ?? -1);
+            var metricReader = PeriodicExportingMetricReaderHelper.CreatePeriodicExportingMetricReader(
+                metricExporter,
+                metricReaderOptions,
+                DefaultExportIntervalMilliseconds,
+                DefaultExportTimeoutMilliseconds);
 
-            metricReader.Temporality = metricReaderOptions.Temporality;
             return builder.AddReader(metricReader);
         }
     }

diff --git a/src/OpenTelemetry.Exporter.InMemory/OpenTelemetry.Exporter.InMemory.csproj b/src/OpenTelemetry.Exporter.InMemory/OpenTelemetry.Exporter.InMemory.csproj
@@ -17,6 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PeriodicExportingMetricReaderHelper.cs" Link="Includes\PeriodicExportingMetricReaderHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\ServiceProviderExtensions.cs" Link="Includes\ServiceProviderExtensions.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\Guard.cs" Link="Includes\Guard.cs" />
   </ItemGroup>

diff --git a/...emetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj b/...emetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj
@@ -46,6 +46,7 @@
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\Guard.cs" Link="Includes\Guard.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\EnvironmentVariableHelper.cs" Link="Includes\EnvironmentVariableHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\OpenTelemetrySdkEventSource.cs" Link="Includes\OpenTelemetrySdkEventSource.cs" />
+    <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PeriodicExportingMetricReaderHelper.cs" Link="Includes\PeriodicExportingMetricReaderHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PooledList.cs" Link="Includes\PooledList.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PeerServiceResolver.cs" Link="Includes\PeerServiceResolver.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\ResourceSemanticConventions.cs" Link="Includes\ResourceSemanticConventions.cs" />

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporterExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporterExtensions.cs
@@ -26,6 +26,7 @@ namespace OpenTelemetry.Metrics
     public static class OtlpMetricExporterExtensions
     {
         private const int DefaultExportIntervalMilliseconds = 60000;
+        private const int DefaultExportTimeoutMilliseconds = 30000;
 
         /// <summary>
         /// Adds <see cref="OtlpMetricExporter"/> to the <see cref="MeterProviderBuilder"/> using default options.
@@ -104,13 +105,12 @@ private static MeterProviderBuilder AddOtlpExporter(
 
             var metricExporter = new OtlpMetricExporter(exporterOptions);
 
-            var metricReader = metricReaderOptions.MetricReaderType == MetricReaderType.Manual
-                ? new BaseExportingMetricReader(metricExporter)
-                : new PeriodicExportingMetricReader(
-                    metricExporter,
-                    metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds ?? DefaultExportIntervalMilliseconds);
+            var metricReader = PeriodicExportingMetricReaderHelper.CreatePeriodicExportingMetricReader(
+                metricExporter,
+                metricReaderOptions,
+                DefaultExportIntervalMilliseconds,
+                DefaultExportTimeoutMilliseconds);
 
-            metricReader.Temporality = metricReaderOptions.Temporality;
             return builder.AddReader(metricReader);
         }
     }

diff --git a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt
@@ -3,10 +3,7 @@ OpenTelemetry.Batch<T>.Batch(T[] items, int count) -> void
 OpenTelemetry.Batch<T>.Count.get -> long
 OpenTelemetry.Metrics.MetricReaderOptions
 OpenTelemetry.Metrics.MetricReaderOptions.MetricReaderOptions() -> void
-OpenTelemetry.Metrics.MetricReaderOptions.MetricReaderType.get -> OpenTelemetry.Metrics.MetricReaderType
-OpenTelemetry.Metrics.MetricReaderOptions.MetricReaderType.set -> void
 OpenTelemetry.Metrics.MetricReaderOptions.PeriodicExportingMetricReaderOptions.get -> OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions
-OpenTelemetry.Metrics.MetricReaderOptions.PeriodicExportingMetricReaderOptions.set -> void
 OpenTelemetry.Metrics.MetricReaderOptions.Temporality.get -> OpenTelemetry.Metrics.AggregationTemporality
 OpenTelemetry.Metrics.MetricReaderOptions.Temporality.set -> void
 OpenTelemetry.Metrics.AggregationTemporality
@@ -81,9 +78,6 @@ OpenTelemetry.Metrics.MetricReader.MetricReader() -> void
 OpenTelemetry.Metrics.MetricReader.Shutdown(int timeoutMilliseconds = -1) -> bool
 OpenTelemetry.Metrics.MetricReader.Temporality.get -> OpenTelemetry.Metrics.AggregationTemporality
 OpenTelemetry.Metrics.MetricReader.Temporality.set -> void
-OpenTelemetry.Metrics.MetricReaderType
-OpenTelemetry.Metrics.MetricReaderType.Manual = 0 -> OpenTelemetry.Metrics.MetricReaderType
-OpenTelemetry.Metrics.MetricReaderType.Periodic = 1 -> OpenTelemetry.Metrics.MetricReaderType
 OpenTelemetry.Metrics.MetricStreamConfiguration
 OpenTelemetry.Metrics.MetricStreamConfiguration.Description.get -> string
 OpenTelemetry.Metrics.MetricStreamConfiguration.Description.set -> void
@@ -105,6 +99,8 @@ OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions
 OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.PeriodicExportingMetricReaderOptions() -> void
 OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.get -> int?
 OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.set -> void
+OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportTimeoutMilliseconds.get -> int?
+OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportTimeoutMilliseconds.set -> void
 OpenTelemetry.ReadOnlyTagCollection
 OpenTelemetry.ReadOnlyTagCollection.Count.get -> int
 OpenTelemetry.ReadOnlyTagCollection.Enumerator

diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt
@@ -3,10 +3,7 @@ OpenTelemetry.Batch<T>.Batch(T[] items, int count) -> void
 OpenTelemetry.Batch<T>.Count.get -> long
 OpenTelemetry.Metrics.MetricReaderOptions
 OpenTelemetry.Metrics.MetricReaderOptions.MetricReaderOptions() -> void
-OpenTelemetry.Metrics.MetricReaderOptions.MetricReaderType.get -> OpenTelemetry.Metrics.MetricReaderType
-OpenTelemetry.Metrics.MetricReaderOptions.MetricReaderType.set -> void
 OpenTelemetry.Metrics.MetricReaderOptions.PeriodicExportingMetricReaderOptions.get -> OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions
-OpenTelemetry.Metrics.MetricReaderOptions.PeriodicExportingMetricReaderOptions.set -> void
 OpenTelemetry.Metrics.MetricReaderOptions.Temporality.get -> OpenTelemetry.Metrics.AggregationTemporality
 OpenTelemetry.Metrics.MetricReaderOptions.Temporality.set -> void
 OpenTelemetry.Metrics.AggregationTemporality
@@ -81,9 +78,6 @@ OpenTelemetry.Metrics.MetricReader.MetricReader() -> void
 OpenTelemetry.Metrics.MetricReader.Shutdown(int timeoutMilliseconds = -1) -> bool
 OpenTelemetry.Metrics.MetricReader.Temporality.get -> OpenTelemetry.Metrics.AggregationTemporality
 OpenTelemetry.Metrics.MetricReader.Temporality.set -> void
-OpenTelemetry.Metrics.MetricReaderType
-OpenTelemetry.Metrics.MetricReaderType.Manual = 0 -> OpenTelemetry.Metrics.MetricReaderType
-OpenTelemetry.Metrics.MetricReaderType.Periodic = 1 -> OpenTelemetry.Metrics.MetricReaderType
 OpenTelemetry.Metrics.MetricStreamConfiguration
 OpenTelemetry.Metrics.MetricStreamConfiguration.Description.get -> string
 OpenTelemetry.Metrics.MetricStreamConfiguration.Description.set -> void
@@ -105,6 +99,8 @@ OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions
 OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.PeriodicExportingMetricReaderOptions() -> void
 OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.get -> int?
 OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.set -> void
+OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportTimeoutMilliseconds.get -> int?
+OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportTimeoutMilliseconds.set -> void
 OpenTelemetry.ReadOnlyTagCollection
 OpenTelemetry.ReadOnlyTagCollection.Count.get -> int
 OpenTelemetry.ReadOnlyTagCollection.Enumerator

diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md
@@ -7,6 +7,19 @@
   period.
   ([#2982](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2982))
 
+* Added the `PeriodicExportingMetricReaderOptions.ExportTimeoutMilliseconds`
+  option.
+  ([#3038](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3038))
+
+* Removed `MetricReaderType`. This enumeration was previously used when
+  configuing a metric reader with an exporter to configure whether the export
+  cycle would be periodic or manual (i.e., requiring a explicit call to flush
+  metrics). This change affects the push-based metric exporters: OTLP, Console,
+  and InMemory. For these exporters, a manual export cycle can now be achieved
+  by setting `PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds`
+  to `-1`.
+  ([#3038](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3038))
+
 ## 1.2.0-rc3
 
 Released 2022-Mar-04

diff --git a/src/OpenTelemetry/Internal/PeriodicExportingMetricReaderHelper.cs b/src/OpenTelemetry/Internal/PeriodicExportingMetricReaderHelper.cs
@@ -0,0 +1,40 @@
+// <copyright file="PeriodicExportingMetricReaderHelper.cs" company="OpenTelemetry Authors">
+// 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.
+// </copyright>
+
+namespace OpenTelemetry.Metrics;
+
+internal static class PeriodicExportingMetricReaderHelper
+{
+    internal static PeriodicExportingMetricReader CreatePeriodicExportingMetricReader(
+        BaseExporter<Metric> exporter,
+        MetricReaderOptions options,
+        int defaultExportIntervalMilliseconds,
+        int defaultExportTimeoutMilliseconds)
+    {
+        var exportInterval =
+            options.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds
+            ?? defaultExportIntervalMilliseconds;
+
+        var exportTimeout =
+            options.PeriodicExportingMetricReaderOptions.ExportTimeoutMilliseconds
+            ?? defaultExportTimeoutMilliseconds;
+
+        var metricReader = new PeriodicExportingMetricReader(exporter, exportInterval, exportTimeout);
+        metricReader.Temporality = options.Temporality;
+
+        return metricReader;
+    }
+}
diff --git a/src/OpenTelemetry/Metrics/MetricReaderOptions.cs b/src/OpenTelemetry/Metrics/MetricReaderOptions.cs
@@ -28,12 +28,7 @@ public class MetricReaderOptions
     public AggregationTemporality Temporality { get; set; } = AggregationTemporality.Cumulative;
 
     /// <summary>
-    /// Gets or sets the <see cref="MetricReaderType" /> to use. Defaults to <c>MetricReaderType.Periodic</c>.
+    /// Gets the <see cref="PeriodicExportingMetricReaderOptions" /> options.
     /// </summary>
-    public MetricReaderType MetricReaderType { get; set; } = MetricReaderType.Periodic;
-
-    /// <summary>
-    /// Gets or sets the <see cref="PeriodicExportingMetricReaderOptions" /> options. Ignored unless <c>MetricReaderType</c> is <c>Periodic</c>.
-    /// </summary>
-    public PeriodicExportingMetricReaderOptions PeriodicExportingMetricReaderOptions { get; set; } = new PeriodicExportingMetricReaderOptions();
+    public PeriodicExportingMetricReaderOptions PeriodicExportingMetricReaderOptions { get; private set; } = new PeriodicExportingMetricReaderOptions();
 }
diff --git a/src/OpenTelemetry/Metrics/MetricReaderType.cs b/src/OpenTelemetry/Metrics/MetricReaderType.cs
diff --git a/src/OpenTelemetry/Metrics/PeriodicExportingMetricReader.cs b/src/OpenTelemetry/Metrics/PeriodicExportingMetricReader.cs
@@ -52,7 +52,7 @@ public PeriodicExportingMetricReader(
         {
             Guard.ThrowIfInvalidTimeout(exportIntervalMilliseconds);
             Guard.ThrowIfZero(exportIntervalMilliseconds);
-            Guard.ThrowIfOutOfRange(exportTimeoutMilliseconds, min: 0);
+            Guard.ThrowIfInvalidTimeout(exportTimeoutMilliseconds);
 
             if ((this.SupportedExportModes & ExportModes.Push) != ExportModes.Push)
             {

diff --git a/src/OpenTelemetry/Metrics/PeriodicExportingMetricReaderOptions.cs b/src/OpenTelemetry/Metrics/PeriodicExportingMetricReaderOptions.cs
@@ -20,7 +20,16 @@ public class PeriodicExportingMetricReaderOptions
     {
         /// <summary>
         /// Gets or sets the metric export interval in milliseconds.
+        /// If not set, the default value depends on the type of metric exporter
+        /// associated with the metric reader.
         /// </summary>
         public int? ExportIntervalMilliseconds { get; set; }
+
+        /// <summary>
+        /// Gets or sets the metric export timeout in milliseconds.
+        /// If not set, the default value depends on the type of metric exporter
+        /// associated with the metric reader.
+        /// </summary>
+        public int? ExportTimeoutMilliseconds { get; set; }
     }
 }
diff --git a/test/Benchmarks/Metrics/HistogramBenchmarks.cs b/test/Benchmarks/Metrics/HistogramBenchmarks.cs
@@ -90,7 +90,6 @@ public void Setup()
                 .AddMeter(this.meter.Name)
                 .AddInMemoryExporter(exportedItems, metricReaderOptions =>
                 {
-                    metricReaderOptions.MetricReaderType = MetricReaderType.Periodic;
                     metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 1000;
                 })
                 .AddView(this.histogram.Name, new ExplicitBucketHistogramConfiguration() { Boundaries = this.bounds })

diff --git a/test/Benchmarks/Metrics/MetricsBenchmarks.cs b/test/Benchmarks/Metrics/MetricsBenchmarks.cs
@@ -74,7 +74,6 @@ public void Setup()
                 .AddMeter(this.meter.Name) // All instruments from this meter are enabled.
                 .AddInMemoryExporter(exportedItems, metricReaderOptions =>
                 {
-                    metricReaderOptions.MetricReaderType = MetricReaderType.Periodic;
                     metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 1000;
                     metricReaderOptions.Temporality = this.AggregationTemporality;
                 })