[release/8.0] Fix metrics duration and http.route tag with exception handling

[JamesNK]

[release/8.0] Fix metrics duration and http.route tag with exception handling

The http.server.request.duration counter records information about every HTTP request. One piece of recorded information is the http.route which is the matched route pattern. The route pattern is taken from HttpContext's Endpoint. This information is very useful because it allows someone to match requests with pages in their web app.

There is an interaction between metrics and exception handling that causes http.route to be lost. It happens when:

• Incoming request to an app that has enabled metrics
• There is an unhandled exception
• App is using UseExceptionHandler
• Response hasn't started or aborted
• The exception handler clears the HttpContext and re-executes the middleware pipeline <- problem
• Value is recorded to http.server.request.duration without http.route

The exception handler clears the endpoint from the context and re-executes the middleware pipeline. There is no longer an endpoint on the context when http.server.request.duration is recorded and the http.route value is lost.

The fix is to stash the original endpoint to a known location (a stable key in HttpContext.Items) and then fall back to getting the endpoint from that location if the endpoint isn't set. HttpContext.Items is used because IExceptionHandlerPathFeature (which has an Endpoint property with the original endpoint) isn't available at the hosting layer.

Fixes #52648

Customer Impact

Has been reported by customers in the opentelemetry-dotnet repo and the aspnetcore repo.

Without this fix, customers using metrics would unexpectedly have no http.route value when a request throws an exception and exception handling runs. It would be non-obvious why this happens, and create confusion about where errors are occurring in their web app.

Regression?

• Yes
• No

Risk

• High
• Medium
• Low

The change modifies code that runs when request metrics are recorded. It runs often, but the change is very simple (looking up a value in a dictionary, or setting a value in a dictionary when there is an error)

Verification

• Manual (required)
• Automated

Manual test with this change correctly includes http.route value:

Packaging changes reviewed?

• Yes
• No
• N/A

[ghost]

Hi @JamesNK. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

[ghost]

Hi @JamesNK. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

[ghost]

Hi @JamesNK. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

[JamesNK]

Approved over email.

[ghost]

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

[wtgodbe]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).