TCK RestClientSpanTest Span Name Doesn't Follow Semantic Conv

[arieki]

Description

Based on OpenTelemetry Semantic conventions for HTTP spans "Instrumentation MUST NOT default to using URI path as span name, but MAY provide hooks to allow custom logic to override the default span name."
Meanwhile, I found in the TCK use URI path as the span name Assert.assertEquals(server.getName(), url.getPath() + "span");

As the guideline span name described "an RPC method name, a function name, or the name of a subtask or stage within a larger computation" is a good example

[pdudits]

To expand what @arieki is referring to:
Assertions for server span names in the TCK are incorrect, or go beyond specification.

MP Tracing spec says:

These should follow the rules specified in the Semantic Conventions section. https://github.com/eclipse/microprofile-telemetry/blob/main/tracing/spec/src/main/asciidoc/microprofile-telemetry-tracing-spec.asciidoc?plain=1#L87

The word "should" indicate optional requirement. If we take it one step further, Semantic conventions for HTTP spans also use word "should" to describe the requirement.

If the spec would say, that compatible implementation MUST follow recommendations given for http span names set forth by conventions, then checking for those span names would be reasonable.

However, in that case, the span names that are validated now are not the ones set forth by convention:

HTTP server span names SHOULD be {http.method} {http.route} if there is a (low-cardinality) http.route available. HTTP server span names SHOULD be {http.method} if there is no (low-cardinality) http.route available. HTTP client spans have no http.route attribute since client-side instrumentation is not generally aware of the “route”, and therefore HTTP client spans SHOULD use {http.method}

Test expect client spans to be called HTTP GET, whereas convention recommends GET.
Server spans should be called GET {route} and TCK tests for {route}.

[Azquelt]

This issue lists several problems that I'd like to address individually:

the span names that are validated now are not the ones set forth by convention [...] HTTP server span names SHOULD be {http.method} {http.route}

This is true for the current version of the OpenTelemetry specification, but MicroProfile Telemetry 1.0 is based on v1.13.0 of OpenTelemetry and this section of the spec has changed since then. The relevant part in 1.13 doesn't suggest including the HTTP method in the name.

[The spec says] "Instrumentation MUST NOT default to using URI path as span name" [...] Meanwhile, I found in the TCK use URI path as the span name Assert.assertEquals(server.getName(), url.getPath() + "span");

The section of the spec talks about using the full URI path vs. using a more concise HTTP route. The example given is /api/users/123 vs. /api/users/{user_id}. The spec does say you must not default to using the full URI path.

However, in the TCK test you quote the URI path has no placeholders, so the URI path and the HTTP route are the same. The TCK is expecting the HTTP route, and in this case it just happens to be the same as the full URI path.

In a later test a URI with a placeholder is used and you can see the test expects the placeholder to be used in the span name, rather than using the full URI path for the request.

Assert.assertEquals(server.getName(), (url.getPath() + "span/{name}"));

The word "should" indicate optional requirement. If we take it one step further, Semantic conventions for HTTP spans also use word "should" to describe the requirement.

I think you are correct here. The spec suggests but doesn't mandate that a particular name is used so the TCK should not test it.

I think the existing asserts for span names should be removed, though we could check to ensure that the name does not include the full URI path for the cases where the HTTP route includes placeholders.

[Emily-Jiang]

@pdudits will take a look at this.

[Emily-Jiang]

@pdudits will create a PR to cover this.