Support for error.type

[kaspertygesen]

Feature Request

Is your feature request related to a problem?

When using AspNetCoreInstrumentation an Activity gets status Error when a client cancels the request.
It is currently not very obvious why the trace has status Error in this case.

Describe the solution you'd like:

The semantic conventions has been updated with the addition of the error.type-attribute.

Support for error.type should be implemented in general and not only support requests cancelled by the client.

Example:
If error.type clearly states that the client cancelled the request, then it would be much more obvious why the trace has status Error.
It would also allow users to filter on cancelled requests if these are not considered errors in a given scenario.
This scenario is described in the spec here.

Describe alternatives you've considered.

During EnrichWithHttpResponse I could do one of the following but I would prefer general support for error.type.

• Changing the status of the activity to OK in case of cancelled requests
• Adding error.type in case of cancelled requests

Additional Context

The spec seems a little unclear on the well-known values for error.type.
It mentions the following values which doesn't seem to follow any common naming standard: WebSocketDisconnect; _OTHER; timeout; name_resolution_error; 500;

In addition the name of an exception may be used.

An initial discussion on which values to use for AspNetCoreInstrumentation might be a good first step.

[kaspertygesen]

Related discussion on introduction of error.type which states that leaving out well-known values is intentional: open-telemetry/semantic-conventions#205

[kaspertygesen]

This issue might also solve #3600 even though the proposed solution is different.

[kaspertygesen]

If possible I would like to take this issue my self when the time is right and we have settled on which values to use 😉

[kaspertygesen]

This might be relevant for .NET 8.
dotnet/aspnetcore#44697

[kaspertygesen]

Workaround for cancelled requests until error.type is implemented.

.AddAspNetCoreInstrumentation(options =>
{
    options.EnrichWithHttpResponse = (activity, response) =>
    {
        if (response.HttpContext.RequestAborted.IsCancellationRequested)
        {
            activity.AddTag("custom.error.type", "cancelled");
        }
    };
})

[vishweshbankwar]

@kaspertygesen Thanks for the detailed issue and the links. Here is my observation:

Status code in case of cancelled requests in ASP.NET Core will vary based on the target framework.

1. if targeted framework is net6.0/net7.0 then the status code is reported as 0. Reported in #3600 and #3800
2. if targeted framework is net8.0 and above then the status code is reported as 499. This was updated in Added operation cancel operation dotnet/aspnetcore#46330

Now, regarding the issue that you have raised. There are two possibilities we can go with

1. As per the spec, Span Status should be left Unset if the status code is in range 4XX.
   a) For .NET8.0 and above this means we do not do anything. It is already left as Unset
   b) For NET6.0\NET7.0, We could add a check for StatusCode 0 and set it to 499 for consistency.

2. As per spec we need to set the Span Status to Error in case of Cancelled requests. This also requires us to set error.type attribute.

2) is tricky when we compare it with metrics. Since, we are going to leverage metrics added in .NET8.0, We need to be consistent with how .NET8.0 treats cancelled requests. And it does not treat it as error. The metric is not reported with error.type attribute. It does report the status code as 499. e.g.

http.request.method: GET http.response.status_code: 499 http.route: foo network.protocol.name: http network.protocol.version: 2 url.scheme: https

I think we should go with option 1 i.e. do not add error.type attribute in case of cancelled requests. @JamesNK Could you please confirm if the above assessment is correct with respect to .NET8.0 changes?

[kaspertygesen]

@vishweshbankwar Thanks for the feedback. I will wait for a decision on which option to go with.

[vishweshbankwar]

Related: dotnet/aspnetcore#51620