Add SelfDiagnostics module to provide infrastructure to log EventSource events - part 2

[xiang17]

Provide implementation for classes for step#3 and class structure for step#4 and step#5 in Implementation Plan section of #1258.

Changes

Add EventListener and Recorder (step#3), and boilerplate for Recorder (step#4) and ConfigRefresher (step#5).

(The TODO parts won't be covered in this PR and will be done in future PRs.)

[codecov]

Codecov Report

Merging #1306 into master will decrease coverage by 0.25%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1306      +/-   ##
==========================================
- Coverage   78.11%   77.85%   -0.26%     
==========================================
  Files         224      226       +2     
  Lines        6369     6386      +17     
==========================================
- Hits         4975     4972       -3     
- Misses       1394     1414      +20     

Impacted Files	Coverage Δ
src/OpenTelemetry/Internal/SelfDiagnostics.cs	0.00% <0.00%> (ø)
...lemetry/Internal/SelfDiagnosticsConfigRefresher.cs	0.00% <0.00%> (ø)
...Telemetry/Internal/SelfDiagnosticsEventListener.cs	0.00% <0.00%> (ø)
...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs	77.77% <0.00%> (-3.18%)	⬇️
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs	61.76% <0.00%> (-2.95%)	⬇️

[ejsmith]

I guess the plan is that we would be able to create event listeners for these and be able to create one that redirects the events to an ILogger? I see a lot of stuff marked internal currently. Can we please make sure this is open to be able to create other event listeners? :-)

[cijothomas]

we would be able to create event listeners for these and be able to create one that redirects the events to an ILogger? I see a lot of stuff marked internal currently. Can we please make sure this is open to be able to create other event listeners? :-)

Based on the description, this is not the plan. My understanding is - the selfdiag module just captures eventsource logs and writes them to disk. And only thing can configure is the file location/size/level. @xiang17 please correct if this is incorrect.

Self Diagnostics Module
The self diagnostics module captures and write log messages to disk, when enabled.

[ejsmith]

I really don't understand. It's got to be easy to debug what is going on with the client. Setting up a file that I have to go find and use some text editor to look at and hope that permissions work when attempting to write to it vs just outputting to the logging system that all of the modern apps built with .NET Core are already using and can be easily seen seems like a no brainer to me. I understand that the core client might not want to take a dependency on logging, but IMO it's got to be super easy and really the default behavior when targeting ASP.NET Core.

[xiang17]

I guess the plan is that we would be able to create event listeners for these and be able to create one that redirects the events to an ILogger? I see a lot of stuff marked internal currently. Can we please make sure this is open to be able to create other event listeners? :-)

Currently only the second feature in the proposal got agreement from the community, so I'm implementing the internal features only.

just outputting to the logging system that all of the modern apps built with .NET Core are already using
super easy and really the default behavior when targeting ASP.NET Core

The use case for this self diagnostics module is when a customer sees an issue with OpenTelemetry and tries to figure out what happened. It would be really handy if we can tell them to set up a configuration file and have the log file available without redeployment or restarting the server app, rather than asking them to do some development work to generate the logs for OpenTelemetry specifically.

We have a similar use case where our solution is asking the customer to run a custom tool or perfview to see whether the ETW events are flowing and what went wrong inside the SDK, however in some cases, the user cannot run programs adhoc, thus making it very hard to debug. There is also a tricky scenario where the customer sees different behavior when debugging locally and when deployed in prod hosts.

[cijothomas]

I really don't understand. It's got to be easy to debug what is going on with the client. Setting up a file that I have to go find and use some text editor to look at and hope that permissions work when attempting to write to it vs just outputting to the logging system that all of the modern apps built with .NET Core are already using and can be easily seen seems like a no brainer to me. I understand that the core client might not want to take a dependency on logging, but IMO it's got to be super easy and really the default behavior when targeting ASP.NET Core.

We can discuss this more on the original issue itself. We cannot do ILogger integration right now and have to stick with EventSource for now due to :

1. Lack of support of ilogger in net452/net46.
2. This library is used not just in Asp.Net core apps (where Ilogger integration is done out of the box), but in other non-host apps where its not straightforward to use ilogger. We need to first solve the issues like: who will provide the loggerfactory from which this library can create Ilogger instances and do the logging? (Its easy in Asp.Net core case, but not trivial if we are not in asp.net core app).

There is not yet a guarantee that Msft.Extensions.Logging is always backward compatible (like the way System.Diagnostics.DiagnosticSource is, which has a guarantee of no breaking changes, even with major version changes). This prevents us from simply taking a dependency on the newest version of it. (We have to conditional dependency based on runtime, and do code based on the actual version, as there are certain APIs available only on certain version)

[ejsmith]

Completely understand that you want the core implementation to use EventSource events. Also understand that you don't want to take a dependency on Msft.Extensions.Logging in the core. What I am saying is that this is a nice helper that listens to all of the EventSource events and does something with them. Currently you are working to write them out to a file. It seems like it would be really nice to make an ILogger implementation that takes the EventSource events and writes them to an ILogger instance instead of to a file. Then from the higher level packages that already reference ASPNET Core assemblies and register the APM client, it could also register the EventSource logger.

[cijothomas]

Yes, we can in the future consider "Ilogger" as a additional output apart from the file writing. Is this what you were suggesting?