juliuskoval / NLog.Targets.OpenTelemetryProtocol

1 stars 4 forks source link

Share named options infrastructure for OtlpExporterOptions via NLog RegisterServiceProvider support? #13

Closed TheXenocide closed 2 months ago

TheXenocide commented 3 months ago

Rather than using the default ILogger implementation and having two completely separate configuration areas for logging (NLog, OpenTelemetry), I am hoping to leverage this target to send all logs to NLog and then configure NLog to send specific logs to OTLP. The challenge that I'm facing now is that I would like for this logging target to play nicely with all of our other OpenTelemetry configuration for both appsettings-based and code-based configuration using consistent DI conventions through the IOptions pattern.

It looks like this target creates its own instance of OtlpExporterOptions which is configured via options on the NLog target. I can understand there might be some intentional use for that, but I would dearly appreciate it if it would provide a setting for loading the options via the DI system (NLog provides an option for exposing the IServiceProvider via its RegisterServiceProvider option) and, additionally a setting for specifying the name of the options instance to use.

Per the guidance in the opentelemetry-dotnet repo:

DO support the .NET Options pattern and DO support named options. The Options pattern allows users to bind configuration to options classes and provides extension points for working with instances as they are created. Multiple providers may exist in the same application for a single configuration and multiple components (for example exporters) may exist in the same provider. Named options help users target configuration to specific components.

In practice, what I would like to do is something like this:

In Program.cs, use a consistent pattern for all telemetry configuration:

// Use the shared settings for each telemetry type
services.Configure<OtlpExporterOptions>("Tracing", context.Configuration.GetSection("OpenTelemetry:OTLP"));
services.Configure<OtlpExporterOptions>("Metrics", context.Configuration.GetSection("OpenTelemetry:OTLP"));
services.Configure<OtlpExporterOptions>("Logging", context.Configuration.GetSection("OpenTelemetry:OTLP"));

// Then allow signal-specific overrides
services.Configure<OtlpExporterOptions>("Tracing", context.Configuration.GetSection("OpenTelemetry:Tracing:OTLP"));
services.Configure<OtlpExporterOptions>("Metrics", context.Configuration.GetSection("OpenTelemetry:Metrics:OTLP"));
services.Configure<OtlpExporterOptions>("Logging", context.Configuration.GetSection("OpenTelemetry:Logging:OTLP"));

In appsettings.NLog.json, which we're using instead of NLog.config for very similar (layered configurability) reasons:

{
  "NLog": {
    "targets": {
      "otlp": {
        "type": "OtlpTarget",
        "useInjectedOptions": true,
        "injectedOptionsName": "Logging"
      }
    }
    // ...
  }
}
TheXenocide commented 3 months ago

Oof, I just pulled down source to see how much of an effort it would be to do this myself and noticed that the official 1.8.0 release (which we are using elsewhere) has the vast majority of these types conditionally set to internal instead of public. This makes it feel like I won't be able to leverage this target in our upcoming efforts. Is there supposed to be a different way of implementing this type of integration now?

juliuskoval commented 3 months ago

I'll try to take a look at the issue itself tomorrow, but to answer your question, the .NET implementation of OpenTelemetry logging isn't stable yet, so the APIs related to it are internal in stable releases and public in prerelease versions of the OpenTelemetry package. But right now I don't know of a way to make the target work with stable releases of OpenTelemetry.

TheXenocide commented 3 months ago

Understandable.

Better logging integration with Telemetry is an important enough upcoming objective of ours that we may be able to rationalize using pre-release versions for this purpose, provided it's stable enough in practice to use.

We've also considered just making our own NLog target that created its own internal ILoggingBuilder and configured it for OTLP, then just calling that from our custom target, but that really feels more like a throw-away (eventually) band-aid than taking steps the right direction for a real approach we'd like to pursue, which this library appears to offer (at a glance, at least).