Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP client spans: how to represent operation name/route and how to break things #923

Open
lmolkova opened this issue Apr 15, 2024 · 4 comments
Assignees
Labels
area:http enhancement New feature or request

Comments

@lmolkova
Copy link
Contributor

lmolkova commented Apr 15, 2024

Area(s)

area:http

Is your change request related to a problem? Please describe.

See the original discussion in #675 (comment)

Describe the solution you'd like

Option 1:

  • http.route
    • change span name (GET -> GET user/{id}) - breaking
    • add to metrics (opt-in), but ideally recommended - breaking

Option 2:

  • url.template - client route would not match server one, so it should probably be recorded with a different attribute
    • change span name (GET -> GET user/{id}) - breaking
    • add to metrics (opt-in), but ideally recommended - breaking

Option 3:

  • http.client.operation.name - not necessarily a route, can be OpenAPI operation name
    • change span name (GET -> getUserById) - breaking
    • add to metrics (opt-in), but ideally recommended - breaking

Ideally we want to emit the attribute on metrics by default since it would improve HTTP client metrics significantly in a small amount of cases when route is available.

@lmolkova lmolkova added enhancement New feature or request triage:needs-triage labels Apr 15, 2024
@lmolkova lmolkova changed the title HTTP client spans: how to represent operation name/route HTTP client spans: how to represent operation name/route and how to break things Apr 15, 2024
@lmolkova
Copy link
Contributor Author

Discussed at SemConv SIG:

  1. changing span name:
    • HTTP client span names are not so useful, adding route/name would be beneficial.
    • I will open PR to the spec to suggest removing span names from stability guarantees
    • For HTTP semconv, we want to rename span anyway.
  2. adding metric attribute: discuss next time

@gregkalapos
Copy link
Member

gregkalapos commented Apr 15, 2024

Do server and client HTTP span names have to be in the same format? Following up on the "HTTP client span names are not so useful" from the SemConv SIG, I think adding something like host to client side span name would make it more useful.

Currently #675 suggests:

HTTP server and client span names SHOULD be {method} {http.route} if there is a
(low-cardinality) http.route available (see below for the exact definition of the {method} placeholder).
If there is no (low-cardinality) http.route available, HTTP server and client span names
SHOULD be {method}.

This is fine, but I think in reality http.route will be very rarely available for client side spans (I'd argue most client side HTTP spans are generated by general HTTP client side libraries that'll only have pure URL, but not route). Therefore I'd suggest adding a fallback, something like:

HTTP server and client span names SHOULD be {method} {http.route} if there is a
(low-cardinality) http.route available (see below for the exact definition of the {method} placeholder).
If there is no (low-cardinality) http.route available, HTTP client span names
SHOULD be {method} {server.address}.

server.address is still low enough cardinality and if grouping happens on the span name in the backend then it'l naturally group based on method and target address, which to me would be the most natural way to group client side HTTP calls.

@lmolkova
Copy link
Contributor Author

Based on the offline discussion with @trask:

  • Java WebClient instrumentation has information about URL template. The same seems to be true for .NET HTTP Client URL template proposal
  • .*operation.name comes as a request method replacement (or a full span name replacement), while URL template should work in addition to request method (e.g. GET /user/{user_id} vs getUserById)
  • Operation name would be an application-level (or client library) detail, while URL template is something client instrumentation might have (or calculate).
  • To properly implement operation name, we'd need a mechanism similar to Propose Context-scoped attributes. oteps#207

So *.operation.name and url template seem like separate concepts that can coexist. We don't need operation name concept to resolve #675.

Between http.route and url.template: if we used http.route, it would create a false expectation that the value on the client matches the value on the server. It will rarely be a case (since route is web framework specific). Also routing is server-side concept.

As a result, from HTTP semconv perspective, we'd like to use url.template attribute.

@lmolkova
Copy link
Contributor Author

Breaking things was discussed at the 4/22 SemConv SIG:

  • we can proceed with opt-in attribute, span name, and new attribute added on metrics - it's not breaking.
  • we'd like to allow instrumentations to change span name and add attribute to metric with major version update and will explore how it can be done on the semconv level.
  • client libraries can do the breaking change with major version update. It applies to Java OTel agent (v3 is expected to include stable database/messaging and can include other minor breaking changes). .NET 9 is a new major version and can introduce breaking changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:http enhancement New feature or request
Projects
Development

No branches or pull requests

3 participants