HttpMessageHandler cannot be configured when initialization Serilog.Sinks.OpenTelemetry via configuration

[HHobeck]

Dear community.

I'm initialization the Serilog.Sinks.OpenTelemetry only via the configuration file and getting an error because the HTTP connection cannot be estabshiled. The reason why is quite simple because my application uses the default proxy. I have taken a look into the code and saw that it is not possible to inject the HttpMessageHandler via the options BatchedOpenTelemetrySinkOptions to disable the proxy.

Here is my serilog configuration in appsetttings.json:

  "Serilog": {
    "Using": [ "Serilog.Sinks.OpenTelemetry" ],
    "WriteTo:OpenTelemetry": {
      "Name": "OpenTelemetry",
      "Args": {
        "endpoint": "http://localhost:4318/v1/logs",
        "protocol": "HttpProtobuf"
    }
  }
}

And my host builder setup:

hostBuilder.ConfigureServices((hostBuilderContext, serviceCollection) =>
{
    serviceCollection.Configure<OpenTelemetrySinkOptions>(options =>
    {
        options.HttpMessageHandler = new HttpClientHandler() { UseProxy = false };
    });
});

hostBuilder.ConfigureLogging((hostBuilderContext, loggingBuilder) =>
{
    var loggerConfiguration = new LoggerConfiguration();

    ConfigurationReaderOptions configurationReaderOptions = new()
    {
        SectionName = $"{ConfigurationLoggerConfigurationExtensions.DefaultSectionName}"
    };
    Log.Logger = loggerConfiguration.ReadFrom.Configuration(
        hostBuilderContext.Configuration, configurationReaderOptions
    ).CreateLogger();

    loggingBuilder.AddSerilog();
});

I'm using the Serilog.Sinks.OpenTelemetry nuget package version 1.2.0 in a dot core 6.0 console application on windows10.

Regards
Hardy

[HHobeck]

I see also the usage of HttpClient in combination with the usage of HttpMessageHandler problematic:

serilog-sinks-opentelemetry/src/Serilog.Sinks.OpenTelemetry/Sinks/OpenTelemetry/Exporters/GrpcExporter.cs

Line 53 in 8aba132

grpcChannelOptions.HttpClient = new HttpClient(httpMessageHandler);

Better would be to use the IHttpClientFactory or inject the HttpClient via constructor. (see Guidelines for using HttpClient)

Anyway I think serilog-sinks-opentelemetry has two issues which might be worth to fix:

1. Always create the HttpClient via the dependency injection framework IServiceProvider to allow the user to setup retry policy or HttpMessageHandler or what every for HTTP communication with the build-in mechanism of dotnet core
2. Allow the user to specify the configuration options via the build-in configuration system by using the IOptions pattern.

In my opinion we are in the ecosystem of dotnet core and should use the concepts which are existing.

[nblumhardt]

Thanks for all the notes.

This seems like something to consider from the Serilog.Settings.Configuration angle; the configuration system is responsible for resolving parameters for Serilog sinks, so plugging in an IServiceProvider (via ReadFrom.Configuration()) or doing something options-based, over there, would provide this functionality for all Serilog sinks.

Needs a design sketch to get the ball rolling, I think 👍

[alsi-lawr]

I see also the usage of HttpClient in combination with the usage of HttpMessageHandler problematic:

serilog-sinks-opentelemetry/src/Serilog.Sinks.OpenTelemetry/Sinks/OpenTelemetry/Exporters/GrpcExporter.cs

Line 53 in 8aba132

grpcChannelOptions.HttpClient = new HttpClient(httpMessageHandler);

Better would be to use the IHttpClientFactory or inject the HttpClient via constructor. (see Guidelines for using HttpClient)

Anyway I think serilog-sinks-opentelemetry has two issues which might be worth to fix:

1. Always create the HttpClient via the dependency injection framework IServiceProvider to allow the user to setup retry policy or HttpMessageHandler or what every for HTTP communication with the build-in mechanism of dotnet core
2. Allow the user to specify the configuration options via the build-in configuration system by using the IOptions pattern.

In my opinion we are in the ecosystem of dotnet core and should use the concepts which are existing.

Serilog sinks aren't necessarily instantiated through dependency injection, so the IHttpClientFactory route wouldn't really be feasible unless we create a transient service collection during instantiation. We'd want to have the option for direct creation for .NET Framework support.

Even the UseLogger and AddLogger extensions will either reference a concrete Logger created ahead of time or it just uses the global Serilog.Log.Logger. So, while reading from configuration creates dependencies with reflection, the other route of logger configuration manually creates the dependencies.

The HttpClient method used follows the guidance, and is only used if you're manually overriding the HttpMessageHandler. Otherwise, the GrpcChannel manages the connection. Bear in mind that the logger (and GrpcExporter) is a singleton, so disposing of the HttpClient would never happen unless manually done.

In my opinion, the current implementation follows best practice for modern dotnet.

@nblumhardt I think that an easier approach would be to support mixed configuration (file and code) in Serilog.Settings.Configuration instead of the IServiceProvider route.
Something to consider:

• Which configuration would take precedence? Should the precedence be configurable?

I think an ideal scenario would be to have something like this:

loggerConfiguration
    .ReadFrom.Configuration(...)
    .ThenFrom.Options<SinkOptions>((opts) => ...)
...

Or just chained ReadFrom extensions if the semantic convenience of ThenFrom is too burdensome, where precedence is just the ordered call order.