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.32k stars 9.97k forks source link

HTTP OTEL metrics: use `error.type` instead of `exception.type` attribute #51029

Closed lmolkova closed 8 months ago

lmolkova commented 1 year ago

Is there an existing issue for this?

Describe the bug

OTel recently defined error.type attribute as a general-purpose error status. It is used on http.server.* and other metrics (https://github.com/open-telemetry/semantic-conventions/pull/205).

ASP.NET Core now reports exception.type on .NET-specific metrics like (kestrel.*, aspnetcore.*) as well as common HTTP metrics.

e.g. here

https://github.com/dotnet/aspnetcore/blob/36e03b5be98b34804fcf23a2e9c05a5211e1108b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs#L77

Reporting exception.type on common OTel metrics makes .NET incompatible with the OTel spec.

Expected Behavior

ASP.NET Core metrics in http.* namespace should use error.type attribute (populating full exception type on it, in the same was as today). .NET-specific metrics can keep using exception.type or switch to exception.type.

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

8.0

noahfalk commented 1 year ago

@JamesNK

JamesNK commented 1 year ago

Lets switch to error.type everywhere.

JamesNK commented 1 year ago

I noticed that exception.type is still used by logging and tracing in the conventions. Is it intentional that there is a difference between metrics and other telemetry?

https://github.com/search?q=repo%3Aopen-telemetry%2Fsemantic-conventions%20%22exception.type%22&type=code

lmolkova commented 1 year ago

I noticed that exception.type is still used by logging and tracing in the conventions. Is it intentional that there is a difference between metrics and other telemetry?

https://github.com/search?q=repo%3Aopen-telemetry%2Fsemantic-conventions%20%22exception.type%22&type=code

from offline conversation - those are semconv for exceptions on logs/events and not for traces/metrics. The error.type attribute is intended for traces/metrics and all sorts of errors (exceptions or not)

JamesNK commented 8 months ago

Fixed with https://github.com/dotnet/aspnetcore/pull/51084