Span naming is problematic for REST-endpoints: Using the method is too generic

[pilhuhn]

Describe the bug

In 2.11.3 the REST-server spans were named after the rest-method called (e.g. /api/bla/...) while since 2.12.1 (at least) they are just "HTTP GET".
This is problematic from a user / admin point of view, as in Jaeger e.g. the search is for "operation is populated by span name
Also both in Jaeger and Tempo UI, the span name is what is shown in the list of traces. Meaning that now I have tons of "HTTP GET" operations and can't easily see and select for which endpoint
Apparently there is a http.target tag that can be used for filtering, but that is not auto-populated in the Jaeger UI.

#opentelemetry

Expected behavior

Span names should be the name of the HTTP endpoint.
If that can't be determined e.g. in the reactive case, it should at least be possible to use the well established behaviour for non-reactive endpoints. perhaps enabled via a flag.

Actual behavior

Span names are 'HTTP GET' or 'HTTP POST', which is much too generic

How to Reproduce?

1. Deploy a quarkus app with 2.11 with a rest-endpoint and opentelemetry enabled. Hit the endpoint and look at the created span
2. Bump Quarkus to 2.11 and hit the endpoint again -> behaviour has changed.

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[brunobat]

Not sure if this is a bug... We need to evaluate.

[quarkus-bot]

/cc @Ladicek, @radcortez

[pilhuhn]

I (as User) consider it a bug as it is a regression. If I had an ops team trained with the old behaviour, I could not upgrade Quarkus without re-training them. Potentially even rewriting standard operation procedures

[brunobat]

It's a bug.
According to the OTel Guidelines the current behaviour is wrong and it should be not too general, must be human readable and have low cardinality, like:

• get_account
• get_account/{accountId}
  https://opentelemetry.io/docs/reference/specification/trace/api/#span

HTTP_GET_account/{accountId} or
http_get_account/{accountId}

would be optimal.

[pilhuhn]

+1 on something like <method> path as in GET get_account/{accountId} . The HTTP is not necessarily needed, but the method is helpful, as the same REST-path can be served with multiple methods in parallel.

[radcortez]

In previous versions, we used the request URI as the span name, which is not recommended by the spec (due to high cardinality): https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#name

We are now using the route name if we can determine it. If we are getting a HTTP {METHOD_NAME}, we could not determine the route. I know there is one situation when this is not yet possible when @Path annotations are available in parent classes due to #12736. Do you know if this is the case? Can you provide us with a reproducer?

Regardless, I think we should not change the current name handling since this is delegated to the OTel implementation.

[brunobat]

This affects Metrics as well and will require in depth analysis, See Micrometer URI templating does not apply for unauthorized requests

[geoand]

Do we need to do anything about this ticket?

[radcortez]

We are following the OTel spec here. I suspect that the reason we are not getting a route name is that the endpoint is defined in a super class, and due to #12736, we are not able to retrieve the route information, so we go with the OTel default (which is OK, because OTel does not mandate to have a route name).

Since we never got a reproducer and we are confident that we are following the OTel spec, I think it is ok to close this issue.

[gsmet]

@pilhuhn if you can provide us a reproducer, please reopen the issue and ping me.

[brunobat]

I think we can do better here, if time allows it.
@pilhuhn a reproducer would help.