Spike: Capture well known EventSource counters

[jamescrosswell]

Resolves #734

EventSource based metrics are a bit different to the OpenTelemetry metrics (and Sentry Metrics), so some consideration needs to be given to the API.

Initially what I've built is quite naive. You can define one or more EventSourceMatcher objects that are used to define which EventSources should be captured by our listener. The matchers consist of a SubstringOrRegEx expression that matches against the EventSource.Name and also a couple of other filters (for Level and something called EventKeywords, which is a flags enum) that will be applied to the listener as well):

sentry-dotnet/src/Sentry/Internal/SystemDiagnosticsEventSourceListener.cs

Lines 58 to 62 in bbde463

	if (_metricsOptions.CaptureSystemDiagnosticsEventSources
	.FirstOrDefault(matcher => matcher.IsMatch(candidateSource)) is { } match)
	{
	EnableEvents(candidateSource, match.Level, match.Keywords, Arguments.Value);
	}

The Arguments.Value supplied above configures events to be captured every 10 seconds, which aligns with our bucketing strategy... this would be one way to ensure we don't overcount metrics which are already pre-aggregated in the EventCounter.

Challenges

So far, so good. Where our implementation is a bit naive however is when we report the metrics to Sentry. This happens in the OnEventWritten method of our listener:

sentry-dotnet/src/Sentry/Internal/SystemDiagnosticsEventSourceListener.cs

Lines 76 to 96 in bbde463

	protected override void OnEventWritten(EventWrittenEventArgs eventData)
	{
	#if NET5_0_OR_GREATER
	DateTimeOffset eventTime = eventData.TimeStamp.ToUniversalTime();
	#else
	DateTimeOffset eventTime = DateTime.UtcNow;
	#endif
	var name = eventData.EventName ?? eventData.EventSource.Name + eventData.EventId.ToString();
	Dictionary<string, string> tags = new()
	{
	["EventSource"] = eventData.EventSource.Name,
	["EventId"] = eventData.EventId.ToString(),
	["Level"] = eventData.Level.ToString(),
	["Opcode"] = eventData.Opcode.ToString()
	};
	if (eventData.Message is { } message)
	{
	tags.Add("Message", message);
	}
	MetricsAggregator.Increment(name, 1, MeasurementUnit.None, tags, eventTime);
	}

There are a couple of challenges with this implementation.

Firstly, it assumes every Counter will be an incrementing counter and that the increment count will always be 1. This is rarely the case.

Secondly, most of the interesting data reported by counters is sent in the EventWrittenEventArgs.Payload property... this is unfortunately a ReadOnlyCollection<object?>? so, yeah, not strongly typed and specific to each EventCounter.

Potential Solution

Whilst our current implementation of the OnEventWritten method might work for some EventCounters, other counters will need to be handled differently. Effectively we have one "EventCounterProcessor" that could be registered to process certain counters for which it was appropriate, but we really need to allow SDK users to supply their own EventCounterProcessors (to handle whatever counters they might want to create themselves) and we should probably implement various built in processors to handle all of the (useful) Well Known counters.

For example, the code below gives a feel for how the System.Net.Sockets[bytes-sent] Eventcounter might be parsed (this code isn't very resilient but demonstrates how we'd need to pull info out of the Payload):

    protected override void OnEventWritten(EventWrittenEventArgs eventData)
    {
        if (eventData.EventName.Equals("EventCounters"))
        {
            foreach (var payload in eventData.Payload)
            {
                var counterPayload = (IDictionary<string, object>)payload;
                var name = (string)counterPayload["Name"];
                if (name == "bytes-sent")
                {
                    Console.WriteLine($"{name}: {counterPayload["Mean"]}");
                }
            }
        }
    }

That particular counter stores a bunch of stuff as an IDictionary<string, object> in the payload... with a bit of grunt work we could build some logic to pull all the useful info out and report it to Sentry... we could then encapsulate that logic in a processor which would (hopefully) be able to be reused with other EventCounters that stored their payloads similarly (and differed only in name).

Alternatives

It looks like there's an OpenTelemetry wrapper around counters... perhaps we could leverage this?

[codecov]

Codecov Report

Attention: 32 lines in your changes are missing coverage. Please review.

Comparison is base (c6c4d63) 76.87% compared to head (bbde463) 75.66%.
Report is 20 commits behind head on main.

Files	Patch %	Lines
src/Sentry/WellKnownEventCounters.cs	0.00%	16 Missing ⚠️
...y/Internal/SystemDiagnosticsEventSourceListener.cs	78.72%	9 Missing and 1 partial ⚠️
src/Sentry/EventSourceMatcher.cs	44.44%	5 Missing ⚠️
...rations/SystemDiagnosticsEventSourceIntegration.cs	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3100      +/-   ##
==========================================
- Coverage   76.87%   75.66%   -1.22%     
==========================================
  Files         357      361       +4     
  Lines       13466    13558      +92     
  Branches     2671     2684      +13     
==========================================
- Hits        10352    10258      -94     
- Misses       2435     2623     +188     
+ Partials      679      677       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jamescrosswell]

Discussed with @bitsandfoxes and we agreed to put this on pause for a bit until the Metrics capability in Sentry itself is a bit more mature.