Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASP.NET http.route appears to be incorrect when exception handlers are invoked #5556

Closed
tyler-boyd opened this issue Apr 19, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@tyler-boyd
Copy link

Bug Report

List of all OpenTelemetry NuGet
packages
and version that you are
using (e.g. OpenTelemetry 1.0.2):

  • OpenTelemetry 1.7.0
  • OpenTelemetry.Instrumentation.AspNetCore 1.7.0

Runtime version (e.g. net462, net48, netcoreapp3.1, net6.0 etc. You can
find this information from the *.csproj file):

  • net8

Symptom

A clear and concise description of what the bug is.

What is the expected behavior?

http.route should always be populated, regardless of if the route handler threw an exception or not.

What is the actual behavior?

http.route is only populated when there's no exception; the tag is null when an exception occurs.

Reproduce

In lieu of a reproduction repo, I can point to the likely cause, which is a discrepancy between how the metrics instrumentation handles http.route compared to the tracing instrumentation.

  • Metrics:
    // Check the exception handler feature first in case the endpoint was overwritten
    var route = (context.Features.Get<IExceptionHandlerPathFeature>()?.Endpoint as RouteEndpoint ??
    context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
    if (!string.IsNullOrEmpty(route))
    {
    tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRoute, route));
    }
    , note that it checks context.Features.Get<IExceptionHandlerPathFeature>()?.Endpoint first, falling back to context.GetEndpoint()
  • Tracing:
    var routePattern = (context.Features.Get<IExceptionHandlerPathFeature>()?.Endpoint as RouteEndpoint ??
    context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
    if (!string.IsNullOrEmpty(routePattern))
    {
    RequestMethodHelper.SetActivityDisplayName(activity, context.Request.Method, routePattern);
    activity.SetTag(SemanticConventions.AttributeHttpRoute, routePattern);
    }
    , note it only checks context.GetEndpoint()

I'd assume the metrics way is correct, but am not positive 🤷‍♂️

@tyler-boyd tyler-boyd added the bug Something isn't working label Apr 19, 2024
@CodeBlanch
Copy link
Member

@tyler-boyd I think this was fixed by: #5135

Would you mind trying 1.7.1 or newer and report back?

@alanwest
Copy link
Member

Ah and you also mentioned that you're using net8. The instrumentation in this repository only emits metrics for net6 and net7. With net8 the http.server.request.duration metric is emitted natively by the ASP.NET Core itself.

See comment in the release notes.

Fixed an issue where the http.route attribute was not set on either the Activity or http.server.request.duration metric generated from a request when an exception handling middleware is invoked. One caveat is that this fix does not address the problem for the http.server.request.duration metric when running ASP.NET Core 8. ASP.NET Core 8 contains an equivalent fix which should ship in version 8.0.2 (see: dotnet/aspnetcore#52652). (#5135).

You'll need to update to ASP.NET Core 8.0.2+

@tyler-boyd
Copy link
Author

Thanks for the quick replies, I didn't see that linked issue because I was searching for http.route (which isn't explicitly mentioned).

Updating to the latest version of these libraries did the trick, I should have tried that before opening an issue 🙂 Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants