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

[AspNetCoreInstrumentation] Incorrect value for http.route when using conventional routing #1730

Open
alanwest opened this issue Nov 16, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@alanwest
Copy link
Member

Bug

The http.route attribute is one of the more important HTTP attributes because users commonly wish to facet their metric and span data on http.route in order to understand the performance characteristics of specific endpoints in their applications.

ASP.NET Core apps that use conventional routing are not well supported. With conventional routing, it is common that a single route template matches many distinct routes. For these applications the instrumentation sets http.route to the value of the route template and not to a value that represents the actual route invoked. In turn, this means users lose the ability to understand the performance of specific endpoints.

This bug was introduced in open-telemetry/opentelemetry-dotnet#5026 to conform with the behavior of ASP.NET Core 8 which natively emits the http.server.request.duration metric. We wanted to avoid a surprise change in behavior for users migrating from earlier versions of .NET to .NET 8. Fixing this bug should be done in coordination with the ASP.NET Core team.

Reproduce

Run:

dotnet new mvc

The following is abbreviated, but Program.cs will look something like:

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddControllersWithViews();

var app = builder.Build();
app.UseRouting();
app.MapControllerRoute(
    name: "default",
    pattern: "{controller=Home}/{action=Index}/{id?}");

app.Run();

The application's HomeController contains multiple actions - e.g., Index, Privacy, etc. Invoking any of these endpoints will result in a metric and span with the http.route attribute set to {controller=Home}/{action=Index}/{id?}.

Suggested fix

Well-known route parameters (e.g., controller, action) should be replaced by their actual values. Using the HttpContext.GetRouteData() method is one way to get the actual route parameter values.

One potential option would be to apply the fix for .NET 6 and .NET 7 users gated by an environment variable.

@TimothyMothra
Copy link
Contributor

I hit this today. Sharing some more concrete examples:

I configured my test app same as Alan:

app.MapControllerRoute(
    name: "default",
    pattern: "{controller=Home}/{action=Index}/{id?}");

My test app has a HomeController and CustomerController.
Regardless which controller is invoked, The url.path shows the correct Controller/Action. The http.route shows the configured pattern.

  • https://localhost:44311/Home/Index

    Activity.ActivitySourceName: Microsoft.AspNetCore
    Activity.DisplayName:        GET {controller=Home}/{action=Index}/{id?}
    Activity.Kind:               Server
    Activity.Tags:
        server.address: localhost
        server.port: 44311
        http.request.method: GET
        url.scheme: https
        url.path: /
        network.protocol.version: 2
        http.route: {controller=Home}/{action=Index}/{id?}
        http.response.status_code: 200
    
  • https://localhost:44311/Customer/Index

    Activity.ActivitySourceName: Microsoft.AspNetCore
    Activity.DisplayName:        GET {controller=Home}/{action=Index}/{id?}
    Activity.Kind:               Server
    Activity.Tags:
        server.address: localhost
        server.port: 44311
        http.request.method: GET
        url.scheme: https
        url.path: /Customer/Index
        network.protocol.version: 2
        http.route: {controller=Home}/{action=Index}/{id?}
        http.response.status_code: 200
    

@stevenmccormack
Copy link

I am facing the exact same issue for all requests in net8. As a workaround I found the following:

.AddAspNetCoreInstrumentation(
            o => {
                o.EnrichWithHttpResponse = (activity, response) => {
                    var template = response.HttpContext.Request.Path;
                    activity.DisplayName = template;
                    activity.SetTag("http.route", template);
                };
            }
)

@DanielLaberge
Copy link

This regression needs some attention. I agree with the OP that the http.route attribute is very important.

Thanks for the workaround @stevenmccormack

@reyang reyang transferred this issue from open-telemetry/opentelemetry-dotnet May 13, 2024
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

4 participants