Hosting Diagnostics risk double counting of events

[tim-gill]

Is there an existing issue for this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

There are many issues with using the current diagnostic events (for instance you need to use reflection to retrieve the payload for the majority of them - see #48616) but they are also incomplete assuming that the naming of "deprecatedxxx" for some payloads in the code means that they should not be taken as a dependency.

If a service developer is interested in adding custom logging/metrics per request they have two options - add their own middleware layer, or use a diagnostic events listener. The latter has a bunch of benefits in terms of code decoupling and avoiding unnecessary slowdown of the core request processing loop. However, the usage of it is problematic.

The "deprecated' event type Microsoft.AspNetCore.Hosting.EndRequest (payload type DeprecatedDiagnosticsEndRequest (private)) was fired for every successfully processed request and requests thowing unhandled exceptions instead raised Microsoft.AspNetCore.Hosting.UnhandledException events (also with a private class payload). This meant a listener could process both of these event types and get a 1:1 match with the processed events (see code here).
In contrast, developers not wanting to take a dependency on private class payloads with "deprecated" in the name could use the Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop event (with a HttpContext payload) and get all of the events. However, there is nothing in the context which flags the request as having raised an exception (see code here) - severely hampering its usefulness in powering custom logs and meters.

While you can potentially look at adding a flag to the httpcontext when processing UnhandledException events to stop double processing in the HttpRequestIn.Stop event processing, this effectively creates a public contract in the ordering of these events and makes things too brittle for long term reliability concerns.

Describe the solution you'd like

Ideally a single event type could be used here - Exception details raised in processing the request could be added to the request context and then be easily retrievable by a single event processor targeting HttpRequestIn.Stop.

I also encourage making all diagnostic payload classes public. Right now they are effectively public as any developer using them needs to use Reflection to retrieve payload values - this again makes the feature too dangerous to use for many developers.

Additional context

No response

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.