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

Are empty string values equivalent to unset attributes? #431

Closed
Oberon00 opened this issue Jan 29, 2020 · 4 comments · Fixed by #459
Closed

Are empty string values equivalent to unset attributes? #431

Oberon00 opened this issue Jan 29, 2020 · 4 comments · Fixed by #459
Assignees
Milestone

Comments

@Oberon00
Copy link
Member

In open-telemetry/opentelemetry-java#771, opentelemetry-java decided to silently ignore calls to Span.setAttribute with empty string values (see also the brief discussion at open-telemetry/opentelemetry-java#765 (comment)).

I wonder what the specification position on this is. The semantic conventions for http.host say

When the header is empty or not present, this attribute should be the same.

-- https://github.com/open-telemetry/opentelemetry-specification/blob/cf0bb600b2eef359dbdd8fbe6f4696ed3e67ed1d/specification/data-http.md#common-attributes

(which is maybe a bit of an unfortunate formulation but is meant to say: "When the header is empty, this attribute should be empty. When the header is not present, this attribute should not be present.")

If we consider an empty string the same as not set, why don't we do the same for numeric values of zero or boolean values of false? 😉

I think we should add a note that explicitly states what to do with empty strings in SetAttribute (probably the same applies to metric labels and resource key values).

My personal opinion is that empty strings should be distinct from an unset attributes.

@Oberon00 Oberon00 changed the title Should empty strings be equivalent to unset attributes? Are empty string values equivalent to unset attributes? Jan 29, 2020
@carlosalberto carlosalberto added this to the Alpha v0.4 milestone Jan 30, 2020
@Oberon00
Copy link
Member Author

Oberon00 commented Jan 31, 2020

Depending on the outcome, we may need to separately discuss this for:

  • null arrays
  • null or empty strings inside arrays

See also open-telemetry/opentelemetry-java#807 (comment).

@jmacd
Copy link
Contributor

jmacd commented Feb 3, 2020

This has been discussed in the context of metrics as well, with no great resolution. At present, the protocol spec says that empty values are considered the same as unspecified. I have a preference for treating them distinctly, but it implies a lot of changes in a lot of places and there wasn't a great deal of enthusiasm when this was discussed.

#345

@jmacd
Copy link
Contributor

jmacd commented Feb 4, 2020

#341

@jmacd
Copy link
Contributor

jmacd commented Feb 4, 2020

#345

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 a pull request may close this issue.

4 participants