Optional HTTP Client 404 as OK configurable

[driverpt]

Is your feature request related to a problem? Please describe.

Currently on any Http Client, any responses with 404 are setting the Span to ERROR, which may not be the case (in our case Http Status Code is used as an Application Protocol).

We have OTEL Collector to sample any kind of error and in some cases this is always being sampled, causing a lot of data ingestion on the Observability Provider, causing a bill increase.

Describe the solution you'd like

We'd like to have something like OTEL_INSTRUMENTATION_HTTP_CLIENT_404_NO_ERROR=true which defaults to false but makes OTEL Instrumentation interpret HttpClient's 404 on Spans as UNSET or OK.

Describe alternatives you've considered

We have tried disabling Instrumentation of Http Clients, but we'd lose the visibility

Additional context

No response

Tip

_{React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.}

[driverpt]

@trask , WDYT ?

[trask]

yeah, this has come up before, see open-telemetry/semantic-conventions#1003 (comment)

I wish there was a way for a caller to signal that the http client should treat a 404 as success. Something like OTEL_INSTRUMENTATION_HTTP_CLIENT_404_NO_ERROR is a very broad brush.

We have OTEL Collector to sample any kind of error and in some cases this is always being sampled

is there a way to have the OTEL Collector not treat these 404s as an error?

[breedx-splk]

Apologies in advance, I am sorry, but 404 is still an error. It always has been. Even if it's an expected error, you have attempted to access a resource that is not found, and that is considered an error condition. If your application is choosing to treat 404s as anything other than an error, then I think you're choosing to misuse http.

The semantic conventions are also pretty clear about this:

For HTTP status codes in the 4xx range span status MUST be left unset in case of SpanKind.SERVER and SHOULD be set to Error in case of SpanKind.CLIENT.

I think that allowing 404s to be treated as non-errors is a mistake.

[trask]

If your application is choosing to treat 404s as anything other than an error

I'm not sure if I agree

It's not uncommon to implement "create if not exists" operations by checking if GET returns 404

[driverpt]

Apologies in advance, I am sorry, but 404 is still an error. It always has been. Even if it's an expected error, you have attempted to access a resource that is not found, and that is considered an error condition. If your application is choosing to treat 404s as anything other than an error, then I think you're choosing to misuse http.

The semantic conventions are also pretty clear about this:

For HTTP status codes in the 4xx range span status MUST be left unset in case of SpanKind.SERVER and SHOULD be set to Error in case of SpanKind.CLIENT.

I think that allowing 404s to be treated as non-errors is a mistake.

Emphasis on SHOULD, it's not a MUST. For some Apps 404 is not an actual error. Because semantically it's only says that the resource does not exist. IMHO the signaling of error or not should go to the SDK User.

[breedx-splk]

I'm not sure if I agree
It's not uncommon to implement "create if not exists" operations by checking if GET returns 404

I bet people do do that, but that doesn't seem like a good practice. HEAD seems better suited than GET for that use case, but there are still race conditions (where the resource could be created between the "check" and the "set")....which is why people would normally use an HTTP POST for such cases, and verify that 201 Created was returned (or an error if it already exists.).

Emphasis on SHOULD, it's not a MUST.

Right, so setting the status to something other than error wouldn't strictly be a spec violation here...but it does go directly against the guidance.

For some Apps 404 is not an actual error.

Sure, apps are free to abuse HTTP in any number of nonstandard and unexpected ways. The RFC still says that it's an error. For some apps, 200 can be an error. Is that right? Nah.

Because semantically it's only says that the resource does not exist.

Yes, and the fact that you attempted to fetch a resource that does not exist is the error condition. Is it important? Probably depends on the resource and the app. It's still an error.

Anyway, I've said my piece and don't need to draw it out any longer. As long as this is off by default, I suppose it doesn't hurt that much....other than supporting/encouraging/enabling nonstandard use of http codes.