Skip to content

Substitute empty for unset config env vars without defaults - #5408

Merged
xrmx merged 5 commits into
open-telemetry:mainfrom
ocelotl:issue_5405_unset_env_var_empty_value
Jul 22, 2026
Merged

Substitute empty for unset config env vars without defaults#5408
xrmx merged 5 commits into
open-telemetry:mainfrom
ocelotl:issue_5405_unset_env_var_empty_value

Conversation

@ocelotl

@ocelotl ocelotl commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #5405.

Declarative config environment variable substitution rejected any ${VAR} reference to an unset variable that had no default, raising a ConfigurationError and preventing SDK initialization:

Environment variable substitution failed: Environment variable 'OTEL_EXPORTER_OTLP_HEADERS' not found and no default provided

The declarative configuration spec requires an unset variable without a default to be replaced with an empty value (which the YAML parser then interprets as null). This matches the Java and Node.js implementations and lets configuration files be shared across languages.

Changes

  • opentelemetry-configuration: an unset ${VAR} with no default now substitutes an empty value instead of raising.
  • Removed the now-unused EnvSubstitutionError (its only raise site is gone) from the module and the file package's public exports.
  • Updated the substitution and loader tests to assert the empty/null behavior; ${ENV:-production} still applies its default.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • opentelemetry-configuration test suite (363 passed, 5 subtests).
  • ruff check and ruff format --check clean on the changed files.

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelog has been updated
  • Unit tests have been added
  • Documentation has been updated

@ocelotl
ocelotl marked this pull request as ready for review July 13, 2026 02:04
@ocelotl
ocelotl requested a review from a team as a code owner July 13, 2026 02:04
@ocelotl
ocelotl force-pushed the issue_5405_unset_env_var_empty_value branch from c108bb7 to 9697dea Compare July 13, 2026 02:33

@MikeGoldsmith MikeGoldsmith left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Left one comment regarding allowing empty substitute values which we'll need to resolve.

@github-project-automation github-project-automation Bot moved this to Reviewed PRs that need fixes in Python PR digest Jul 13, 2026
@ocelotl
ocelotl requested a review from MikeGoldsmith July 16, 2026 02:41
@ocelotl
ocelotl force-pushed the issue_5405_unset_env_var_empty_value branch from 1aa11c5 to 0700cf7 Compare July 16, 2026 02:42
ocelotl added 2 commits July 20, 2026 16:06
Declarative config environment variable substitution rejected any
${VAR} reference to an unset variable that had no default, raising a
ConfigurationError and preventing SDK initialization. The declarative
configuration spec requires an unset variable without a default to be
replaced with an empty value (which YAML then reads as null), matching
the Java and Node.js implementations and allowing config files to be
shared across languages.

Replace the raise with empty substitution. EnvSubstitutionError is kept
as public API for backward compatibility but is no longer raised.
Update tests accordingly.

Fixes open-telemetry#5405
…resource attributes

Address review feedback on open-telemetry#5408:

- Remove the now-unused EnvSubstitutionError class and its public export.
  Nothing raises it since unset variables substitute an empty value.
- Skip resource attributes whose value resolves to null (an unset ${VAR}
  with no default) and log a warning, instead of inserting a None value
  or coercing it into garbage. create_resource and _coerce_attribute_value
  did not guard None, so before this the empty substitution would have
  propagated a null attribute value into the SDK Resource.
@ocelotl
ocelotl force-pushed the issue_5405_unset_env_var_empty_value branch from 0700cf7 to cff2dea Compare July 20, 2026 21:06
…f removing

EnvSubstitutionError shipped in the released 0.65b0, so removing it from the
public API broke the public-symbols-check (griffe). Restore it as a public
export marked with typing_extensions.deprecated; it is no longer raised now
that an unset variable without a default substitutes an empty value. Declare
typing-extensions as a direct dependency since it is now imported directly.
@ocelotl
ocelotl requested a review from herin049 July 22, 2026 01:46
Comment thread opentelemetry-configuration/pyproject.toml Outdated
…set substitution

Per review, the beta opentelemetry-configuration package may make breaking
changes, so remove EnvSubstitutionError (and its now-unnecessary
typing-extensions dependency) rather than deprecating it. The public-symbols
check is bypassed via the 'Approve Public API check' label. Also simplify the
unset-variable branch to 'return default_value or ""', which already covers
the no-default case.
@ocelotl

ocelotl commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@open-telemetry/python-approvers Please skip public symbols check

@xrmx xrmx added the Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary label Jul 22, 2026
@xrmx
xrmx enabled auto-merge July 22, 2026 12:36
@xrmx
xrmx dismissed MikeGoldsmith’s stale review July 22, 2026 12:38

Mike feedback was incorporated in the changes and looks like Mike is on PTO

@xrmx
xrmx added this pull request to the merge queue Jul 22, 2026
Merged via the queue into open-telemetry:main with commit 684b19d Jul 22, 2026
567 checks passed
@github-project-automation github-project-automation Bot moved this from Reviewed PRs that need fixes to Done in Python PR digest Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Declarative config env substitution rejects unset variables without defaults

4 participants