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

Define handling of null and empty attribute values #459

Conversation

arminru
Copy link
Member

@arminru arminru commented Feb 13, 2020

Resolves #431.

A follow-up PR might reference this definition for resource attributes as well to address #446.

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

thx for this!

Copy link
Member

@krnowak krnowak left a comment

Choose a reason for hiding this comment

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

I wonder if metrics also need this kind of clarification.

CC @jmacd

Related: open-telemetry/opentelemetry-go#320

specification/api-tracing.md Outdated Show resolved Hide resolved
@arminru arminru force-pushed the attribute-null-empty branch from f625245 to c4beea2 Compare February 17, 2020 15:59
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
Copy link
Member

Choose a reason for hiding this comment

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

if overwriting of values is supported...

The paragraph above says that attribute overwriting SHOULD be supported, so I think there is no need for this conditional.

Copy link
Member Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

See #503

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]`).
Copy link
Member

Choose a reason for hiding this comment

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

👍

@carlosalberto carlosalberto self-requested a review February 26, 2020 16:19
@SergeyKanzhelev
Copy link
Member

last sign off was from two days back. And we have enough sign offs. @bogdandrutu I remember you mentioned you want to use null value as a notion of an attribute removal. I would suggest we have a RemoveAttribute method if it's needed later. Please raise an issue if this is a concern. Merging for now...

@andrewhsu
Copy link
Member

Looks like this could be added to the v0.4 milestone because this was merged after v0.3 was cut.

@arminru arminru added this to the v0.4 milestone Apr 28, 2020
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
* Define handling of null and empty attribute values

* undefined -> not set

* Apply suggestions from code review

Co-Authored-By: Wolfgang Ziegler <[email protected]>

* Apply suggestions from code review

Co-Authored-By: Christian Neumüller <[email protected]>

* Allow exporters to replace null values in arrays

* Apply suggestions from code review

Co-Authored-By: Christian Neumüller <[email protected]>

* attribute semantics -> restrictions

* emphasize MUST

Co-authored-by: Wolfgang Ziegler <[email protected]>
Co-authored-by: Christian Neumüller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Are empty string values equivalent to unset attributes?
9 participants