serilog/serilog-sinks-opentelemetry

Add a ConfigurationMethod to Populate ResourceAttributes-dictionary from appsettings.json

MikkelPorse opened this issue · 4 comments

MikkelPorse commented

When using new LoggerConfiguration().ReadFrom.Configuration(config) I would like to be able to assign values to ResourceAttributes: I'm thinking something like

"WriteTo": [
{
        "Name": "OpenTelemetry",
        "Args" : {
            "endpoint" : "http://localhost:4317",
            "protocol": "Grpc",
            "resourceAttributes": {
                "service.name": "my choice of ServiceName in stead of 'unknown service: executable.exe'."
            }
        }
    }
]

Of course that doesn't work without a little help. I pulled the source and built it into the existing ConfigurationExtension like so

public static LoggerConfiguration OpenTelemetry(
    this LoggerSinkConfiguration loggerSinkConfiguration,
    string endpoint = OpenTelemetrySinkOptions.DefaultEndpoint,
    OtlpProtocol protocol = OpenTelemetrySinkOptions.DefaultProtocol,
    IDictionary<string,object>? resourceAttributes = null)
{
    if (loggerSinkConfiguration == null) throw new ArgumentNullException(nameof(loggerSinkConfiguration));

    return loggerSinkConfiguration.OpenTelemetry(options =>
    {
        options.Endpoint = endpoint;
        options.Protocol = protocol;
        if (resourceAttributes is not null)
            options.ResourceAttributes = resourceAttributes!;
    });
}

It works but is sort of hard to unittest without resorting to reflection. Could the above be integrated into the project? Or is there a better way to do what I propose?

nblumhardt commented

Hi!

I think the intended path for this is serilog/serilog-settings-configuration#398 - and some help over there would be most welcome if you have the opportunity.

I can see the place for resourceAttributes in a top-level configuration method, though, given their prominence in the OTel world. We initially dropped this but might need to reconsider - any thoughts @loomis ?

MikkelPorse commented

Ah, I didn't think to look there, as I assumed it would be Otel-specific. :-)

I think maybe 0xced's suggestion is a bit ... ambitious ... compared to say FileLoggerConfigurationExtensions, where all (?) the possible configuration combinations have been brute-forced into extension methods, which are easily understood and require a fair bit less testing - and would not have to deal with options that refer to object instances like the LevelSwitch.

  • 👍1
nblumhardt commented

See also #123, just merged, which will support the changes in Serilog.Settings.Configuration.

MikkelPorse commented

Solved by Serilog config PR #405. Horray 🎉

  • 🎉1