`OTEL_DOTNET_EXPERIMENTAL_{component}_DISABLE_URL_QUERY_REDACTION` is not honored via `Configuration` for `Owin` and `AspNet` instrumentations

[julealgon]

`OTEL_DOTNET_EXPERIMENTAL_{component}_DISABLE_URL_QUERY_REDACTION` is not honored via `Configuration` for `Owin` and `AspNet` instrumentations

[julealgon]

question:

Is https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Extensions.Hosting expected to be used with Asp.Net app at all? Asp.Net instrumentation, examples - none of them show that pattern.

I know this question wasn't for me, but I can suggest that this shouldn't concern the individual instrumentation packages themselves: note that the Http or Wcf or EntityFrameworkCore or any other instrumentations are agnostic of how the are instantiated, so they work with IConfiguration if present, and fallback to envvar if not.

I strongly believe this should be the case for all instrumentations, even if they normally aren't used with the Hosting package.

Again, keep in mind that people can use parts of Microsoft.Extensions.Hosting anywhere. Either they use just DI, or they use DI + Configuration, or they use DI + Configuration + Logging, etc. This is what we are doing to modernize areas that can be modernized of our projects. We don't use the running parts of hosting however, like the hosted services mechanism, application lifetime, etc.

[CodeBlanch]

So what the SDK does is make sure there is always an IConfiguration available. In the case of Microsoft.Extensions.Hosting that IConfiguration is the host one. In the case of the "detached" build-up APIs ("Sdk.Create*" and "OpenTelemetrySdk.Create" being added in 1.10.0) the IConfiguration is created automatically from envvars.

Issue here seems to be Owin and AspNet aren't using the IConfiguration. My guess would be due to some static coupling. I'll take a quick look and see if there is a way to solve that.

[cijothomas]

but I can suggest that this shouldn't concern the individual instrumentation packages themselves: note that the Http or Wcf or EntityFrameworkCore or any other instrumentations are agnostic of how the are instantiated, so they work with IConfiguration if present, and fallback to envvar if not.

I strongly believe this should be the case for all instrumentations, even if they normally aren't used with the Hosting package.

Yes I agree with that. @CodeBlanch has designed the infra pieces (core api/sdk) with this in mind already. Don't think every instrumentation/component has fully embraced it yet.

[CodeBlanch]

@julealgon Here is a diff which should fix Owin: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/compare/main...CodeBlanch:owin-iconfiguration-support?expand=1

You have any bandwidth to run with that and take it to PR by chance? I'm pretty swamped at the moment.

AspNet package should be similar process just rinse and repeat 😄

[cijothomas]

question:

Is https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Extensions.Hosting expected to be used with Asp.Net app at all? Asp.Net instrumentation, examples - none of them show that pattern.

[julealgon]

@CodeBlanch

You have any bandwidth to run with that and take it to PR by chance? I'm pretty swamped at the moment.

Not at the moment as I still need to run the CLA process through with my company.

I figured I'd share the workaround we used though, just in case others end up running into the same issue while this gets worked on.

First, I created a simple extension method that I could easily call on 2 of our projects (one that uses Owin instrumentation, and another that uses the AspNet instrumentation):

.ConfigureServices((context, services) =>
{
    services.AddOpenTelemetry()
        .PropagateLegacySettingsToEnvironmentVariables(context.Configuration)
        ...
        .ConfigureResource(c => c ...)
        .WithMetrics(m => m ...)
        .WithTracing(t => t ...)
        .UseOtlpExporter();
}

Code is fairly straightforward: just take the 2 "problematic" flags and forcibly set them as real environment variables in the process:

public static IOpenTelemetryBuilder PropagateLegacySettingsToEnvironmentVariables(
    this IOpenTelemetryBuilder builder,
    IConfiguration configuration)
{
    ArgumentNullException.ThrowIfNull(builder);
    ArgumentNullException.ThrowIfNull(configuration);

    ReadOnlySpan<string> legacySettingNames = [
        "OTEL_DOTNET_EXPERIMENTAL_ASPNET_DISABLE_URL_QUERY_REDACTION",
        "OTEL_DOTNET_EXPERIMENTAL_OWIN_DISABLE_URL_QUERY_REDACTION",
        ];

    foreach (var settingName in legacySettingNames)
    {
        if (configuration[settingName] is string settingValue)
        {
            Environment.SetEnvironmentVariable(settingName, settingValue);
        }
    }

    return builder;
}

Once this is fixed, we'll drop the extension without too much impact if at all.