Skip to content

Commit

Permalink
[Otlp] Make sure Otlp trace and metric exporters have dedicated optio…
Browse files Browse the repository at this point in the history
…ns instances (#4200)

* Make sure Otlp trace and metric exporters have dedicated options instances.

* CHANGELOG patch.
  • Loading branch information
CodeBlanch committed Feb 17, 2023
1 parent 87ce1fe commit 0edcb07
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 16 deletions.
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* `AddOtlpExporter` extension methods will now always create a new options
instance when named options are NOT used.
([#4200](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4200))

## 1.4.0-rc.4

Released 2023-Feb-10
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,25 @@ public static MeterProviderBuilder AddOtlpExporter(

return builder.AddReader(sp =>
{
var exporterOptions = sp.GetRequiredService<IOptionsMonitor<OtlpExporterOptions>>().Get(finalOptionsName);
OtlpExporterOptions exporterOptions;

if (name == null && configureExporter != null)
if (name == null)
{
// If we are NOT using named options, we execute the
// configuration delegate inline. The reason for this is
// If we are NOT using named options we create a new
// instance always. The reason for this is
// OtlpExporterOptions is shared by all signals. Without a
// name, delegates for all signals will mix together. See:
// https://github.com/open-telemetry/opentelemetry-dotnet/issues/4043
configureExporter(exporterOptions);
exporterOptions = sp.GetRequiredService<IOptionsFactory<OtlpExporterOptions>>().Create(finalOptionsName);

// Configuration delegate is executed inline on the fresh instance.
configureExporter?.Invoke(exporterOptions);
}
else
{
// When using named options we can properly utilize Options
// API to create or reuse an instance.
exporterOptions = sp.GetRequiredService<IOptionsMonitor<OtlpExporterOptions>>().Get(finalOptionsName);
}

return BuildOtlpExporterMetricReader(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,25 @@ internal static TracerProviderBuilder AddOtlpExporter(

return builder.AddProcessor(sp =>
{
var exporterOptions = sp.GetRequiredService<IOptionsMonitor<OtlpExporterOptions>>().Get(finalOptionsName);
OtlpExporterOptions exporterOptions;

if (name == null && configure != null)
if (name == null)
{
// If we are NOT using named options, we execute the
// configuration delegate inline. The reason for this is
// If we are NOT using named options we create a new
// instance always. The reason for this is
// OtlpExporterOptions is shared by all signals. Without a
// name, delegates for all signals will mix together. See:
// https://github.com/open-telemetry/opentelemetry-dotnet/issues/4043
configure(exporterOptions);
exporterOptions = sp.GetRequiredService<IOptionsFactory<OtlpExporterOptions>>().Create(finalOptionsName);

// Configuration delegate is executed inline on the fresh instance.
configure?.Invoke(exporterOptions);
}
else
{
// When using named options we can properly utilize Options
// API to create or reuse an instance.
exporterOptions = sp.GetRequiredService<IOptionsMonitor<OtlpExporterOptions>>().Get(finalOptionsName);
}

inlineConfigurationAction?.Invoke(exporterOptions, sp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,8 @@ public void Null_BatchExportProcessorOptions_SupportedTest()
[Fact]
public void NonnamedOptionsMutateSharedInstanceTest()
{
var testOptionsInstance = new OtlpExporterOptions();

OtlpExporterOptions tracerOptions = null;
OtlpExporterOptions meterOptions = null;

Expand All @@ -649,11 +651,15 @@ public void NonnamedOptionsMutateSharedInstanceTest()
services.AddOpenTelemetry()
.WithTracing(builder => builder.AddOtlpExporter(o =>
{
Assert.Equal(testOptionsInstance.Endpoint, o.Endpoint);

tracerOptions = o;
o.Endpoint = new("http://localhost/traces");
}))
.WithMetrics(builder => builder.AddOtlpExporter(o =>
{
Assert.Equal(testOptionsInstance.Endpoint, o.Endpoint);

meterOptions = o;
o.Endpoint = new("http://localhost/metrics");
}));
Expand All @@ -676,12 +682,7 @@ public void NonnamedOptionsMutateSharedInstanceTest()
Assert.NotNull(meterOptions);
Assert.Equal("http://localhost/metrics", meterOptions.Endpoint.OriginalString);

// Note: tracerOptions & meterOptions are actually the same instance
// in memory and that instance was actually mutated after
// OtlpTraceExporter was created but this is OK because it doesn't
// use the options after ctor.
Assert.True(ReferenceEquals(tracerOptions, meterOptions));
Assert.Equal("http://localhost/metrics", tracerOptions.Endpoint.OriginalString);
Assert.False(ReferenceEquals(tracerOptions, meterOptions));
}

[Fact]
Expand Down

0 comments on commit 0edcb07

Please sign in to comment.