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 3 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
41 changes: 41 additions & 0 deletions specification/data-events.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# 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. |
| `"error"` | Event with the details of one captured error, fault or exception. |

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


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.

## Error event attributes
Copy link
Member

Choose a reason for hiding this comment

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

Can we split this PR into two (message and errors). I think we should consider if for errors we want to have a more specific helper API than just attributes, also the correlation with the Status.

So to make progress we can probably soon merge the message part and keep discussing the errors.

Copy link
Member

Choose a reason for hiding this comment

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

+1 on splitting it up :)

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 will take care of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


Event `"name"` MUST be `"error"`.

| Attribute name | Notes and examples | Required? |
| :------------- | :------------------------------------- | --------- |
| `error.kind` | The type or "kind" of an error. E.g., `"Exception"`, `"OSError"` | Yes |
Copy link
Contributor

Choose a reason for hiding this comment

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

In some places we use kind and in other places we use type to describe the same thing. So now we have message.type and error.kind, along with span.kind and db.type. We should standardize on a single convention for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW in the Metrics API spec I've avoided using "type" except when it refers to an actual programming language type, not a more abstract term. So, Instruments have kinds. The values they produce have types.

| `error.message` | A concise, human-readable, one-line message explaining the event. E.g., `"Could not connect to backend"`, `"Cache invalidation succeeded"` | Yes |
Copy link
Contributor

Choose a reason for hiding this comment

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

There's going to be a desire to record information events that are not errors. It would be nice to have a "message" attribute and a reserved event name for those.

| `error.object` | For languages that support such a thing (e.g., Java, Python), the actual Throwable/Exception/Error object instance itself. E.g., A `java.lang.UnsupportedOperationException` instance, a python `exceptions.NameError` instance | No |
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest error.type here. It's not clear how to set error.kind when you have a definite type name, though. Do we need both?

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 am fine with error.type. These came from the OpenTracing spec.

| `error.stack` | A stack trace in platform-conventional format; may or may not pertain to an error. E.g., `"File \"example.py\", line 7, in \<module\>\ncaller()\nFile \"example.py\", line 5, in caller\ncallee()\nFile \"example.py\", line 2, in callee\nraise Exception(\"Yikes\")\n"` | No |
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you allow this to include the caused-by fragments of a Java exception?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you ever want to record the location where the error is being recorded as distinct from the location where an exception occurred? I think of these as the location of the log statement vs the location of the throw statement, for example.

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 have submitted an OTEP open-telemetry/oteps#69 addressing what the value of error.object should be. It would include all of the data items mentioned above.

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.