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

And semantic conventions for Display Hints #730

Closed
wants to merge 6 commits into from
Closed

And semantic conventions for Display Hints #730

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
83e8b2a
Add display hints for name, field, and type
Jul 23, 2020
7a9ff22
whitespace
Jul 23, 2020
f8c8323
change field to attribute for clarity
Jul 23, 2020
a838b48
Clarify how to handle unset attributes
Jul 23, 2020
8ba3817
formatting
Jul 23, 2020
1bf5f4e
Add example
Jul 23, 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
49 changes: 48 additions & 1 deletion specification/trace/semantic_conventions/span-general.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Particular operations may refer to or require some of these attributes.
<!-- Re-generate TOC with `markdown-toc --no-first-h1 -i` -->

<!-- toc -->

- [Display hints for spans](#display-hints)
- [General network connection attributes](#general-network-connection-attributes)
* [`net.transport` attribute](#nettransport-attribute)
* [`net.*.name` attributes](#netname-attributes)
Expand All @@ -16,6 +16,53 @@ Particular operations may refer to or require some of these attributes.

<!-- tocstop -->

## Display hints

UIs are expected to utilize all available semantic conventions when choosing how to
Copy link
Member

Choose a reason for hiding this comment

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

Could you specify what kind of UI is meant here? "Tracing backends" or the like would make it more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will do

display a span, and to make their own decisions about what information is most important.
However, there are cases when display hints can be helpful, especially when the spans
represent a non-standard operation or have a span name which is useful for grouping but
poor for human readability.

| Attribute name | Notes and examples |
| :----------------- | :------------------------------------------------------------------ |
|`display.name` | a custom span name to display instead of the span name |
|`display.attribute` | the key to the attribute which should be used as the display name |
|`display.type` | clarifies the type of operation being performed |


All display hints are optional. These display hints are not required to be set by
instrumentation, and it is not required that UIs respect their values, even when they
are set.

The value of `display.attribute` MUST match another attribute key, with the expectation that
the value of that attribute will be used as the span name in the display. If both
`display.name` and `display.attribute` are set, `display.name` should take precedence. If
the value of the attribute `display.attribute` points to is unset or empty, this field
should be ignored.

The `display.type` field may match an appropriate namespace (db, http, faas, etc) or be
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how this is specific to display

completely custom.

### Example
Imagine instrumenting a simple http client, HipClient, which does not have a concept
of routes or handlers. In this case, the span name would default to a low cardinaility grouping
such as "HTTP {METHOD_NAME}". This may be useful for grouping, but it is not nearly as helpful as
the url, or potentially method name plus the url. It may also be helpful to display to the user
that the type of http request was a HipClient request.

In this example, the low cardinality span name could be replaced with something more readable:

* **Span Name**: "HTTP GET"
* **display.name**: "GET /account/123"
Copy link
Member

Choose a reason for hiding this comment

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

This is a rather contrived example. The backends already know when the span is for HTTP from other semantic attributes, and can easily change the display name to {method} {url}. What other examples do you have that do not fall into the same pattern (e.g. the same approach can be used for DB spans, etc.).

Copy link
Contributor Author

@tedsuo tedsuo Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, in some of these cases it is possible for a backend to construct a better display name, but will backends actually choose to override the span name by default in these cases? Adding a display hint when the span name is already known to be bad for human comprehension would make life easier for everyone trying to consume our data.

To give other examples, there are two use cases where a backend cannot derive the appropriate name:

  • Custom spans, where the backend has no way of knowing the pattern.
  • A pattern provided by the user, for their own clarity. For example, by adding hints in a collector pipeline.

Copy link
Member

Choose a reason for hiding this comment

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

I think examples in the specification should showcase real scenarios. An example with HTTP span just doesn't make any sense to me, because the new attributes do not provide anything that's not already available to the backend if the span follows the HTTP semantic conventions. Using it as example in the spec is an example of what not to do, since it just wastes bandwidth. Not to mention that HTTP instrumentation is usually done by reusable middleware, not by the end user code.

I am not opposed to the display.name attribute, but let's show an example where this attribute enables something that's difficult or even impossible otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

Another aspect to think about is that introducing this semantic convention explicitly encourages a pattern of providing data that is clearly static (esp. if using display.template) and associated with the span name. We never talked about it before, but a lot of span attributes are static for a given span, but the trace API provides no facilities to optimize for that (similar to bound instruments in the metrics API). Booked #734.

* **display.type**: "HipClient"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is a clear example of an "operation being performed". Perhaps some other examples would help? What about a server request, message queue, or internal span?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to add more examples. Is there anything in particular that makes this confusing?


Alternatively, if the `http.url` attribute has been set on the span, it could be referenced
for the display name:
* **Span Name**: "HTTP GET"
* **display.attribute**: "http.url"
* **display.type**: "HipClient"

## General network connection attributes

These attributes may be used for any network related operation.
Expand Down