Using ILogger instead of EventSource for logging exceptions

[timmydo]

Is your feature request related to a problem?

We're running the prometheus exporter middleware in production container environment where we have a setup with ILogger exporting to Geneva. We don't have the means to connect perfview or read files on disk to diagnose why the prometheus exporter is returning 500.

Describe the solution you'd like:

What do you want to happen instead? What is the expected behavior?

https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs#L92

            catch (Exception ex)
            {
                PrometheusExporterEventSource.Log.FailedExport(ex);
                if (!response.HasStarted)
                {
                    response.StatusCode = 500;
                }
            }

something like this

                LogFailedExportException(LogLevel.Error, this.logger);

    public partial class PrometheusMiddlewareLogs
    {
        [LoggerMessage(EventId = 1, Message = "ExportFailed exception")]
        static partial void LogFailedExportException(LogLevel logLevel, ILogger logger);
    }

I looked at replacing the middleware class but exporter.CollectionManager is internal so I might need to use some reflection to get around that if we have to hack a fix together.

@cijothomas

[cijothomas]

Linking previous attempts https://github.com/open-telemetry/opentelemetry-dotnet/pull/1867

[cijothomas]

Tagging for consideration in 1.6, as that is the milestone when we expect to finalize Logging changes which are currently worked on in main-logs branch.

[cijothomas]

Linking to https://github.com/open-telemetry/opentelemetry-dotnet/issues/3447#issuecomment-1185329104

[pharring]

If you do this (replace internal EventSources with ILogger), please make sure that ActivityStarted/ActivityStopped events are turned into logger scopes (BeginScope/Dispose).

[cijothomas]

If you do this (replace internal EventSources with ILogger), please make sure that ActivityStarted/ActivityStopped events are turned into logger scopes (BeginScope/Dispose).

I am not sure why is that relevant/needed? Could you elaborate...

[Code-Grump]

I am somewhat confused - if I'm routing my logging via OTel and I'm having problems with an exporter, writing the problems back into that system feels like it could make the problem worse, or otherwise make it harder to diagnose. Writing to an ILogger is definitely a lot easier to work with than an EventSource, I'd just like to see an example of how this is expected to work.

[cijothomas]

I am somewhat confused - if I'm routing my logging via OTel and I'm having problems with an exporter, writing the problems back into that system feels like it could make the problem worse, or otherwise make it harder to diagnose. Writing to an ILogger is definitely a lot easier to work with than an EventSource, I'd just like to see an example of how this is expected to work.

It is a valid point, and yes, solving that (i.e avoiding dead-loop situations) is part of implementation details. See https://github.com/open-telemetry/opentelemetry-dotnet/pull/1867#discussion_r588066689

[cijothomas]

@Yun-Ting is still working on it targeting 1.10 milestone.