Skip to content

Commit

Permalink
Default to periodic metric reader for push based exporters (#2982)
Browse files Browse the repository at this point in the history
  • Loading branch information
alanwest authored Mar 11, 2022
1 parent 00af781 commit 8ca6574
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,11 @@ private static MeterProviderBuilder AddConsoleExporter(

var metricExporter = new ConsoleMetricExporter(exporterOptions);

if (metricReaderOptions.MetricReaderType == (MetricReaderType)(-1))
{
metricReaderOptions.MetricReaderType = MetricReaderType.Manual;
}

var metricReader = metricReaderOptions.MetricReaderType == MetricReaderType.Manual
? new BaseExportingMetricReader(metricExporter)
: new PeriodicExportingMetricReader(metricExporter, metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds);
: new PeriodicExportingMetricReader(
metricExporter,
metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds ?? -1);

metricReader.Temporality = metricReaderOptions.Temporality;
return builder.AddReader(metricReader);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,11 @@ private static MeterProviderBuilder AddInMemoryExporter(

var metricExporter = new InMemoryExporter<Metric>(exportedItems);

if (metricReaderOptions.MetricReaderType == (MetricReaderType)(-1))
{
metricReaderOptions.MetricReaderType = MetricReaderType.Manual;
}

var metricReader = metricReaderOptions.MetricReaderType == MetricReaderType.Manual
? new BaseExportingMetricReader(metricExporter)
: new PeriodicExportingMetricReader(metricExporter, metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds);
: new PeriodicExportingMetricReader(
metricExporter,
metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds ?? -1);

metricReader.Temporality = metricReaderOptions.Temporality;
return builder.AddReader(metricReader);
Expand Down
7 changes: 7 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## Unreleased

* Fixed OTLP metric exporter to default to a periodic 60 second export cycle.
A bug was introduced in #2717 that caused the OTLP metric export to default
to a manual export cycle (i.e., requiring an explicit flush). A workaround
for this bug has been provided
[here](https://github.com/open-telemetry/opentelemetry-dotnet/issues/2979#issuecomment-1061060541).
([#2982](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2982))

## 1.2.0-rc3

Released 2022-Mar-04
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace OpenTelemetry.Metrics
/// </summary>
public static class OtlpMetricExporterExtensions
{
private const int DefaultExportIntervalMilliseconds = 60000;

/// <summary>
/// Adds <see cref="OtlpMetricExporter"/> to the <see cref="MeterProviderBuilder"/> using default options.
/// </summary>
Expand Down Expand Up @@ -102,14 +104,11 @@ private static MeterProviderBuilder AddOtlpExporter(

var metricExporter = new OtlpMetricExporter(exporterOptions);

if (metricReaderOptions.MetricReaderType == (MetricReaderType)(-1))
{
metricReaderOptions.MetricReaderType = MetricReaderType.Periodic;
}

var metricReader = metricReaderOptions.MetricReaderType == MetricReaderType.Manual
? new BaseExportingMetricReader(metricExporter)
: new PeriodicExportingMetricReader(metricExporter, metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds);
: new PeriodicExportingMetricReader(
metricExporter,
metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds ?? DefaultExportIntervalMilliseconds);

metricReader.Temporality = metricReaderOptions.Temporality;
return builder.AddReader(metricReader);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ OpenTelemetry.Metrics.PeriodicExportingMetricReader
OpenTelemetry.Metrics.PeriodicExportingMetricReader.PeriodicExportingMetricReader(OpenTelemetry.BaseExporter<OpenTelemetry.Metrics.Metric> exporter, int exportIntervalMilliseconds = 60000, int exportTimeoutMilliseconds = 30000) -> void
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.PeriodicExportingMetricReaderOptions() -> void
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.get -> int
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.get -> int?
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.set -> void
OpenTelemetry.ReadOnlyTagCollection
OpenTelemetry.ReadOnlyTagCollection.Count.get -> int
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ OpenTelemetry.Metrics.PeriodicExportingMetricReader
OpenTelemetry.Metrics.PeriodicExportingMetricReader.PeriodicExportingMetricReader(OpenTelemetry.BaseExporter<OpenTelemetry.Metrics.Metric> exporter, int exportIntervalMilliseconds = 60000, int exportTimeoutMilliseconds = 30000) -> void
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.PeriodicExportingMetricReaderOptions() -> void
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.get -> int
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.get -> int?
OpenTelemetry.Metrics.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds.set -> void
OpenTelemetry.ReadOnlyTagCollection
OpenTelemetry.ReadOnlyTagCollection.Count.get -> int
Expand Down
5 changes: 5 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

* The `PeriodicExportingMetricReader` now accepts an
`ExportIntervalMilliseconds` of `-1` indicating an infinite export interval
period.
([#2982](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2982))

## 1.2.0-rc3

Released 2022-Mar-04
Expand Down
23 changes: 2 additions & 21 deletions src/OpenTelemetry/Metrics/MetricReaderOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,35 +21,16 @@ namespace OpenTelemetry.Metrics;
/// </summary>
public class MetricReaderOptions
{
private const MetricReaderType MetricReaderTypeUnspecified = (MetricReaderType)(-1);
private MetricReaderType metricReaderType = MetricReaderTypeUnspecified;

/// <summary>
/// Gets or sets the AggregationTemporality used for Histogram
/// and Sum metrics.
/// </summary>
public AggregationTemporality Temporality { get; set; } = AggregationTemporality.Cumulative;

/// <summary>
/// Gets or sets the <see cref="MetricReaderType" /> to use. Defaults to <c>MetricReaderType.Manual</c>.
/// Gets or sets the <see cref="MetricReaderType" /> to use. Defaults to <c>MetricReaderType.Periodic</c>.
/// </summary>
public MetricReaderType MetricReaderType
{
get
{
if (this.metricReaderType == MetricReaderTypeUnspecified)
{
this.metricReaderType = MetricReaderType.Manual;
}

return this.metricReaderType;
}

set
{
this.metricReaderType = value;
}
}
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>.
Expand Down
3 changes: 2 additions & 1 deletion src/OpenTelemetry/Metrics/PeriodicExportingMetricReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public PeriodicExportingMetricReader(
int exportTimeoutMilliseconds = DefaultExportTimeoutMilliseconds)
: base(exporter)
{
Guard.ThrowIfOutOfRange(exportIntervalMilliseconds, min: 1);
Guard.ThrowIfInvalidTimeout(exportIntervalMilliseconds);
Guard.ThrowIfZero(exportIntervalMilliseconds);
Guard.ThrowIfOutOfRange(exportTimeoutMilliseconds, min: 0);

if ((this.SupportedExportModes & ExportModes.Push) != ExportModes.Push)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ namespace OpenTelemetry.Metrics
public class PeriodicExportingMetricReaderOptions
{
/// <summary>
/// Gets or sets the metric export interval in milliseconds. The default value is <c>60000</c>.
/// Gets or sets the metric export interval in milliseconds.
/// </summary>
public int ExportIntervalMilliseconds { get; set; } = 60000;
public int? ExportIntervalMilliseconds { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,40 @@

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
{
public class OtlpMetricsExporterTests
public class OtlpMetricsExporterTests : Http2UnencryptedSupportTests
{
[Fact]
public void TestAddOtlpExporter_SetsCorrectMetricReaderDefaults()
{
if (Environment.Version.Major == 3)
{
// Adding the OtlpExporter creates a GrpcChannel.
// This switch must be set before creating a GrpcChannel when calling an insecure HTTP/2 endpoint.
// See: https://docs.microsoft.com/aspnet/core/grpc/troubleshoot#call-insecure-grpc-services-with-net-core-client
AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true);
}

var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddOtlpExporter()
.Build();

var bindingFlags = System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance;

var metricReader = typeof(MetricReader)
.Assembly
.GetType("OpenTelemetry.Metrics.MeterProviderSdk")
.GetField("reader", bindingFlags)
.GetValue(meterProvider) as PeriodicExportingMetricReader;

Assert.NotNull(metricReader);

var exportIntervalMilliseconds = (int)typeof(PeriodicExportingMetricReader)
.GetField("exportIntervalMilliseconds", bindingFlags)
.GetValue(metricReader);

Assert.Equal(60000, exportIntervalMilliseconds);
}

[Fact]
public void UserHttpFactoryCalled()
{
Expand Down

0 comments on commit 8ca6574

Please sign in to comment.