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 parse requirements around null vs. empty #4181

Closed
jack-berg opened this issue Aug 7, 2024 · 0 comments · Fixed by #4269
Closed

Clarify parse requirements around null vs. empty #4181

jack-berg opened this issue Aug 7, 2024 · 0 comments · Fixed by #4269
Assignees
Labels
area:configuration Related to configuring the SDK sig-issue A specific SIG should look into this before discussing at the spec spec:miscellaneous For issues that don't match any other spec label

Comments

@jack-berg
Copy link
Member

The file config spec says:

Parse MUST interpret null as equivalent to unset.

This has led to some confusion (see discussion 1) and could use clarification.

Let's illustrate the intent of this statement with some examples:

Example 1

meter_provider:
  views:
    - selector:
         name: my-metric1
      stream:
        aggregation:
          drop:
    - selector:
         name: my-metric2
      stream:
        aggregation:
          explicit_bucket_histogram:
            boundaries: [1.0, 5.0, 10.0]

Views have an option to configure the aggregation of the resulting metric stream. Some aggregations like explicit_bucket_histogram accept some configurable parameters. Others like drop do not. We want users to be able to use the syntax above, and not have to specify drop: {}.

When the YAML above is parsed, drop: resolves to null, while drop:{} resolves to an empty object. We need the fact that the drop key was provided to be retained after parse. In other words, it would not be acceptable if key/values with a null value were dropped by parse. It is semantically meaningful that a key was set, even if its value is null.

Example 2

attribute_limits:
  # Configure max attribute value size.
  attribute_value_length_limit: ${OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT}

The attribute_value_length_limit has no default value. If unset, no limit should be imposed on the length of attribute values. In this snippet, we reference the OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT env var using the env var substitution syntax. If the env var is unset, we want ensure the resulting SDK is created with no length limit. The result should be equivalent if the the .attribute_limits.attribute_value_length_limit is omitted altogether.

We should clarify the spec to ensure that the behavior in these examples is accomplished.

@jack-berg jack-berg added spec:miscellaneous For issues that don't match any other spec label area:configuration Related to configuring the SDK sig-issue A specific SIG should look into this before discussing at the spec labels Aug 7, 2024
@jack-berg jack-berg self-assigned this Sep 16, 2024
@jack-berg jack-berg moved this from Todo to In Progress in Declarative Configuration Stability Oct 21, 2024
jack-berg added a commit that referenced this issue Oct 28, 2024
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 31, 2024
Fixes open-telemetry#4181

cc @open-telemetry/configuration-approvers

---------

Co-authored-by: Robert Pająk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:configuration Related to configuring the SDK sig-issue A specific SIG should look into this before discussing at the spec spec:miscellaneous For issues that don't match any other spec label
Development

Successfully merging a pull request may close this issue.

1 participant