dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.52k stars 10.04k forks source link

[ASP.NET Core metrics] Discrepancies between spec and implementation #51454

Open vishweshbankwar opened 1 year ago

vishweshbankwar commented 1 year ago

Is there an existing issue for this?

Describe the bug

http.server.request.duration

1) Actual description does not match expected description src: https://github.com/dotnet/aspnetcore/blob/6e1cebdcb07c3577023f68825b20c60a3692d540/src/Hosting/Hosting/src/Internal/HostingMetrics.cs#L32 spec; https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md#metric-httpserverrequestduration:~:text=Duration%20of%20HTTP%20server%20requests

2) Do we need network.protocol.name? Spec says recommended only if not http by default. HttpClient does not report this attribute as well. https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs

src https://github.com/dotnet/aspnetcore/blob/6e1cebdcb07c3577023f68825b20c60a3692d540/src/Hosting/Hosting/src/Internal/HostingMetrics.cs#L57

spec https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md#metric-httpserverrequestduration

http.server.active_requests

1) Actual description does not match expected description as per spec. src: https://github.com/dotnet/aspnetcore/blob/c379b08edd68e6864b327e486d6e97352841db3a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs#L27

spec: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md#metric-httpserverrequestduration:~:text=Number%20of%20active%20HTTP%20server%20requests.

Expected Behavior

Metric description matches the spec.

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

.NET8

Anything else?

No response

vishweshbankwar commented 1 year ago

@JamesNK @lmolkova FYI

JamesNK commented 1 year ago

I don't think the counter descriptions have to match, but it doesn't hurt because the same.

Removing the http tag would be good. The question is whether we can make that change for .NET 9.