Remove HTTP method from span name

[anuraaga]

OTel semantic conventions dictate the format {method} {route} for server span names

https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#name

In the Trace UI, I see the HTTP method displayed under Method, as well as {method} {route} under URL. I suspect this is because cloud trace expects the span name to be only the URL, not method and URL. Does it make sense to check if there is an http.method attribute present and trim off {method} prefix from the name where it is? If there is no route available, it will just be {method}, so to avoid an empty span name it should only happen when there is {method} stuff

[dashpole]

A screenshot might help, if you are able to share. My initial reaction is that I would think {method} {route} appearing in the waterfall view would be helpful... But I can see how URL being something other than the URL would be confusing. I generally lean towards not modifying names, etc. from OTel unless we have a really compelling reason.

I think we might not be mapping labels correctly. Here are the "special" labels in cloud trace: https://cloud.google.com/trace/docs/trace-labels. That list includes http/url, but I don't see that in our mappings: https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/blob/main/exporter/trace/trace_proto.go#L59.

[anuraaga]

The list view looks something like this

The fact that GET shows up twice looks a bit buggy. But indeed, changing the span name may not be best after all since it would affect the waterfall view.

I realized I accidentally opened the issue in the Go repo while looking at a JS app, though the span name itself is the same in both. But JS does seem to map more

https://github.com/GoogleCloudPlatform/opentelemetry-operations-js/blob/main/packages/opentelemetry-cloud-trace-exporter/src/transform.ts#L277

Notably, my span does have/http/url, an absolute http://api.example.com/v1/logging/log and /http/route (!) /v1/logging/log. Ideally the list view used route for displaying in URL column but it seems to always be span name? It seems it would be nice to have the attributes affect it, may need to check with the backend team on what's going on.

[dashpole]

I reached out to the appropriate team. I'll let you know if I get any updates.

[dashpole]

Response: The URL field in that table is actually mis-labeled. It refers to the name of the root span for that trace. The "URL" was originally designed to work well with Google App Engine, in which the name of the root span is the URL being invoked.

I also realized while looking into it that the table is a table of traces, not a table of spans, so there could be cases where a trace has many spans with an http.url attribute. Still... It would be nice if it wasn't misleading

[anuraaga]

Thanks for confirming - if the intent was to display a URL, then perhaps the logic could be updated to refer to an attribute when available. In any case it seems to clearly not be well aligned with OTel semantic conventions right now since there is always stuttering between the http method attribute and span name.

so there could be cases where a trace has many spans with an http.url attribute.

Indeed this causes some issues. We have one frontend using go with gin and I'll check how it looks, but our main frontend is actually NestJS. I have this gross hack to make that list view even somewhat useful, otherwise it was all just POST POST.

One thing I noticed since I experimented with cloud propagator with an http load balancer, the list reflects the root span if present, or otherwise it seems to be an arbitrary span from the trace if missing (load balancer doesn't export so root span is always missing when adding that propagator, I thought this could still be ok but not with the randomness introduced to the list view).

 import {
   ReadableSpan,
   Span,
   SpanProcessor,
 } from '@opentelemetry/sdk-trace-base';
 import { Context, SpanKind, trace } from '@opentelemetry/api';
 
 interface ParentSpan extends Span {
   __parent_span: Span;
 }
 
 export class UpdateServerSpan implements SpanProcessor {
   constructor(readonly delegate: SpanProcessor) {}
 
   onStart(span: Span, parentContext: Context): void {
     // There is no way "partial" sdk usage meaning we know every API span is a SDK span.
     // While somewhat hacky, it's reasonable to cast to get access to the parent's data.
     const parent = trace.getActiveSpan() as Span;
     if (parent && parent.kind === SpanKind.SERVER) {
       // Workaround https://github.com/open-telemetry/opentelemetry-js/issues/4188 by updating
       // within onEnd instead of here. The specification requires the span instance to be the
       // same, so while somewhat hacky, this should still be fine.
       (span as ParentSpan).__parent_span = parent;
     }
 
     this.delegate.onStart(span, parentContext);
   }
 
   onEnd(span: ReadableSpan): void {
     const parent = (span as ParentSpan).__parent_span;
     const route = span.attributes['http.route'];
     if (parent && !parent.attributes['http.route'] && route) {
       parent.setAttribute('http.route', route);
       const method = parent.attributes['http.method'];
       // Should always be true in practice.
       if (method) {
         parent.updateName(`${method} ${route}`);
       }
     }
 
     this.delegate.onEnd(span);
   }
 
   shutdown(): Promise<void> {
     return Promise.resolve(undefined);
   }
 
   forceFlush(): Promise<void> {
     return Promise.resolve(undefined);
   }
 }