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

Clarify the need to have "unspecified" values of metric labels #345

Closed
tigrannajaryan opened this issue Nov 6, 2019 · 7 comments
Closed
Labels
spec:metrics Related to the specification/metrics directory

Comments

@tigrannajaryan
Copy link
Member

The spec currently says:

When the SDK interprets a LabelSet in the context of grouping aggregated values for an exporter, and where there are keys that are missing, the SDK is required to consider these values explicitly unspecified, a distinct value type of the exported data model.

The unspecified values are specifically called out and need to be supported explicitly in export data formats, which raised a debate here.

We need to clarify the need for "unspecified" values and if they are not necessary remove them from the spec.

@jmacd
Copy link
Contributor

jmacd commented Mar 4, 2020

The current metrics specification says nothing of "unspecified" values. While we haven't made a positive statement to this effect in the specification, I believe we have converged on a desire to treat empty values and unspecified values as the same, for metrics.

See this comment. open-telemetry/opentelemetry-go#320 (comment)

@jmacd
Copy link
Contributor

jmacd commented Mar 4, 2020

The current specification for span attributes states this:

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.

On the surface it appears that we've specified different behaviors for empty-string values in tracing and metrics. However, generally as we export aggregated metrics there is a set of "grouping" dimensions for which values must be supplied. There is a common practice (e.g., in Prometheus) to treat unspecified values as empty strings.

Although this applies to the case where a grouping value must be supplied, this motivates the general treatment of empty-strings as unspecified, since they create the same exported value in industry-standard exporters.

I will propose that we change the behavior of empty-string values for span attributes (and resources, and span links, etc.) to be equivalent to an unspecified value.

As a language-level supporting point, I'll note that in Go does not consider nil a valid string value and there is no Value constructor for nil, so it is impossible to have a nil-valued label in the Go API. Because we desire to treat unspecified values as empty strings in common exporters, therefore, it makes sense to treat empty-string values as unspecified.

@jmacd
Copy link
Contributor

jmacd commented Mar 4, 2020

The issue where this was discussed for span attributes
#431

The PR:
#459

@bogdandrutu
Copy link
Member

@jmacd this issue was about how do we model in the protocol unspecified vs empty, currently in the protocol we have a good way to support both, so I don't think it is still an issue for the protocol. Can we move your comment to a separate issue (maybe a new one)?

@jmacd
Copy link
Contributor

jmacd commented Mar 4, 2020

This issue states:

We need to clarify the need for "unspecified" values and if they are not necessary remove them from the spec.

@jmacd
Copy link
Contributor

jmacd commented Mar 4, 2020

I propose to close this issue. The matter is / will be resolved as follows:

  1. We have a good way to provide both in the OTLP protocol currently
  2. Span attributes, metric labels, resources, and all key-values generally treat empty-string as a meaningful value, which is currently the case
  3. The default metric SDK "Accumulator" will consider label sets with empty-string and unspecified values as distinct
  4. The default metric SDK "Integrator" will implement a configurable policy here, because some metric exporters are known to group unspecified and empty-string into equivalent classes

We may update the SDK specification to state that when aggregating metric events for a given label key, and that label key has no specified value, and where the usage implies that string value is required, that the empty string shall be the preferred representation. It is as close to "No value" as we can get.

@jmacd
Copy link
Contributor

jmacd commented Mar 5, 2020

This was discussed in the Metrics SIG--closing with the explanation above.

@jmacd jmacd closed this as completed Mar 5, 2020
TuckTuckFloof pushed a commit to TuckTuckFloof/opentelemetry-specification that referenced this issue Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

No branches or pull requests

3 participants