-
Notifications
You must be signed in to change notification settings - Fork 894
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
Define handling of null and empty attribute values #459
Merged
SergeyKanzhelev
merged 9 commits into
open-telemetry:master
from
dynatrace-oss-contrib:attribute-null-empty
Feb 28, 2020
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e719dbb
Define handling of null and empty attribute values
arminru 42600bd
undefined -> not set
arminru e1fd4a2
Apply suggestions from code review
arminru 4f3498c
Apply suggestions from code review
arminru b2c97e2
Allow exporters to replace null values in arrays
arminru d84a354
Apply suggestions from code review
arminru c4beea2
attribute semantics -> restrictions
arminru 82cbd17
emphasize MUST
arminru 24be6e5
Merge branch 'master' into attribute-null-empty
arminru File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -308,7 +308,8 @@ description](overview.md#links-between-spans). | |
A `Link` is defined by the following properties: | ||
|
||
- (Required) `SpanContext` of the `Span` to link to. | ||
- (Optional) One or more `Attribute`. | ||
- (Optional) One or more `Attribute`s with the same restrictions as defined for | ||
[Span Attributes](#set-attributes). | ||
|
||
The `Link` SHOULD be an immutable type. | ||
|
||
|
@@ -370,7 +371,7 @@ A `Span` MUST have the ability to set attributes associated with it. | |
|
||
An `Attribute` is defined by the following properties: | ||
|
||
- (Required) The attribute key, which must be a string. | ||
- (Required) The attribute key, which MUST be a non-`null` and non-empty string. | ||
- (Required) The attribute value, which is either: | ||
- A primitive type: string, boolean or numeric. | ||
- An array of primitive type values. The array MUST be homogeneous, | ||
|
@@ -386,6 +387,21 @@ Attributes SHOULD preserve the order in which they're set. Setting an attribute | |
with the same key as an existing attribute SHOULD overwrite the existing | ||
attribute's value. | ||
|
||
Attribute values expressing a numerical value of zero or an empty string are | ||
considered meaningful and MUST be stored and passed on to span processors / exporters. | ||
Attribute values of `null` are considered to be not set and get discarded as if | ||
that `SetAttribute` call had never been made. | ||
As an exception to this, if overwriting of values is supported, this results in | ||
clearing the previous value and dropping the attribute key from the set of attributes. | ||
|
||
`null` values within arrays MUST be preserved as-is (i.e., passed on to span | ||
processors / exporters as `null`). If exporters do not support exporting `null` | ||
values, they MAY replace those values by 0, `false`, or empty strings. | ||
carlosalberto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This is required for map/dictionary structures represented as two arrays with | ||
indices that are kept in sync (e.g., two attributes `header_keys` and `header_values`, | ||
both containing an array of strings to represent a mapping | ||
`header_keys[i] -> header_values[i]`). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
Note that the OpenTelemetry project documents certain ["standard | ||
attributes"](data-semantic-conventions.md) that have prescribed semantic meanings. | ||
|
||
|
@@ -397,7 +413,8 @@ with the moment when they are added to the `Span`. | |
An `Event` is defined by the following properties: | ||
|
||
- (Required) Name of the event. | ||
- (Optional) One or more `Attribute`. | ||
- (Optional) One or more `Attribute`s with the same restrictions as defined for | ||
[Span Attributes](#set-attributes). | ||
- (Optional) Timestamp for the event. | ||
|
||
The `Event` SHOULD be an immutable type. | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The paragraph above says that attribute overwriting SHOULD be supported, so I think there is no need for this conditional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically it is not but I still think this clarification might avoid some potential confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #503