[Tracing] Improve dependency injection support in tracing build-up using SDK

[CodeBlanch]

Fixes #1215 - Multiple registrations now all configure the same builder/provider
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 ServiceCollection from hosting library directly into SDK so they are universally available. This is similar to what was done on [Logs] Support dependency injection in logging build-up #3504 for logs.

• Enabled a few more detached scenarios.

• We decided on the SIG meeting 8/2/2022 that only a single provider should be supported in a given ServicesCollection this PR makes that a reality.

SDK public API

namespace Microsoft.Extensions.DependencyInjection
{
+   public static class TracerProviderBuilderServiceCollectionExtensions
+   {
       // These are basically what existed previously in Hosting minus the IHostedService registration
+      public static IServiceCollection ConfigureOpenTelemetryTracing(this IServiceCollection services) {}
+      public static IServiceCollection ConfigureOpenTelemetryTracing(this IServiceCollection services, Action<TracerProviderBuilder> configure) {}
+   }
}

namespace OpenTelemetry.Trace
{
   public static class TracerProviderBuilderExtensions
   {
       // These existed previously in Hosting, now they are part of the SDK and can 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 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; }
+   }
}

OpenTelemetry.Extensions.Hosting public API

namespace OpenTelemetry.Trace
{
   public static class TracerProviderBuilderExtensions
   {
-      public static TracerProviderBuilder AddInstrumentation<T>(this TracerProviderBuilder tracerProviderBuilder) {}
-      public static TracerProviderBuilder AddProcessor<T>(this TracerProviderBuilder tracerProviderBuilder) where T : BaseProcessor<Activity> {}
-      public static TracerProviderBuilder SetSampler<T>(this TracerProviderBuilder tracerProviderBuilder) where T : Sampler {}
-      public static TracerProvider Build(this TracerProviderBuilder tracerProviderBuilder, IServiceProvider serviceProvider) {}

        // Obsoleted these so no one breaks upgrading
+      [Obsolete("Call ConfigureBuilder instead this method will be removed in a future version.")]
       public static TracerProviderBuilder Configure(this TracerProviderBuilder tracerProviderBuilder, Action<IServiceProvider, TracerProviderBuilder> configure) {}
+      [Obsolete("Call ConfigureServices instead this method will be removed in a future version.")]
       public static IServiceCollection? GetServices(this TracerProviderBuilder tracerProviderBuilder) {}
   }
}

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.

Detached configuration method

Some users like @martinjt have asked for a way to configure things completely detached from the builder.

The SDK ConfigureOpenTelemetryTracing extension is safe to be called multiple times. Only one TracerProvider will exist in the IServiceProvider and multiple calls will all contribute to its configuration, in order. This makes it possible to detach from the TracerProviderBuilder. This enables a scenario like this...

  // ### Application bootstrap services configuration...
  services.AddMyLibrary(); // <- Does detached tracing configuration
  // This is using the existing hosting extension which will start the TracerProvider automatically
  services.AddOpenTelemetryTracing();

  // ### Can be implemented anywhere...
  public static IServiceCollection AddMyLibrary(this IServiceCollection  services)
  {
     services.AddSingleton<MyLibraryService>();
     // This is using the new SDK extension, no dependency on hosting required
     services.ConfigureOpenTelemetryTracing(configure => configure.AddSource("MyLibrarySource"));
  }

Design details

TracerProviderBuilderBase has 3 ctors which drive its 2 phases.

• Phase one: Service configuration

  The first phase is used to register services before the IServiceProvider is ready. Everything done on the builder in this phase is queued up into the IServiceCollection. Previously the builder had some state and certain calls like AddProcessor would modify that state. Other things like Configure/Builder would be queued up for later. Moving everything to the queue is how we are now able to call AddOpenTelemetryTracing multiple times. Previously that would result in state needing to be managed for each builder, now everything just dumps into IServiceCollection. Also now everything will happen in the order of the builder invocation. Previously some stuff would happen instantly via the state, some things would happen later on. The order now should be predictable.

  The two ctors:

  TracerProviderBuilderBase() <- Used by Sdk.CreateTracerProviderBuilder() where the builder owns its services fully
  TracerProviderBuilderBase(IServiceCollection services) <- Used by AddOpenTelemetryTracing where the services are external

• Phase two: Build-up

  At some point the IServiceProvider is ready and services are closed. In the case of Sdk.CreateTracerProviderBuilder() this happens when Build() is called. In the case of AddOpenTelemetryTracing the host controls that and we don't build until something asks for a TracerProvider from the IServiceProvider. In either case phase two is executed here:

  opentelemetry-dotnet/src/OpenTelemetry/Trace/TracerProviderSdk.cs

  Lines 52 to 56 in 9e02797

  	var state = new TracerProviderBuilderState(serviceProvider);
  	TracerProviderBuilderServiceCollectionHelper.InvokeRegisteredConfigureStateCallbacks(
  	serviceProvider,
  	state);

  We create a state object (TracerProviderBuilderState) and then execute all the queued configuration actions against that state.

  We use this ctor:

  TracerProviderBuilderBase(TracerProviderBuilderState state)

  Which tells the builder to modify the state instead of queuing things up.

  The TracerProvider is created using the state after all the configuration is applied.

I'm sure this all feels very non-standard but actually it is very close to the built-in patterns. For example, AddLogging immediately invokes the configuration callback on a LoggingBuilder which only modifies the IServiceCollection. Nothing actually happens until the LoggerFactory is created at which point everything is retrieved from the IServiceProvider.

Our case is more complicated because we weren't disciplined about using interfaces registered for all the TracerProvider "services"/features, and we have a callback for configuring the builder once IServiceProvider is available, but the underlying mechanisms are essentially the same.

TODOs

• Appropriate CHANGELOG.md updated for non-trivial changes
• Changes in public API reviewed
• Unit tests

[codecov]

Codecov Report

Merging #3533 (c91f02d) into main (8700d5d) will increase coverage by 0.31%.
The diff coverage is 92.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%)	⬇️
...rc/OpenTelemetry/Trace/TracerProviderExtensions.cs	68.00% <ø> (ø)
...s.Hosting/Implementation/TelemetryHostedService.cs	73.33% <75.00%> (+4.10%)	⬆️
...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%)	⬆️
src/OpenTelemetry/BaseExportProcessor.cs	86.95% <100.00%> (+0.59%)	⬆️
src/OpenTelemetry/BatchExportProcessor.cs	84.11% <100.00%> (+1.86%)	⬆️
... and 23 more

[alanwest]

I'm beginning to question if we should be less conservative here. That is, just dump GetServices. Likewise dump Configure.

Yes, it'll break folks who are currently using OpenTelemetry.Extensions.Hosting, but my sense of these methods are that they're less frequently used.

[CodeBlanch]

@alanwest I pushed an update which removed them from SDK but I left them in Hosting and added [Obsolete] there. I don't feel passionately that we shouldn't remove them outright, but I feel like the Hosting API has existed for so long it would be nice for us to not break anyone. LMK what you think about that.

[alanwest]

👍 I think this is definitely the right direction. It'll be good to land this sooner than later to unblock bringing these patterns to metrics and logs. I think there'll be ample opportunity to continue to iron things out as needed.

[cijothomas]

LGTM.
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.

[CodeBlanch]

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