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

Common event attribute names #397

Closed
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
42 changes: 42 additions & 0 deletions specification/data-events.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# 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

Each message sent/received within a span should be recorded as an event. In the
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm inclined to say that messages "MAY" be recorded as events. For a gRPC stream, in particular, I'm not sure that message events are always desired. I'd like the semantic convention to apply when the decision is made to record message events, but not to specify when you SHOULD record message events.

case of synchronous RPC calls there will be one sent and one received event per
Copy link
Member

Choose a reason for hiding this comment

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

should the recommended limit on number of messages associated with the span defined?

span. In case of a stream span such as a gRPC stream, WebSocket or HTTP
server-sent events, there could be multiple messages with each message recorded
as an individual event.

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`,
Copy link
Member

@Oberon00 Oberon00 Jan 17, 2020

Choose a reason for hiding this comment

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

What is the use-case for message.id? It seems rather complicated to implement. AFAIK, the list of events on a Span is already ordered.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be called a sequence number? It's not clear that message-ids must be sequential integers, nor who is responsible for calculating them. I'd be more comfortable calling them message-ids and not mentioning counters. Some implementations may use counters, some may not.

Copy link
Member

Choose a reason for hiding this comment

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

The "two different counters" wording is confusing to me. Above it says this is a single incrementing integer, but this implies that there are 2 integers? Does it mean that a message spend span has an incrementing id, and the message receive span has its own separate incrementing id? In this case, if a parent span sends 3 messages that get ids 1, 2, and 3, and they arrive out of order, one gets dropped, or arrive in separate invocations of the receiver, could they have different ids on the receiving side than they did on the sending side?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added more clarifying verbiage.

the first for sent messages and the second for received messages. This way we
guarantee that the values will be consistent between different implementations.
In protocols where the message id is included as a header in the message, the
received message id should be that sent be the server instead of its own
incremented value.

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

@Oberon00 Oberon00 Jan 17, 2020

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
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.