Introduce request error type attribute

[lmolkova]

Fixes #204

Proposal:

• Reserve an attribute for the error code when response code was not received (or reading response body has failed) - error.type (TBD)
• Start with a small, non-controversial set of error types common across HTTP clients in different languages. Don't try to unify common errors initially. Even things like timeout (vs cancellation) can be controversial.
• Define 'other' as a dimension cap
• Allow instrumentations to report custom (known, documented, low-cardinality) error types
• If we see similarities in error types across clients/languages, we can consider adding common valuesit into the enum even after HTTP semconv goes stable

(naming and common error suggestions are welcome!)

Error rate queries

• rate(http_request_duration_count{error_type!=""}[1m]) by error_type

[lmolkova]

/cc @trask @noahfalk @JamesNK @antonfirsov

[noahfalk]

This sounded fine to me, perhaps "http.request.error.reason" -> http.client.error.reason or http.error.reason?

What makes sense to me is that http.request.error.reason would encode errors that occurred locally and status_code covers errors that occurred remotely received via http response. This means all combinations of (error.reason is set or unset) x (status code is unset, status code is 200, status code is non-200) are possible. For span errors, I would guess you could have a span error if error.reason is set OR status_code == (whatever set of statuses are defined as errors).

[mateuszrzeszutek]

• Span status is OK or UNSET -> no http.request.error.reason
• Span status is ERROR -> http.request.error.reason is _OTHER by default, it might also be possible to extract it from span status description

This might be a thing for a separate issue, but in the Java world I know of several HTTP instrumentations (Reactor Netty, Spring Webflux) that have knowledge if the request is cancelled, but they don't set the span status to ERROR cause there is no exception thrown. Would we want to set the span status in that case? Or an attribute?

[jsuereth]

This looks to cover a major gap we have.

One thing I want to call out -

Some metric backends treat absence of an attribute as an "empty" attribute. As such, we should probably REQUIRE that the reason is never empty, but also allow the "empty" means there was no reason.

[lmolkova]

This might be a thing for a separate issue, but in the Java world I know of several HTTP instrumentations (Reactor Netty, Spring Webflux) that have knowledge if the request is cancelled, but they don't set the span status to ERROR cause there is no exception thrown. Would we want to set the span status in that case? Or an attribute?

@mateuszrzeszutek great point!
I updated the condition so it accounts for no-exception cancellation.

I think span status UNSET and http.error.reason = canceled would do the trick?
Cancellation is not necessarily an error for the end user (if I intentionally cancel an operation that's not necessary anymore).
So perhaps no need to translate span status to the metric attribute since the attribute is defined for both?

[lmolkova]

Some metric backends treat absence of an attribute as an "empty" attribute. As such, we should probably REQUIRE that the reason is never empty, but also allow the "empty" means there was no reason.

@jsuereth
I added empty value to the enum, but don't see why we need to REQUIRE it to be present. If it's the case we also need it for the HTTP response status code that could be missing.

[Oberon00]

The PR title still contains only "metrics" which might make some people skip over it that would want to look at tracing-related PRs

[mateuszrzeszutek]

I think span status UNSET and http.error.reason = canceled would do the trick?
Cancellation is not necessarily an error for the end user (if I intentionally cancel an operation that's not necessary anymore).

Yes, this makes sense to me. Thanks!

So perhaps no need to translate span status to the metric attribute since the attribute is defined for both?

👍

[Oberon00]

I think a common list (at least as recommendation for the most common ones) of error categories is achievable and is actually required if you want to have low cardinality in large systems with multiple techs.

However, we should probably not come up with our own list but use an existing one, like https://en.cppreference.com/w/cpp/error/errc or https://pkg.go.dev/google.golang.org/grpc/codes

[antonfirsov]

Client part LGTM.