[Otlp] Make sure Otlp trace and metric exporters have dedicated options instances (#4200)

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

* CHANGELOG patch.
This commit is contained in:
Mikel Blanchard 2023-02-16 14:04:45 -08:00 committed by GitHub
parent d0829fff2f
commit 7139c7a656
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 39 additions and 16 deletions

View File

@ -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

View File

@ -73,16 +73,25 @@ namespace OpenTelemetry.Metrics
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(

View File

@ -76,16 +76,25 @@ namespace OpenTelemetry.Trace
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);
}
// Note: Not using finalOptionsName here for SdkLimitOptions.

View File

@ -641,6 +641,8 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
[Fact]
public void NonnamedOptionsMutateSharedInstanceTest()
{
var testOptionsInstance = new OtlpExporterOptions();
OtlpExporterOptions tracerOptions = null;
OtlpExporterOptions meterOptions = null;
@ -649,11 +651,15 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
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");
}));
@ -676,12 +682,7 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
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]