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

Common event attribute names #397

Closed
wants to merge 16 commits into from
Closed

Common event attribute names #397

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
d7834fe
initial def of event attribute names
kbrockhoff Dec 17, 2019
c77a664
Revert "initial def of event attribute names"
kbrockhoff Dec 17, 2019
89729ad
initial def of event attribute names
kbrockhoff Dec 17, 2019
ccfcd27
Merge remote-tracking branch 'upstream/master'
kbrockhoff Dec 26, 2019
27e93e8
Merge remote-tracking branch 'upstream/master' into event-sem-convent…
kbrockhoff Dec 26, 2019
f177ad2
Merge remote-tracking branch 'upstream/master'
kbrockhoff Jan 7, 2020
94cfd25
Merge remote-tracking branch 'upstream/master'
kbrockhoff Jan 15, 2020
86a1479
Merge remote-tracking branch 'upstream/master'
kbrockhoff Jan 17, 2020
824d777
Merge branch 'master' into event-sem-conventions-#362
kbrockhoff Jan 17, 2020
c54869d
remove error event info to separate PR
kbrockhoff Jan 17, 2020
c3aeebf
improve wording
kbrockhoff Jan 17, 2020
df0d8ba
add more clarifying verbage
kbrockhoff Jan 18, 2020
02e7bd0
Merge remote-tracking branch 'upstream/master'
kbrockhoff Jan 22, 2020
a42d217
Merge remote-tracking branch 'upstream/master'
kbrockhoff Jan 23, 2020
148d2fd
Merge branch 'master' into event-sem-conventions-#362
kbrockhoff Jan 23, 2020
a5c59ac
Improve and expand on the definition of message.id
kbrockhoff Jan 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
35 changes: 35 additions & 0 deletions specification/data-events.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Semantic conventions for events

Event types are identified by the event name. Library and application implementors
are free to define any event name that make sense with the exception of the
following reserved names.

## Reserved event names

| Event name | Notes and examples |
| :---------- | :----------------------------------------------------------------- |
| `"message"` | Event with the details of a single message within a span. |

## Message event attributes

Event `"name"` MUST be `"message"`.
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 missing a description of what a message event is even supposed to describe, i.e. when should I add message events to my span.

Copy link
Member

Choose a reason for hiding this comment

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

@arminru 29 days ago:

I don't quite understand what the proposed message event is about. When would I use this one and when would I create a (child) span with the attributes defined in #395?

@kbrockhoff please help :-)


Message events MUST be associated with a tracing span.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel there needs to be some guidance as to when we add these attributes on an event vs on a span. It's fine to say that they should always be an event, even when the span only have one "event" such as an http request.

Copy link
Member

Choose a reason for hiding this comment

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

Yes we should always use them as event attributes even though we know only one event can occur per Span.


| Attribute name | Notes and examples | Required? |
| :------------- | :------------------------------------- | --------- |
| `message.type` | Either `"SENT"` or `"RECEIVED"`. | Yes |
Copy link
Member

Choose a reason for hiding this comment

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

should it be boolean or more types are expected?

| `message.id` | Incremented integer value within the parent span starting with `1`. | Yes |
Copy link
Member

Choose a reason for hiding this comment

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

This describes the format, but not the semantic meaning at all. "An incremented integer value" doesn't describe what the attribute is actually telling me.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added paragraph on meaning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: maybe just specify these as opaque identifiers?

| `message.compressed_size` | Compressed size in bytes. | No |
Copy link
Member

Choose a reason for hiding this comment

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

is it always known that the message was compressed? Perhaps size will be size of a message, and than two separate sizes for compressed and uncompressed be specified.

| `message.uncompressed_size` | Uncompressed size in bytes. | No |
Copy link
Member

Choose a reason for hiding this comment

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

just as an idea, maybe we should think about a bit shorter names :) if you have any suggestion I would be happy to hear :)

Copy link
Member

Choose a reason for hiding this comment

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

You don't really need the word "uncompressed" in my opinion.

  • message.size - uncompressed size
  • message.compressed or message.compressed_size - compressed size

| `message.content` | The body or main contents of the message. If binary, then message should be Base64 encoded. | No |
Copy link
Member

Choose a reason for hiding this comment

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

should there be an attribute for the metadata/headers? May be more valuable than the content of the message and cheaper to collect.


The `message.id` MUST be calculated as two different counters starting from `1`
one for sent messages and one for received message. This way we guarantee that
Copy link
Member

@dyladan dyladan Jan 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Three different uses of one/1 in this sentence, which refer to message id and types of message ids are confusing. I would go with something along the lines of:

The message.id MUST be calculated as two different counters starting from 1, the first for sent messages and the second for received messages.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

the values will be consistent between different implementations. In case of
unary calls only one sent and one received message will be recorded for both
client and server spans.

Most exporters will likely drop the `message.content` attribute if present.
Copy link
Member

@Oberon00 Oberon00 Jan 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Most exporters will likely not drop message.content because they don't know they should 😃 I'm against introducing such "drop by default" attributes without a corresponding API/general convention to mark them as such. Though I certainly find the idea of adding such an API intriguing.

Copy link
Member

Choose a reason for hiding this comment

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

I could see 3 levels of attribute:

  • required
  • recommended
  • enhanced

The default tracer would keep recommended and required, a low-bandwidth tracer could drop all except required, and an enhanced attributes tracer could keep all attributes.

Copy link
Member

Choose a reason for hiding this comment

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

Additionally there would need to be some database that the exporter can consult to determine in which category an attribute falls.

Copy link
Member

Choose a reason for hiding this comment

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

And a way (another use for named tracer?) to tell the tracer which "mode" to operate in.

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 the use case of the logging exporter shows that this is a configuration you want to do on the exporter level (logging exporter: all, Jaeger exporter: normal, for example).

Copy link
Contributor

Choose a reason for hiding this comment

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

Tangentially, it makes me wonder why we don't have an API to declare keys with metadata like a description, some kind of priority as discussed here, potentially type information, as well as a clearly stated namespace.

However, logging-only exporters will likely want to log it as this information
is highly useful during the early stages of developing a new application.
1 change: 1 addition & 0 deletions specification/data-semantic-conventions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ The following semantic conventions for spans are defined:
* [Database](data-database.md): Spans for SQL and NoSQL client calls.
* [RPC/RMI](data-rpc.md): Spans for remote procedure calls (e.g., gRPC).
* [General](data-span-general.md): General semantic attributes that may be used in describing different kinds of operations.
* [Event](data-events.md): Attributes for events.