This class didn't feel worth it (open-telemetry/opentelemetry-java#6970)

I'm wondering about the usefulness of capturing db.response.status_code for non-errors.

Semantic conventions say db.response.status_code is conditionally required:

If the operation failed and status code is available.

so it may be ok if we don't capture it for non-errors.

But if we only capture it for errors, then I'm wondering how it's useful since those error codes will be captured (also) in error.type.

cc @lmolkova

For elastic and opensearch that use http client underneath the current behavior would is similar to what http client instrumentations use. For other database clients I'd guess that even now status code and error type are going to match when status code is present and there is no status code for successful operations (do we have a database client that reports status code for success?).

Interesting. I don't remember us discussing this during stabilization so not sure if there are good reasons for it.

do we have a database client that reports status code for success?

We have at least Postgres, MS SQL, mariaDB, cosmosdb that do have non-error codes.

I think we use both because it's consistent with HTTP client - most of the time error code is the same as http status code and only when there is no response at all. We probably did it in HTTP because status code is a common grouping key on dashboards and it's not always related to errors/success.
It's also part of error.type definition

If a specific domain defines its own set of error identifiers (such as HTTP or RPC status codes), it's RECOMMENDED to:

• Use a domain-specific attribute
• Set error.type to capture all errors

If we made status code opt-in by default:

• we'd let individual systems override it and provide status code attribute by default in success cases
• we wouldn't provide status code in case of failure by default
• we'd ask uses to build dashboards using error.type (which we already do).

I think it makes more sense than what we have today, but I don't like the idea of a breaking change in DB semconv.
We should fix it for unstable DBs and consider for RPC/future.

created open-telemetry/semantic-conventions#3478

Would you folks feel bad for not following this conditionally_required in instrumentations? It makes sense from practical standpoint and there is nobody that would be broken, right?

Would you folks feel bad for not following this conditionally_required in instrumentations?

nope, I won't feel bad at all 😄

Previously we had this in the DbResponseStatusUtil class. Since multiple instrumentations need to decide which http statuses are errors perhaps it would make sense to share this code?

see #16276 (comment)

I realize we may decide never to remove all shared .internal code, but this one is just 3 LOC used in only Elasticsearch and OpenSearch instrumentations (which are sort of effectively the same instrumentation).

I personally think that removing shared internal code isn't too useful. Using unaligned artifacts can lead to failures even without the shared internal code. Another alternative would be to move https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpStatusCodeConverter.java to internal and make it public so that HttpStatusCodeConverter.CLIENT.isError could be used.