Can't inject resource detectors after upgrading to 1.4.0-rc.3

[austindrenski]

Bug Report

This morning I started integrating last night's release of 1.4.0-rc.3 and quickly encountered some trouble caused by the removal of ConfigureBuilder(...) in #4103.

I've looked around for alternative APIs, but it seems that the alternatives are also marked internal:

opentelemetry-dotnet/src/OpenTelemetry/Resources/ResourceBuilder.cs

Lines 145 to 153 in 31cf729

	// Note: This API may be made public if there is a need for it.
	internal ResourceBuilder AddDetector(Func<IServiceProvider?, IResourceDetector> resourceDetectorFactory)
	{
	Guard.ThrowIfNull(resourceDetectorFactory);
	this.ResourceDetectors.Add(new ResolvingResourceDetector(resourceDetectorFactory));
	return this;
	}

Big fan of this project and all the hard work you folks are doing, so if this is user error, I'd appreciate a pointer in the right direction, but if not, would greatly appreciate seeing this functionality return soon so I can keep my projects on track with 1.4.0-rc.3.

What is the expected behavior?

Bumping from 1.4.0-rc.2 to 1.4.0-rc.3 does not break resource builder DI functionality.

What is the actual behavior?

Bumping from 1.4.0-rc.2 to 1.4.0-rc.3 breaks resource builder DI functionality.

Reproduce

With versions 1.4.0-rc.2, I can use the ConfigureBuilder(...) method to inject custom resource detectors from an IServiceProvider.

var builder = WebApplication.CreateBuilder(args);

