Add support for endpoint routing in route template detection #2986
Description
Feature Request
For a while now (since ~3.0) ASP.NET Core has been pretty flexible about how it allows you to define routes/patterns, much of which boils down internally to endpoint routing. For example, that's what the default WebApplicationBuilder does and it's how attribute routing (and other routing) gets added to the route table.
Currently the HttpInListener specifically gets the attribute route info from the current request and uses that to set the display name and route information. While this works fine for attribute routing, it misses cases like...
app
.UseRouting()
.UseEndpoints(endpoints =>
{
endpoints.MapControllers();
endpoints.Map("/demo/{id}", ctx =>
{
ctx.Response.StatusCode = 200;
return ctx.Response.WriteAsJsonAsync("missed");
});
});The way to get routes out for endpoint routing is to get the IEndpointFeature out of the current request context:
if (context.Features.Get<IEndpointFeature>()?.Endpoint is RouteEndpoint endpoint)
{
// endpoint.RoutePattern is the pattern - from attribute or otherwise
// endpoint.RoutePattern.RawText is probably what the activity DisplayName should be
}
else
{
// Fallback to path or something that isn't route pattern.
}I noticed in the code that the MVC DiagnosticListener setup was referenced (which has actually moved to a new location since the old MVC repo is deprecated) and the current context does look like it's attached to the event so it should be available.
I also noticed the code mentioned...
// Reflection accessing: ActionDescriptor.AttributeRouteInfo.Template
// The reason to use reflection is to avoid a reference on MVC package.
// This package can be used with non-MVC apps and this logic simply wouldn't run.
// Taking reference on MVC will increase size of deployment for non-MVC apps....which doesn't make much sense as the OpenTelemetry.Instrumentation.AspNetCore library already references the MVC framework and is useless outside of an MVC context.
Is your feature request related to a problem?
Endpoint routing and other non-attribute route patterns are not caught by the activity naming logic.
Describe the solution you'd like:
I'd like the code to be updated to support endpoint routing, which provides a wider array of options in development.
Describe alternatives you've considered.
Right now I have to manually set the enricher to handle it, like:
services.AddOpenTelemetryTracing((builder) =>
builder
.AddAspNetCoreInstrumentation(b => b.Enrich = (activity, eventName, data) =>
{
HttpContext context;
if (data is HttpRequest request)
{
context = request.HttpContext;
}
else if (data is HttpResponse response)
{
context = response.HttpContext;
}
else
{
return;
}
if (context.Features.Get<IEndpointFeature>()?.Endpoint is RouteEndpoint endpoint)
{
// We like display name like: GET /normalized-lower-case-route
// so it's easier to differentiate and sort, but that's just our pref. You can
// see how we get the route pattern, though.
activity.DisplayName = $"{context.Request.Method.ToUpperInvariant()} {endpoint.RoutePattern?.RawText?.ToLowerInvariant()}";
}
else
{
// Fall back to path.
activity.DisplayName = $"{context.Request.Method.ToUpperInvariant()} {context.Request.Path.ToString().ToLowerInvariant()}";
}
})
.AddHttpClientInstrumentation()
.AddConsoleExporter());Additional Context
It would be nice if the enrichment mechanism was easier to wire up. Having to cast the data and check types like this is sort of painful given the ASP.NET Core instrumentation generally already knows what kind of data it's passing for each event type. Having something more strongly typed would be nice.
services.AddOpenTelemetryTracing((builder) =>
builder
.AddAspNetCoreInstrumentation(b =>
{
b.Enrich.OnStartActivity = (activity, httpRequest) =>
{
// Use the already-cast HttpRequest object to enrich the activity.
}
});