opentelemetry-dotnet
opentelemetry-dotnet copied to clipboard
[Tracing] Improve dependency injection support in tracing build-up using SDK
Fixes #1215 - Multiple registrations no longer allowed
Relates to #3086 - Adds a mechanism to do this (see scenarios)
Relates to #1642 - Moves all of the dependency bits into SDK, only thing in "hosting" is the IHostedService. May or may not help with the name 😄
Relates to #2980 - Makes it possible to solve this
Relates to #2971 - Makes it possible to solve this
Changes
-
Moves dependency injection features based on
ServiceCollectionfrom hosting library directly into SDK so they are universally available. This is similar to what was done on #3504 for logs. -
Enabled a few more detached scenarios.
-
We decided on the SIG meeting 8/2/2022 that multiple providers registered into the same service collection using the
AddOpenTelemetryTracingextension would NOT be supported. This PR enforces that.
Public API
namespace OpenTelemetry.Trace
{
public abstract class TracerProviderBuilderBase
{
// This ctor is used by the hosting builder to pass in the external services the builder is bound to
+ protected TracerProviderBuilderBase(IServiceCollection services) {}
// This property is used by the hosting builder to set the external service provider once it is available
+ protected IServiceProvider? ServiceProvider { get; set; }
}
public static class TracerProviderBuilderExtensions
{
// All of these existed previously in Hosting, now it is part of the SDK and be used anywhere SDK is referenced
+ public static TracerProviderBuilder AddInstrumentation<T>(this TracerProviderBuilder tracerProviderBuilder) {}
+ public static TracerProviderBuilder AddProcessor<T>(this TracerProviderBuilder tracerProviderBuilder) where T : BaseProcessor<Activity> {}
+ public static TracerProviderBuilder ConfigureBuilder(this TracerProviderBuilder tracerProviderBuilder, Action<IServiceProvider, TracerProviderBuilder> configure) {}
+ public static TracerProviderBuilder ConfigureServices(this TracerProviderBuilder tracerProviderBuilder, Action<IServiceCollection> configure) {}
+ public static TracerProviderBuilder SetSampler<T>(this TracerProviderBuilder tracerProviderBuilder) where T : Sampler {}
// These are provided so no one breaks upgrading:
+ [Obsolete] public static TracerProviderBuilder Configure(this TracerProviderBuilder tracerProviderBuilder, Action<IServiceProvider, TracerProviderBuilder> configure) {}
+ [Obsolete] public static IServiceCollection? GetServices(this TracerProviderBuilder tracerProviderBuilder) {}
// These are new
+ public static TracerProviderBuilder AddExporter(this TracerProviderBuilder tracerProviderBuilder, ExportProcessorType exportProcessorType, BaseExporter<Activity> exporter) {}
+ public static TracerProviderBuilder AddExporter(this TracerProviderBuilder tracerProviderBuilder, ExportProcessorType exportProcessorType, BaseExporter<Activity> exporter, Action<ExportActivityProcessorOptions> configure) {}
+ public static TracerProviderBuilder AddExporter<T>(this TracerProviderBuilder tracerProviderBuilder, ExportProcessorType exportProcessorType) where T : BaseExporter<Activity> {}
+ public static TracerProviderBuilder AddExporter<T>(this TracerProviderBuilder tracerProviderBuilder, ExportProcessorType exportProcessorType, Action<ExportActivityProcessorOptions> configure) where T : BaseExporter<Activity> {}
}
// This class is similar to MetricReaderOptions and is used by AddExporter extensions
+ public class ExportActivityProcessorOptions
+ {
+ public ExportProcessorType ExportProcessorType { get; set; }
+ public BatchExportActivityProcessorOptions BatchExportProcessorOptions { get; set; }
+ }
}
Existing scenarios made available in SDK
All of this stuff you could already do using the hosting library. But now you can do it just with the SDK. That makes it more universal. Notice below the root is Sdk.CreateTracerProviderBuilder previously that builder had zero support for any kind dependency injection. Now it has the same surface as the builder returned by the AddOpenTelemetryTracing API. All of this will work for .NET Framework as well as .NET/Core.
using var tracerProvider = Sdk.CreateTracerProviderBuilder()
// Scenario 1: Register services...
.ConfigureServices(services =>
{
services.AddSingleton<MyCustomExporter>();
})
// Scenario 2: Add a processor retrieved through DI...
.AddProcessor<MyCustomProcessor>()
// Scenario 3: Add a sampler retrieved through DI...
.SetSampler<MyCustomSampler>()
// Scenario 4: Configure the builder once the service provider is available...
.ConfigureBuilder((sp, builder) =>
{
builder.AddProcessor(new SimpleActivityExportProcessor(sp.GetRequiredService<MyCustomExporter>()));
})
The hidden value being created here is really for library/extension authors. Now I can build a library with this extension...
public static TracerProviderBuilder AddMyLibraryFeature(this TracerProviderBuilder builder)
{
return builder
.ConfigureServices(services =>
{
services.AddSingleton<MyDependencyA>();
services.AddSingleton<MyDependencyB>();
})
.AddProcessor<MyCustomProcessor>();
}
...and it will work equally well for .NET Framework users as it will .NET/Core users, with only a reference to SDK.
It also allows us to clean up some of the strange registration code we have. For example this...
public static TracerProviderBuilder AddJaegerExporter(this TracerProviderBuilder builder, Action<JaegerExporterOptions> configure = null)
{
Guard.ThrowIfNull(builder);
if (builder is IDeferredTracerProviderBuilder deferredTracerProviderBuilder)
{
return deferredTracerProviderBuilder.Configure((sp, builder) =>
{
AddJaegerExporter(builder, sp.GetOptions<JaegerExporterOptions>(), configure, sp);
});
}
return AddJaegerExporter(builder, new JaegerExporterOptions(), configure, serviceProvider: null);
}
...becomes this...
public static TracerProviderBuilder AddJaegerExporter(this TracerProviderBuilder builder, Action<JaegerExporterOptions> configure = null)
{
Guard.ThrowIfNull(builder);
if (configure != null)
{
builder.ConfigureServices(services => services.Configure(configure));
}
return builder.ConfigureBuilder((sp, builder) =>
{
var options = sp.GetRequiredService<IOptions<JaegerExporterOptions>>().Value;
AddJaegerExporter(builder, options, sp);
});
}
New scenarios enabled
Register an exporter with the builder
This can be done either by providing the exporter instance or using dependency injection.
builder.AddExporter<MyExporter>(ExportProcessorType.Batch);
The BatchExportActivityProcessorOptions used (when batching is configured) is retrieved through the options API. Users will be able to bind BatchExportActivityProcessorOptions to IConfiguration and manage it through the configuration pipeline (JSON, envvars, command-line, whatever they happen to have configured). There is a bit more work to smooth out how environment variables are retrieved (which is what #2980 is about) but I'll tackle that separately.
Register a processor through services which is automatically configured
builder.Services.AddOpenTelemetryTracing();
builder.Services.AddSingleton<BaseProcessor<Activity>, MyCustomProcessor>();
Register a sampler through services which is automatically configured
builder.Services.AddOpenTelemetryTracing();
builder.Services.AddSingleton<Sampler, MyCustomSampler>();
Note: A NotSupportedException is thrown during build if a sampler is set directly using a SetSampler call and also through services.
Swiss-army-knife detached configuration method
Some users like @MartinJT have asked for a way to configure things completely detached from the builder. You can now do this:
services.AddSingleton<Action<IServiceProvider, TracerProviderBuilder>>(MyBuilderConfigurationMethod);
The builder will now look for any actions registered and call them during build-up.
That feature can be used to make all kinds of registration extensions off of IServiceCollection. For example @MartinJT has asked for a way to globally register an exporter. Using that extension point, users could do this:
public static IServiceCollection AddOpenTelemetrySimpleExporter<T>(this IServiceCollection services)
where T : BaseExporter<Activity>
{
return services
.AddSingleton<T>()
.AddSingleton<Action<IServiceProvider, TracerProviderBuilder>>((sp, builder) =>
{
builder.AddProcessor(new SimpleActivityExportProcessor(sp.GetRequiredService<T>()));
});
}
Maybe we should have such extensions in the SDK itself. Not going to tackle that here!
#3086 is asking for an AddOpenTelemetryTracing extension which exposes the final IServiceProvider. I'm hesitant to do that because I think it would be confusing to expose these two methods:
public static IServiceCollection AddOpenTelemetryTracing(this IServiceCollection services, Action<TracerProviderBuilder> configure) {}
public static IServiceCollection AddOpenTelemetryTracing(this IServiceCollection services, Action<IServiceProvider, TracerProviderBuilder> configure) {}
There are huge non-trivial behavioral differences between those two methods which the signatures beguile.
However using this extension point, it would be simple for users to make such an extension exist if they so-desire:
public static IServiceCollection AddOpenTelemetryTracing(this IServiceCollection services, Action<IServiceProvider, TracerProviderBuilder> configure)
{
return services
.AddOpenTelemetryTracing()
.AddSingleton<Action<IServiceProvider, TracerProviderBuilder>>(configure);
}
TODOs
- [X] Appropriate
CHANGELOG.mdupdated for non-trivial changes - [ ] Changes in public API reviewed
- [ ] Unit tests
Codecov Report
Merging #3533 (c91f02d) into main (8700d5d) will increase coverage by
0.31%. The diff coverage is92.95%.
Additional details and impacted files
@@ Coverage Diff @@
## main #3533 +/- ##
==========================================
+ Coverage 87.02% 87.34% +0.31%
==========================================
Files 277 280 +3
Lines 9953 10056 +103
==========================================
+ Hits 8662 8783 +121
+ Misses 1291 1273 -18
| Impacted Files | Coverage Δ | |
|---|---|---|
| ...s.Hosting/Trace/TracerProviderBuilderExtensions.cs | 0.00% <0.00%> (-73.92%) |
:arrow_down: |
| ...rc/OpenTelemetry/Trace/TracerProviderExtensions.cs | 68.00% <ø> (ø) |
|
| ...s.Hosting/Implementation/TelemetryHostedService.cs | 73.33% <75.00%> (+4.10%) |
:arrow_up: |
| ...racerProviderBuilderServiceCollectionExtensions.cs | 85.71% <85.71%> (ø) |
|
| ...lemetry/Trace/Builder/TracerProviderBuilderBase.cs | 91.81% <91.81%> (ø) |
|
| ...emetry/Trace/Builder/TracerProviderBuilderState.cs | 94.73% <94.73%> (ø) |
|
| ...y/Trace/Builder/TracerProviderBuilderExtensions.cs | 97.56% <97.56%> (ø) |
|
| ...ensions.Hosting/OpenTelemetryServicesExtensions.cs | 77.77% <100.00%> (+6.34%) |
:arrow_up: |
| src/OpenTelemetry/BaseExportProcessor.cs | 86.95% <100.00%> (+0.59%) |
:arrow_up: |
| src/OpenTelemetry/BatchExportProcessor.cs | 84.11% <100.00%> (+1.86%) |
:arrow_up: |
| ... and 23 more |
@cijothomas
We need to document/add some examples demonstrating the new capabilities ( i.e everything from PR desc. into docs/examples) later, so people can easily leverage these.
Totally! Once I am done getting traces/metrics/logs all squared away I'll take a stab at spinning up some documentation.