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.

Sign up for GitHub

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

Clarify span name criteria (resolves #557). #810

Merged
merged 2 commits into from
Aug 19, 2020
Merged

Clarify span name criteria (resolves #557). #810

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a96c0ae
Clarify span name criteria.
Oberon00 Aug 14, 2020
f7e714d
Clarify that human-readility is still something we want.
Oberon00 Aug 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions specification/trace/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,16 @@ the entire operation and, optionally, one or more sub-spans for its sub-operatio
- A list of timestamped [`Event`s](#add-events)
- A [`Status`](#set-status).

The _span name_ is a human-readable string which concisely identifies the work
represented by the Span, for example, an RPC method name, a function name,
or the name of a subtask or stage within a larger computation. The span name
should be the most general string that identifies a (statistically) interesting
_class of Spans_, rather than individual Span instances. That is, "get_user" is
a reasonable name, while "get_user/314159", where "314159" is a user ID, is not
a good name due to its high cardinality.
The _span name_ concisely identifies the work represented by the Span,
for example, an RPC method name, a function name,
or the name of a subtask or stage within a larger computation.
The span name SHOULD be the most general string that identifies a
(statistically) interesting _class of Spans_,
rather than individual Span instances while still being human-readable.
That is, "get_user" is a reasonable name, while "get_user/314159",
Copy link
Contributor

@jmacd jmacd Aug 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a contradiction here. get_user/314159 may be a statistically interesting class of spans (the requirement above), but high-cardinality is the rationale given below.

When I first read the (statistically) above, my eyes substituted the word statically, which may be a better way to write the specification. We reject get_user/314159 because it is implied to be a variable. My favorite way to address this is the way the Metrics API does: by requiring an "instrument".

Discussed here #734 (comment).

Copy link
Member Author

@Oberon00 Oberon00 Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a contradiction. It says "the most general string". While "get_user/314159 may be a statistically interesting class of spans" it is certainly less general than get_user, which still an interesting class of of spans.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point. Maybe instead of "high-cardinality" you could write "too specific", i.e., something that that's opposed to the "general" quality.

where "314159" is a user ID, is not a good name due to its high cardinality.
Generality SHOULD be prioritized over human-readability.


For example, here are potential span names for an endpoint that gets a
hypothetical account information:
Expand Down