You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We use the OpenTelemetry ASP.NET Core Instrumentation to collect traces and metrics for http server spans. Specifically the http.server.request.duration metric and the name of span is of interest.
Due to the nature of how IdentityServer handles routing with IEndpoint; all metrics are missing the http.route attribute. The relevant code that populates the attribute can be found in the OpenTelemetry repository.
The inherent problem is that it relies on RouteEndpoint, but IdentityServer relies on a custom IEndpoint with a router and additional middleware that does not play well with the rest of the ASP.NET ecosystem.
The reproduction example listen below was used to generate the following screenshots.
Note that the marked area in the above screenshot should really measure .well-known/openid-configuration.
For traces, the display name is only GET without the route. The EndpointRoute class is again used by OpenTelemetry to populate this.
The end result is that both traces and metrics end up becoming quite useless as they do not contain the route as seen here:
Steps to reproduce
Add the OpenTelemetry ASP.NET Core Instrumentation to a project that uses IdentityServer and observe the traces and metrics.
I created a minimal example that can be found here that demonstrates the issue. Just follow the instructions in the README.
Potential solutions
I think the best solution would be to simply adopt ASP.NET Endpoint Routing. Should dynamic routing be required, use the EndpointDataSource functionality.
I know endpoint routing was previously discussed but I can't find the issue right now. I think it would be a good idea to revisit this and see if you can make it work with IdentityServer. This would also open up opportunities for swagger integration.
Workarounds
For now we will likely have to re-implement IdentityServerMiddleware and populate the http.route attribute and the span name manually. This is not ideal as it will require maintenance and will not be as accurate as the built-in functionality.
app.Use(async(context,next)=>{varrouter= context.RequestServices.GetRequiredService<IEndpointRouter>();varendpoint= router.Find(context);if(endpoint is not null){varresult=await endpoint.ProcessAsync(context);if(result is not null){// Not an ideal way to get the path, but it works.// If the path has any route parameters, they will be included which is less than ideal.varpath= context.Request.Path.Value?.Trim('/');vartagsFeature= context.Features.Get<IHttpMetricsTagsFeature>();if(tagsFeature is not null){ tagsFeature.Tags.Add(newKeyValuePair<string,object?>("http.route", path));}// Get the activity from the context and set the display name to the path.varactivity= System.Diagnostics.Activity.Current;if(activity is not null){ activity.DisplayName +=$"{path}";}await result.ExecuteAsync(context);}return;}await next();});
While this works, we really shouldn't have to do this.
The text was updated successfully, but these errors were encountered:
If I got the history right, IdentityServer4 had the endpoint concept before Asp.Net Core had endpoints and routing. The discussion was then if it was worth the effort to change. The conclusion was that endpoints could not replace the custom middlewares so IdSrv might as well continue using the existing solution.
Now with Open Telemetry (and possibly other features?) working with endpoint routing but not with IdSrv endpoints it might be worth to reconsider.
That's the issue I was referring to yes. I couldn't determine the reasoning behind why standard middleware or endpoint filtering couldn't achieve what you have today. From what I can see IdentityServer uses standard ASP.NET middleware?
It would be appreciated if you could reconsider or take another look at endpoint routing.
Relevant dependencies
Description
We use the OpenTelemetry ASP.NET Core Instrumentation to collect traces and metrics for http server spans. Specifically the
http.server.request.duration
metric and the name of span is of interest.Due to the nature of how IdentityServer handles routing with
IEndpoint
; all metrics are missing thehttp.route
attribute. The relevant code that populates the attribute can be found in the OpenTelemetry repository.The inherent problem is that it relies on
RouteEndpoint
, but IdentityServer relies on a customIEndpoint
with a router and additional middleware that does not play well with the rest of the ASP.NET ecosystem.The reproduction example listen below was used to generate the following screenshots.
Note that the marked area in the above screenshot should really measure
.well-known/openid-configuration
.For traces, the display name is only
GET
without the route. TheEndpointRoute
class is again used by OpenTelemetry to populate this.The end result is that both traces and metrics end up becoming quite useless as they do not contain the route as seen here:
Steps to reproduce
Add the OpenTelemetry ASP.NET Core Instrumentation to a project that uses IdentityServer and observe the traces and metrics.
I created a minimal example that can be found here that demonstrates the issue. Just follow the instructions in the README.
Potential solutions
I think the best solution would be to simply adopt ASP.NET Endpoint Routing. Should dynamic routing be required, use the
EndpointDataSource
functionality.I know endpoint routing was previously discussed but I can't find the issue right now. I think it would be a good idea to revisit this and see if you can make it work with IdentityServer. This would also open up opportunities for swagger integration.
Workarounds
For now we will likely have to re-implement
IdentityServerMiddleware
and populate thehttp.route
attribute and the span name manually. This is not ideal as it will require maintenance and will not be as accurate as the built-in functionality.While this works, we really shouldn't have to do this.
The text was updated successfully, but these errors were encountered: