[di] Expose a detached TracerProviderBuilder extension on IServiceCollection which may modify services.

[CodeBlanch]

Fixes #4503

Changes

• Adds an overload to the IServiceCollection.ConfigureOpenTelemetryTracerProvider extension which doesn't require IServiceProvider and allows for IServiceCollection modification.

Public API Changes

namespace OpenTelemetry.Trace
{
   public static class OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions
   {
+     public static IServiceCollection ConfigureOpenTelemetryTracerProvider(this IServiceCollection services, Action<TracerProviderBuilder> configure) {}
   }
}

Details

We have this pattern for configuring providers using the IServiceCollection: https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/trace/extending-the-sdk#iservicecollection-extension-methods

We refer to this as "detached" configuration.

Today the API passes the IServiceProvider to the configuration delegate.

This type of stuff will work fine:

services.ConfigureOpenTelemetryTracerProvider((sp, builder) =>
{
   builder.AddSource("MyLibrary");
   builder.AddProcessor(sp.GetRequiredService<MyProcessor>());
});

This will throw a NotSupportedException:

services.ConfigureOpenTelemetryTracerProvider((sp, builder) =>
{
   builder.ConfigureServices(services => {}); // Throws
   builder.AddProcessor<MyProcessor>(); // Throws
});

The reason for that is once the IServiceProvider is available, we can no longer add services.

#4503 shows some users want to do detached things but don't really care about the IServiceProvider.

services.ConfigureOpenTelemetryTracerProvider((sp, builder) => 
   builder.AddOtlpExporter()); // Throws because AddOtlpExporter registers services

What this PR does is add a detached extension which may modify services.

services.ConfigureOpenTelemetryTracerProvider(builder => 
   builder.AddOtlpExporter()); // Works using this new API

Merge requirement checklist

• CONTRIBUTING guidelines followed (nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

Merging #4508 (499fde6) into main (eb12f9b) will increase coverage by 0.06%.
The diff coverage is 96.00%.

❗ Current head 499fde6 differs from pull request most recent head 26d2131. Consider uploading reports for the commit 26d2131 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4508      +/-   ##
==========================================
+ Coverage   85.12%   85.19%   +0.06%     
==========================================
  Files         317      317              
  Lines       12602    12594       -8     
==========================================
+ Hits        10728    10729       +1     
+ Misses       1874     1865       -9     

Impacted Files	Coverage Δ
...lemetry/Trace/Builder/TracerProviderBuilderBase.cs	72.00% <90.00%> (-6.67%)	⬇️
...ns/Trace/TracerProviderServiceCollectionBuilder.cs	97.22% <97.22%> (ø)
...encyInjectionTracingServiceCollectionExtensions.cs	100.00% <100.00%> (ø)

... and 6 files with indirect coverage changes

[CodeBlanch]

A bit more detail for anyone curious...

This throws today:

builder.Services.AddOpenTelemetry()
    .WithTracing(builder => builder.AddConsoleExporter());

builder.Services.ConfigureOpenTelemetryTracerProvider((sp, builder) => builder.AddOtlpExporter());

This can be used as a workaround:

builder.Services.AddOpenTelemetry()
    .WithTracing(builder => builder.AddConsoleExporter());

builder.Services.AddOpenTelemetry()
    .WithTracing(builder => builder.AddOtlpExporter());

However there are two issues with the workaround:

• It requires a dependency on OpenTelemetry.Extensions.Hosting. For libraries trying to do configuration as part of other service registration, that may not be desirable. For .NET Framework things, where hosting probably isn't being used at all, it is probably a non-starter.

• ConfigureOpenTelemetryTracerProvider today will no-op if no TracerProvider is requested. Libraries can call it safely and nothing will happen unless the host actually starts up tracing. Calling AddOpenTelemetry().WithTracing() will cause things to start up.

[alanwest]

Good improvement! Was confusing before "used to configure ... used to create the ... for the ... being configured" 😆

[alanwest]

This seems ok to me. It is new public API, but at least it is simply an additional overload of an existing method and not an entirely new method (for example, won't increase the number of methods one might see in intellisense).

Also, it seems this new overload is better than the previous one. If I'm understanding things correctly, I'd imagine this new overload would serve many of the scenarios the existing method serves plus more. The existing method seems like an even more advanced scenario where you know for sure you need a handle on the IServiceProvider.

[alanwest]

Is the plan to also add this for meter and logger providers, too?

[CodeBlanch]

Is the plan to also add this for meter and logger providers, too?

Yes sir! I'll do a PR for each.

[vishweshbankwar]

Many helper extensions register services and may throw if invoked inside the configuration --> This part is bit unclear as end users may not know which extensions register services versus which ones do not.

Could we add some guidance here for end users? some message saying - if you don't need access to ServiceProvider then use the other overload.