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 modeling guidance for mismatches with standard env vars, add resource.attribute_list #106

Merged

Conversation

jack-berg
Copy link
Member

@jack-berg jack-berg commented Jul 22, 2024

Resolves open-telemetry/opentelemetry-specification#3966.

  • Define modeling guidance that properties should use the most appropriate data types for the problem domain
  • This may result in the lack of ability to support standard env vars where type mismatches occur
  • When there is a type mismatch, a demonstrated need for platforms to contribute to config, and no reasonable alternative, we provide alternative versions of the property which match the standard env var
  • For example, OTEL_RESOURCE_ATTRIBUTES is string whose value is a comma separated list of key value pairs. This is not a natural way to represent key value pairs in YAML, yet for reasons discussed in PR#3966 is important for platforms. Therefore, we provide an alternative .resource.attributes_list property which matches the format of OTEL_RESOURCE_ATTRIBUTES. Implementations merge the contents of .resource.attributes and .resource.attributes_list, with .resource.attributes taking priority.

Copy link
Member

@zeitlinger zeitlinger left a comment

Choose a reason for hiding this comment

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

lgtm

@jsuereth
Copy link

FYI - I'd like to go a different direction with resource variables BUT I think this makes good progress on status quo.

See https://github.com/open-telemetry/oteps/pull/264/files - environment variable detector

@jack-berg jack-berg merged commit 50fbb1f into open-telemetry:main Aug 20, 2024
2 checks passed
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.

Ensure that file config has solution for platforms which contribute to config
6 participants