builder.Services
    .AddOpenTelemetry()
    .StartWithHost()
    .WithMetrics(static builder => builder
        .ConfigureBuilder(static (services, builder) => builder
            .SetResourceBuilder(ResourceBuilder
                .CreateDefault()
                .AddDetector(services.GetRequiredService<SomeResourceDetector>())
                .AddTelemetrySdk()))
        .ConfigureServices(static services => services
            .AddSingleton<SomeResourceDetector>())
    .WithTracing(static builder => builder
        .ConfigureBuilder(static (services, builder) => builder
            .SetResourceBuilder(ResourceBuilder
                .CreateDefault()
                .AddDetector(services.GetRequiredService<SomeResourceDetector>())
                .AddTelemetrySdk()))
        .ConfigureServices(static services => services
            .AddSingleton<SomeResourceDetector>());

var app = builder.Build();

app.Run();

In my actual use case, there are many custom resource detectors, not just SomeResourceDetector, but here's a contrived concrete example to illustrate why its being injected:

// contrived example, don't read into it
sealed class SomeResourceDetector : IResourceDetector
{
    readonly Resource _resource;

    public SomeResourceDetector(IHostEnvironment environment)
        => _resource = ResourceBuilder
            .CreateEmpty()
            .AddAttributes(new Dictionary<string, object>
            {
                ["deployment.environment"] = Check.NotNull(environment).EnvironmentName
            })
            .Build();

    public Resource Detect() => _resource;
}

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="OpenTelemetry.Extensions.Hosting" Version="1.4.0-rc.3" />
  </ItemGroup>

</Project>

Additional Context

I'm not necessarily asking for ConfigureBuilder(...) itself to be brought back (it seems like there was some rationale behind removing it from the public API in #4103), but I would like to see the functionality to inject resource detectors (and other things that might be useful when programmatically configuring a resource builder) restored.

Potentially off-topic context that might still be relevant to the team

I'm a big advocate for OTel generally, and your team's work specifically, in my organization and unfortunately breaking functionality like this (even cognizant of the pre-release status) invites FUD about overall reliability from the non-believers yet-to-be-converted.

I believe in this project, so I'm happy to keep answering those questions, but wanted to bubble some non-technical "impact" of this breakage in light of recent discussions around community impression of project stability in issues like #4083 (comment) by @CodeBlanch, @martinjt, and others.

[CodeBlanch]

Hey @austindrenski thanks for this issue! I opened a PR to try and get the AddDetector factory overload back into the public API. Need to see what the other maintainers think, hopefully they will support it 😄

If you want to unblock yourself on the 1.4 rc.3 API (which removed ConfigureBuilder) you can always put back the functionality using extensions in your own code. Like this:

namespace OpenTelemetry.Trace
{
    public static class MyOpenTelemetryTracerProviderBuilderExtensions
    {
        public static TracerProviderBuilder ConfigureBuilder(
            this TracerProviderBuilder builder,
            Action<IServiceProvider, TracerProviderBuilder> configure)
        {
            // Note: When using OpenTelemetry SDK this will always be true starting with 1.4.
            if (builder is IDeferredTracerProviderBuilder deferredTracerProviderBuilder)
            {
                deferredTracerProviderBuilder.Configure(configure);
            }

            return builder;
        }
    }
}

namespace OpenTelemetry.Metrics
{
    public static class MyOpenTelemetryMeterProviderBuilderExtensions
    {
        public static MeterProviderBuilder ConfigureBuilder(
            this MeterProviderBuilder builder,
            Action<IServiceProvider, MeterProviderBuilder> configure)
        {
            // Note: When using OpenTelemetry SDK this will always be true starting with 1.4.
            if (builder is IDeferredMeterProviderBuilder deferredMeterProviderBuilder)
            {
                deferredMeterProviderBuilder.Configure(configure);
            }

            return builder;
        }
    }
}

[gao-artur]

Hey, I also was hit by this change. In my case I'm working around the missing support of IOptions pattern in OpenTelemetry.Instrumentation.ElasticsearchClient and OpenTelemetry.Instrumentation.EntityFrameworkCore

builder.ConfigureBuilder((provider, providerBuilder) =>
{
    var options = provider.GetRequiredService<IOptionsMonitor<EntityFrameworkInstrumentationOptions>>();
    providerBuilder.AddEntityFrameworkCoreInstrumentation(o =>
    {
        o.SetDbStatementForText = options.CurrentValue.SetDbStatementForText;
        o.SetDbStatementForStoredProcedure = options.CurrentValue.SetDbStatementForStoredProcedure;
    });
});

The provided workaround does the job, but I wanted to give you another example where this method was useful.

[ogxd]

I had the same issue while upgrading from 1.0.0 to 1.4.0. My usecase if very similar to @gao-artur 's one

serviceCollection.AddOpenTelemetryTracing(b =>
{
    b.Configure((serviceProvider, builder) =>
    {
        var tracingConfiguration = serviceProvider.GetService<IOptionsMonitor<TracingConfiguration>>();
        ...
        builder.AddAspNetCoreInstrumentation(...);
    }
}

However this workaround does not seems to work for me as I am getting this error:

System.NotSupportedException : Services cannot be configured after ServiceProvider has been created.

This used to work 😢

[martinjt]

You can do this instead now.

builder.Services.ConfigureOpenTelemetryTracerProvider((sp, tp) => {
    var tracingConfiguration = sp.GetService<IOptionsMonitor<TracingConfiguration>>();
    ...
    builder.AddAspNetCoreInstrumentation(...);
});

where builder.Services is the IServiceCollection

One thing to keep in mind is that it won't update based on IOptionsMonitor processes, but you can get the data from IOptions at startup.

[CodeBlanch]

@ogxd Can you share a little bit more about your use case? Are you trying to enable/disable aspnetcore instrumentation based on configuration?

We don't have a great story yet for enabling/disabling the SDK or individual components. But you could make your own extension sort of like this...

appBuilder.AddOpenTelemetry();

    public static class MyAppBuilderExtensions
    {
        public static WebApplicationBuilder AddOpenTelemetry(this WebApplicationBuilder appBuilder)
        {
            var configuration = new OpenTelemetryConfiguration();
            appBuilder.Configuration.GetSection("OpenTelemetry").Bind(configuration);

            if (configuration.EnableTracing)
            {
                appBuilder.Services.AddOpenTelemetry()
                    .WithTracing(builder =>
                    {
                        if (configuration.EnableAspNetCoreInstrumentation)
                        {
                            appBuilder.Services.Configure<AspNetCoreInstrumentationOptions>(
                                appBuilder.Configuration.GetSection("OpenTelemetry:AspNetCoreOptions"));

                            builder.AddAspNetCoreInstrumentation();
                        }
                    });
            }

            return appBuilder;
        }
    }

    public sealed class OpenTelemetryConfiguration
    {
        public bool EnableTracing { get; set; }
        public bool EnableAspNetCoreInstrumentation { get; set; }
    }

[ogxd]

@CodeBlanch Yes this is what I used to do. Thanks a lot for the code snippet I'll see if I can get around my issue with this. I'm sure it will be useful for others as well 😄