-
Notifications
You must be signed in to change notification settings - Fork 161
Client gen template uses incorrect OTel semconv attr http.route #1540
Copy link
Copy link
Closed
Labels
Description
ogen/gen/_template/client.tmpl
Line 291 in f7043ba
| semconv.HTTPRouteKey.String({{ quote $op.Spec.Path }}), |
Both the generated server and client code use the http.route attribute. However, the generated client code incorrectly uses the OTel attribute http.route. This attribute is meant to be used only in server-side metrics and spans. The corresponding client attribute should be used instead, which is url.template.
So, the referenced line of code should instead be:
- semconv.HTTPRouteKey.String({{ quote $op.Spec.Path }}),
+ semconv.URLTemplateKey.String({{ quote $op.Spec.Path }}),See the following:
- https://opentelemetry.io/docs/specs/semconv/registry/attributes/http/#http-route
- https://opentelemetry.io/docs/specs/semconv/registry/attributes/url/#url-template
- The semconv package documentation: https://pkg.go.dev/go.opentelemetry.io/[email protected]/semconv/v1.37.0#URLTemplate
- This is the PR that added support for
url.templateto client attributes. In the PR, the proposal was originally to name ithttp.route, but after discussion, it was decided to name the attributeurl.template: Added url.template to HTTP client attributes open-telemetry/semantic-conventions#675
Note: it looks like the url.template attribute was added after the client code referenced was written. So, while it may have potentially made sense to use http.route at the time, we should update it to reflect the current OpenTelemetry semantic conventions.
Reactions are currently unavailable