Missing route pattern in requests that do not reach the MVC middleware

[aelij]

Bug Report

OpenTelemetry NuGet packages:

• OpenTelemetry.Exporter.Console 1.3.0
• OpenTelemetry.Extensions.Hosting 1.0.0-rc9.4
• OpenTelemetry.Instrumentation.AspNetCore 1.0.0-rc9.4

Runtime version:

• net6.0

Symptom

When MVC requests get stopped by previous middlewares (e.g. authentication), the Microsoft.AspNetCore.Mvc.BeforeAction event isn't sent and so the Activity isn't properly renamed with the route pattern and the http.route tag isn't added.

This is a problem since we can't easily identify requests that belong to the same operation due to various failures in middlewares.
In addition, requests that do not use MVC don't have the route set either.

What is the expected behavior?

The activity name and http.route tag should be equal to the route pattern.

Suggestion for a solution - Instead of relying on Mvc.BeforeAction use Microsoft.AspNetCore.Routing.EndpointMatched, which would also work outside of MVC. Then get the route pattern using:

(httpContext.GetEndpoint() as RouteEndpoint)?.RoutePattern?.RawText

What is the actual behavior?

The activity name equals the full URI path and http.route is empty.

Reproduce

otel-missing-route.zip

public class MyController : Controller
{
    [HttpGet("hasroute/{value}")]
    public int HasRoute(int value) => value;

    [HttpGet("noroute/{value}"), Authorize]
    public int NoRoute(int value) => value;
}

Calling those 2 endpoints (while unauthenticated) produces:

Activity.DisplayName: hasroute/{value}
http.route: hasroute/{value}

and

Activity.DisplayName: /noroute/1

Activity.TraceId:          0ddb494d4312a1033e8650c55b83b894
Activity.SpanId:           5f78fc1e69504bc6
Activity.TraceFlags:           Recorded
Activity.ActivitySourceName: OpenTelemetry.Instrumentation.AspNetCore
Activity.DisplayName: hasroute/{value}
Activity.Kind:        Server
Activity.StartTime:   2022-07-20T14:58:10.5872211Z
Activity.Duration:    00:00:00.1429528
Activity.Tags:
    http.host: localhost:7196
    http.method: GET
    http.target: /hasroute/1
    http.url: https://localhost:7196/hasroute/1
    http.route: hasroute/{value}
    http.status_code: 200
   StatusCode : UNSET
Resource associated with Activity:
    service.name: unknown_service:WebApplication4

Activity.TraceId:          3d8431517010c153dcb6f681bf50a0a4
Activity.SpanId:           7a30556bc3aa45c3
Activity.TraceFlags:           Recorded
Activity.ActivitySourceName: OpenTelemetry.Instrumentation.AspNetCore
Activity.DisplayName: /noroute/1
Activity.Kind:        Server
Activity.StartTime:   2022-07-20T14:58:15.9094598Z
Activity.Duration:    00:00:00.0347058
Activity.Tags:
    http.host: localhost:7196
    http.method: GET
    http.target: /noroute/1
    http.url: https://localhost:7196/noroute/1
    http.status_code: 401
   StatusCode : UNSET
Resource associated with Activity:
    service.name: unknown_service:WebApplication4

[zacharycmontoya]

@cijothomas please let me know if this belongs in a separate issue, but I've found a somewhat related issue where the route pattern is missing when the action uses conventional routing.

This can be repro'ed by extending the provided repo with the following code changes:

1. Program.cs

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

2. MyController.cs

    [HttpGet]
    public int ConventionalRoute(int value) => value;

When the new action is reached by sending a request to /my/conventionalroute/3, the instrumentation produces an Activity with no http.route tag and and a display name of /my/conventionalroute/3.

Suggestion for a solution - Instead of relying on Mvc.BeforeAction use Microsoft.AspNetCore.Routing.EndpointMatched, which would also work outside of MVC. Then get the route pattern using:

(httpContext.GetEndpoint() as RouteEndpoint)?.RoutePattern?.RawText

If the proposed solution is adopted, then the display name and the http.route would be populated with the pattern {controller=Home}/{action=Index}/{id?}. Would this be the desired outcome for the conventional routing scenario?

[cijothomas]

@vishweshbankwar FYI

[cliedeman]

Not sure if I should log a seperate issue but I have a related problem.

The http.route is populate when the request is successful. The http.route is missing on 500s

[HamidEbr]

Not sure if I should log a seperate issue but I have a related problem.

The http.route is populate when the request is successful. The http.route is missing on 500s

"http.route" field is filled when the request is successful and returns a status code of 200, but it is not filled in when the status code is 400 inside a request.
I have noticed that when directly using Results.StatusCode(400), "http.route" is filled.

I use the Latest prerelease 1.5.0-rc.1.

[kerams]

Suggestion for a solution - Instead of relying on Mvc.BeforeAction use Microsoft.AspNetCore.Routing.EndpointMatched, which would also work outside of MVC. Then get the route pattern using:

(httpContext.GetEndpoint() as RouteEndpoint)?.RoutePattern?.RawText

I don't use UseRouting or UseEndpoints, so there isn't even any Endpoint. While I could set one, RoutePattern doesn't seem to have any public constructors.

What I am able to do, is store my route patterns in HttpContext.Items, pull them out in the enrichment callback in AddAspNetCoreInstrumentation and extend the tag list. Prometheus metrics are then grouped as expected, but it would be nice if scenarios that do not use the built-in ASP.NET routing at all could publish route patterns more easily.

[cliedeman]

@HamidEbr #5134 should fix this.

As a temporary workaround you can manually add the tag using the enrich method:

.AddAspNetCoreInstrumentation(opts =>
{
    opts.Enrich = (string _, HttpContext context, ref TagList tags) =>
    {
        ...
        var route = (exceptionHandlerFeature.Endpoint as RouteEndpoint)?.RoutePattern.RawText;
    };
})

[chulliyilmahesh]

Hi, i have a similar issue with AspNetInstrumentation (not core). All the traces are turning up like {controller}/{action}/{id} in the new relic where we are sending the data for now. I have been going through the documentation to find any solution. any pointers to as how it can be implemented.