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

Support for ${env:ENV} syntax #3961

Closed
Tracked by #3963
mx-psi opened this issue Mar 25, 2024 · 2 comments · Fixed by #3974
Closed
Tracked by #3963

Support for ${env:ENV} syntax #3961

mx-psi opened this issue Mar 25, 2024 · 2 comments · Fixed by #3974
Assignees
Labels
area:configuration Related to configuring the SDK triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted

Comments

@mx-psi
Copy link
Member

mx-psi commented Mar 25, 2024

The Collector supports ${env:ENV} style syntax in addition to ${ENV} and $ENV. It would be desirable that the Collector and the SDK configuration files support the same syntax.

The Collector SIG has previously discussed dropping the $ENV syntax, and there seems to be consensus for it. While this needs to be explicitly discussed, I think we are willing to support ${ENV} as a shorthand for ${env:ENV}.

To align the two configuration formats, I would like to ask the Configuration WG to do one of the following:

  1. Support the ${env:ENV} syntax as equivalent to ${ENV} OR
  2. Explicitly fail when the ${env:ENV} syntax is present in a configuration file

Since the environment variable names accepted by the current specification are ASCII alphanumerics + _, there is no ambiguity with the ${env:ENV} syntax. However, we need to make a decision now on this (while experimental), since resolution would work differently.

@jack-berg jack-berg transferred this issue from open-telemetry/opentelemetry-configuration Mar 25, 2024
@jack-berg jack-berg added the area:configuration Related to configuring the SDK label Mar 25, 2024
@jack-berg
Copy link
Member

For context, here's the discussion on this topic from when env var substitution was initially added.

We discussed in the 11/21/23 spec sig. Meeting notes entires on the subject:

Options:

  • ${env:MY_VAR}
    • Aligns with collector
    • Exposes collector provider concept which file configuration WG does not (currently) want
  • ${MY_VAR}
    • More familiar syntax
    • Collector would need to un-deprecate this syntax and use it as alias for “env:” provider
    • Evolvable to ${env:MY_VAR} syntax if we later determine we need provider concept
  • (josh) Alternative: {{MY_VAR}}
  • Don’t want to support provider concept in file configuration because simpler syntax promotes more implementations of file configuration
  • Small group seems to agree on simpler ${MY_VAR} syntax. Reasons:
    • More familiar syntax
    • “Env:” prefix suggests that other providers are possible.
    • Can always come back and change later since this is experimental.

So the pushback was about not wanting to burden implementations with the complexity of having to implement other env vars substitution sources besides env vars, and to have a syntax for familiar to bash users.

I think that alignment with the collector syntax is a good reason to accept the ${env:MY_VAR} syntax as an alternative, as long as its not also a call to support additional providers. For the time being, we should priority simplicity and portability across language implementations. Adding additional providers should be supported by strong user feedback.

@trask trask added the triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted label Mar 26, 2024
@trask
Copy link
Member

trask commented Mar 26, 2024

cc @open-telemetry/configuration-maintainers

jack-berg added a commit that referenced this issue Apr 8, 2024
…bstitution (#3974)

Fixes #3961

## Changes

Allows for the usage of the `${env:ENV}` syntax in SDK file
configuration. This syntax is used in the OpenTelemetry Collector, see
https://opentelemetry.io/docs/collector/configuration/#environment-variables
for more information.

This does not mean that other providers need to be supported; if we want
to allow this we would need to reserve that syntax by rejecting
`${<provider>:URI}` as invalid, since right now it would be left as-is.

* [x] Related issues #3961, part of #3963

---------

Co-authored-by: jack-berg <[email protected]>
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 31, 2024
…bstitution (open-telemetry#3974)

Fixes open-telemetry#3961

## Changes

Allows for the usage of the `${env:ENV}` syntax in SDK file
configuration. This syntax is used in the OpenTelemetry Collector, see
https://opentelemetry.io/docs/collector/configuration/#environment-variables
for more information.

This does not mean that other providers need to be supported; if we want
to allow this we would need to reserve that syntax by rejecting
`${<provider>:URI}` as invalid, since right now it would be left as-is.

* [x] Related issues open-telemetry#3961, part of open-telemetry#3963

---------

Co-authored-by: jack-berg <[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 triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted
Projects
None yet
4 participants