Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Traces] Add named options support to OtlpTraceExporter builder extensions #3653

Merged
merged 3 commits into from
Sep 13, 2022
Merged

[Traces] Add named options support to OtlpTraceExporter builder extensions #3653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e75e89d
Add named options support to otlp trace provider builder extensions.
CodeBlanch Sep 13, 2022
9afd42f
CHANGELOG update.
CodeBlanch Sep 13, 2022
91effcb
Merge branch 'main' into otlp-traces-namedoptions
CodeBlanch Sep 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/net462/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions, OpenTelemetry.Metrics.MetricReaderOptions> configureExporterAndMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configureExporter) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configureExporter) -> OpenTelemetry.Metrics.MeterProviderBuilder
*REMOVED*static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure = null) -> OpenTelemetry.Trace.TracerProviderBuilder
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If some user was binding to this reflectively and specifically looking at metadata for the optional parameter they would no longer find it and break but I don't think that is really worth worrying about. The benefit to this change is to keep consistency with everything else and allow for future safe expansion/overload additions without default parameter ambiguity issues.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they explicitly had builder.AddOtlpExporter(null) could nullablility cause any problems? I think the answer is no, but just asking. Basically should the new overload be

static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions>? configure) -> OpenTelemetry.Trace.TracerProviderBuilder

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we had nullable enabled in OpenTelemetry.Exporter.OpenTelemetryProtocol project then yes we would want the ?. But we don't, so the existing signature accepts null as a traditional ref would. Basically null is expected and works fine. RE: Nullable, I'm almost done annotating SDK after which I was thinking I would do API and then start branching out into other projects. It is a very slow burn 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yea right, forgot it wasn't enabled yet.

static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
6 changes: 5 additions & 1 deletion src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/net6.0/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions, OpenTelemetry.Metrics.MetricReaderOptions> configureExporterAndMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configureExporter) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configureExporter) -> OpenTelemetry.Metrics.MeterProviderBuilder
*REMOVED*static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure = null) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
6 changes: 5 additions & 1 deletion ...elemetry.Exporter.OpenTelemetryProtocol/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions, OpenTelemetry.Metrics.MetricReaderOptions> configureExporterAndMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configureExporter) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configureExporter) -> OpenTelemetry.Metrics.MeterProviderBuilder
*REMOVED*static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure = null) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
6 changes: 5 additions & 1 deletion ...elemetry.Exporter.OpenTelemetryProtocol/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions, OpenTelemetry.Metrics.MetricReaderOptions> configureExporterAndMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configureExporter) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.OtlpMetricExporterExtensions.AddOtlpExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configureExporter) -> OpenTelemetry.Metrics.MeterProviderBuilder
*REMOVED*static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure = null) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.OtlpTraceExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
`AddOtlpExporter` extension to allow for more fine-grained options management
([#3648](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3648))

* Added overloads which accept a name to the `TracerProviderBuilder`
`AddOtlpExporter` extension to allow for more fine-grained options management
([#3653](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3653))

## 1.4.0-alpha.2

Released 2022-Aug-18
Expand Down
31 changes: 27 additions & 4 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpTraceExporterHelperExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,43 @@ public static class OtlpTraceExporterHelperExtensions
/// Adds OpenTelemetry Protocol (OTLP) exporter to the TracerProvider.
/// </summary>
/// <param name="builder"><see cref="TracerProviderBuilder"/> builder to use.</param>
/// <param name="configure">Exporter configuration options.</param>
/// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
public static TracerProviderBuilder AddOtlpExporter(this TracerProviderBuilder builder, Action<OtlpExporterOptions> configure = null)
public static TracerProviderBuilder AddOtlpExporter(this TracerProviderBuilder builder)
=> AddOtlpExporter(builder, name: null, configure: null);

/// <summary>
/// Adds OpenTelemetry Protocol (OTLP) exporter to the TracerProvider.
/// </summary>
/// <param name="builder"><see cref="TracerProviderBuilder"/> builder to use.</param>
/// <param name="configure">Callback action for configuring <see cref="OtlpExporterOptions"/>.</param>
/// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
public static TracerProviderBuilder AddOtlpExporter(this TracerProviderBuilder builder, Action<OtlpExporterOptions> configure)
=> AddOtlpExporter(builder, name: null, configure);

/// <summary>
/// Adds OpenTelemetry Protocol (OTLP) exporter to the TracerProvider.
/// </summary>
/// <param name="builder"><see cref="TracerProviderBuilder"/> builder to use.</param>
/// <param name="name">Name which is used when retrieving options.</param>
/// <param name="configure">Callback action for configuring <see cref="OtlpExporterOptions"/>.</param>
/// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
public static TracerProviderBuilder AddOtlpExporter(
this TracerProviderBuilder builder,
string name,
Action<OtlpExporterOptions> configure)
{
Guard.ThrowIfNull(builder);

name ??= Options.DefaultName;

if (configure != null)
{
builder.ConfigureServices(services => services.Configure(configure));
builder.ConfigureServices(services => services.Configure(name, configure));
}

return builder.ConfigureBuilder((sp, builder) =>
{
var options = sp.GetRequiredService<IOptions<OtlpExporterOptions>>().Value;
var options = sp.GetRequiredService<IOptionsSnapshot<OtlpExporterOptions>>().Get(name);

AddOtlpExporter(builder, options, sp);
});
Expand Down
21 changes: 21 additions & 0 deletions test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,27 @@ static OtlpTraceExporterTests()
ActivitySource.AddActivityListener(listener);
}

[Fact]
public void AddOtlpTraceExporterNamedOptionsSupported()
{
int defaultExporterOptionsConfigureOptionsInvocations = 0;
int namedExporterOptionsConfigureOptionsInvocations = 0;

using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.ConfigureServices(services =>
{
services.Configure<OtlpExporterOptions>(o => defaultExporterOptionsConfigureOptionsInvocations++);

services.Configure<OtlpExporterOptions>("Exporter2", o => namedExporterOptionsConfigureOptionsInvocations++);
})
.AddOtlpExporter()
.AddOtlpExporter("Exporter2", o => { })
.Build();

Assert.Equal(1, defaultExporterOptionsConfigureOptionsInvocations);
Assert.Equal(1, namedExporterOptionsConfigureOptionsInvocations);
}

[Fact]
public void OtlpExporter_BadArgs()
{
Expand Down