-
Notifications
You must be signed in to change notification settings - Fork 896
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
Provide guidelines for low-cardinality span names #416
Changes from all commits
a7fbb62
027abdd
392e363
51f5746
5c0d398
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,10 +21,19 @@ and various HTTP versions like 1.1, 2 and SPDY. | |
|
||
## Name | ||
|
||
Given an [RFC 3986](https://tools.ietf.org/html/rfc3986) compliant URI of the form `scheme:[//host[:port]]path[?query][#fragment]`, | ||
the span name of the span SHOULD be set to the URI path value, | ||
unless another value that represents the identity of the request and has a lower cardinality can be identified | ||
(e.g. the route for server spans; see below). | ||
HTTP spans MUST follow the overall [guidelines for span names](./api-tracing.md#span). | ||
Many REST APIs encode parameters into URI path, e.g. `/api/users/123` where `123` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make a more specific recommendation. E.g. using one of the formats "HTTP {METHOD-NAME}" or "{METHOD-NAME} {HOST-NAME}" sounds reasonable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Method is already suggested in the following sentences, I can change to use the exact pattern. Host name is not acceptable. If clients are directly integrated with service discovery, then host name may be very high cardinality. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, but I would make it more clear that this not merely a suggestion but actually what should be used (in absence of a better option like endpoint name, route, ...). And I would definitely prescribe an exact (fallback) format, because that would allow grouping HTTP requests from different languages together on the back end. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added exact recommendation in L34, but I don't know how strongly prescriptive we want to be. |
||
is a user id, which creates high cardinality value space not suitable for span | ||
names. In case of HTTP servers, these endpoints are often mapped by the server | ||
frameworks to more concise _HTTP routes_, e.g. `/api/users/{user_id}`, which are | ||
recommended as the low cardinality span names. However, the same approach usually | ||
does not work for HTTP client spans, especially when instrumentation is provided | ||
by a lower-level middleware that is not aware of the specifics of how the URIs | ||
are formed. Therefore, HTTP client spans SHOULD be using conservative, low | ||
cardinality names formed from the available parameters of an HTTP request, | ||
such as `"HTTP {METHOD_NAME}"`. 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. | ||
|
||
## Status | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you do in the case that you have the url, but don't know which part of it is an id or dynamic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then you can't use it. It's covered in the 2nd file.