From 11c84b420137461e6bfe410eeb7464dcfe20d774 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Sun, 12 Jul 2026 20:59:24 -0500 Subject: [PATCH 1/2] Substitute empty for unset config env vars without defaults 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 #5405 --- .changelog/5408.fixed | 1 + .../configuration/file/_env_substitution.py | 34 ++++++++----------- .../configuration/file/_loader.py | 1 - .../tests/file/test_env_substitution.py | 20 ++++++----- .../tests/file/test_loader.py | 24 ++++++++----- 5 files changed, 42 insertions(+), 38 deletions(-) create mode 100644 .changelog/5408.fixed diff --git a/.changelog/5408.fixed b/.changelog/5408.fixed new file mode 100644 index 00000000000..67796607782 --- /dev/null +++ b/.changelog/5408.fixed @@ -0,0 +1 @@ +`opentelemetry-configuration`: declarative config environment variable substitution now replaces an unset variable that has no default with an empty value instead of raising an error, per the configuration spec. diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py b/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py index 9d3a1b815aa..1e767214fa2 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py @@ -3,18 +3,16 @@ """Environment variable substitution for configuration files.""" -import logging import os import re -_logger = logging.getLogger(__name__) - class EnvSubstitutionError(Exception): - """Raised when environment variable substitution fails. + """Error raised when environment variable substitution fails. - This occurs when a ${VAR} reference is found but the environment - variable is not set and no default value is provided. + Retained as part of the public API for backward compatibility. An unset + variable without a default is no longer an error (it is replaced with an + empty value per the configuration spec), so this is not currently raised. """ @@ -22,19 +20,23 @@ def substitute_env_vars(text: str) -> str: """Substitute environment variables in configuration text. Supports the following syntax: - - ${VAR}: Substitute with environment variable VAR. Raises error if not found. + - ${VAR}: Substitute with environment variable VAR, or an empty value if + VAR is not set. - ${VAR:-default}: Substitute with VAR if set, otherwise use default value. - $$: Escape sequence for literal $. + Per the declarative configuration specification, a referenced environment + variable that is not set and has no default is replaced with an empty + value (which the YAML parser then interprets as null). This matches the + behavior of the Java and Node.js implementations and lets configuration + files be shared across languages. + Args: text: Configuration text with potential ${VAR} placeholders. Returns: Text with environment variables substituted. - Raises: - EnvSubstitutionError: If a required environment variable is not found. - Examples: >>> os.environ['SERVICE_NAME'] = 'my-service' >>> substitute_env_vars('name: ${SERVICE_NAME}') @@ -60,15 +62,9 @@ def replace_var(match) -> str: value = os.environ.get(var_name) if value is None: - if has_default: - return default_value or "" - _logger.error( - "Environment variable '%s' not found and no default provided", - var_name, - ) - raise EnvSubstitutionError( - f"Environment variable '{var_name}' not found and no default provided" - ) + # An unset variable is replaced with its default if one is + # provided, otherwise with an empty value, per the spec. + return (default_value or "") if has_default else "" # Per spec: "It MUST NOT be possible to inject YAML structures by # environment variables." Newlines are the primary injection vector — diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/file/_loader.py b/opentelemetry-configuration/src/opentelemetry/configuration/file/_loader.py index 6131e3c1b08..c4fcdf8f886 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/file/_loader.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/file/_loader.py @@ -82,7 +82,6 @@ def load_config_file( Raises: ConfigurationError: If file cannot be read, parsed, or validated. - EnvSubstitutionError: If required environment variable is missing. Examples: >>> config = load_config_file("otel-config.yaml") diff --git a/opentelemetry-configuration/tests/file/test_env_substitution.py b/opentelemetry-configuration/tests/file/test_env_substitution.py index 14c79f20fd5..92bbc711abf 100644 --- a/opentelemetry-configuration/tests/file/test_env_substitution.py +++ b/opentelemetry-configuration/tests/file/test_env_substitution.py @@ -7,10 +7,7 @@ import yaml -from opentelemetry.configuration.file import ( - EnvSubstitutionError, - substitute_env_vars, -) +from opentelemetry.configuration.file import substitute_env_vars class TestEnvSubstitution(unittest.TestCase): @@ -40,12 +37,17 @@ def test_substitution_with_default_override(self): result = substitute_env_vars("name: ${SERVICE_NAME:-default}") self.assertEqual(result, "name: actual") - def test_missing_variable_raises_error(self): - """Test ${VAR} raises error when variable missing.""" + def test_missing_variable_without_default_substitutes_empty(self): + """An unset ${VAR} without a default is replaced with an empty value. + + Per the declarative configuration spec, unset variables without + defaults are replaced with an empty value, which YAML then reads as + null. This matches the Java and Node.js implementations. + """ with patch.dict(os.environ, {}, clear=True): - with self.assertRaises(EnvSubstitutionError) as ctx: - substitute_env_vars("name: ${MISSING_VAR}") - self.assertIn("MISSING_VAR", str(ctx.exception)) + result = substitute_env_vars("name: ${MISSING_VAR}") + self.assertEqual(result, "name: ") + self.assertIsNone(yaml.safe_load(result)["name"]) def test_dollar_sign_escape(self): """Test $$ escapes to literal $.""" diff --git a/opentelemetry-configuration/tests/file/test_loader.py b/opentelemetry-configuration/tests/file/test_loader.py index c9dcbee8e67..ea9689008b5 100644 --- a/opentelemetry-configuration/tests/file/test_loader.py +++ b/opentelemetry-configuration/tests/file/test_loader.py @@ -137,19 +137,25 @@ def test_non_dict_root(self): finally: os.unlink(temp_path) - def test_missing_required_env_var(self): - """Test error when required env var is missing.""" + def test_unset_env_var_without_default_substitutes_empty(self): + """An unset env var without a default resolves to an empty value. + + Per the declarative configuration spec, an unset ``${VAR}`` reference + with no default is replaced with an empty value rather than raising, + so the file still loads. A default (``${ENV:-production}``) is still + applied when its variable is unset. + """ config_path = self.test_data_dir / "config_with_env_vars.yaml" with patch.dict(os.environ, {}, clear=True): - with self.assertRaises(ConfigurationError) as ctx: - load_config_file(str(config_path)) + config = load_config_file(str(config_path)) - # Should mention substitution or env var error - self.assertTrue( - "substitution" in str(ctx.exception).lower() - or "environment" in str(ctx.exception).lower() - ) + attributes = { + attribute.name: attribute.value + for attribute in config.resource.attributes + } + self.assertIsNone(attributes["service.name"]) + self.assertEqual(attributes["deployment.environment"], "production") def test_yml_extension(self): """Test .yml extension is supported.""" From cff2dea1ea8a3b4c15dc533a66d90f1caafa4b01 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Jul 2026 19:49:39 -0500 Subject: [PATCH 2/2] opentelemetry-configuration: remove EnvSubstitutionError, guard null resource attributes Address review feedback on #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. --- .changelog/5408.fixed | 2 +- .../opentelemetry/configuration/_resource.py | 9 +++++ .../configuration/file/__init__.py | 2 -- .../configuration/file/_env_substitution.py | 9 ----- .../tests/test_resource.py | 33 +++++++++++++++++++ 5 files changed, 43 insertions(+), 12 deletions(-) diff --git a/.changelog/5408.fixed b/.changelog/5408.fixed index 67796607782..eaaea1c00a3 100644 --- a/.changelog/5408.fixed +++ b/.changelog/5408.fixed @@ -1 +1 @@ -`opentelemetry-configuration`: declarative config environment variable substitution now replaces an unset variable that has no default with an empty value instead of raising an error, per the configuration spec. +`opentelemetry-configuration`: declarative config environment variable substitution now replaces an unset variable that has no default with an empty value instead of raising an error, per the configuration spec. Resource attributes whose value resolves to null (an unset `${VAR}` with no default) are skipped with a warning instead of being inserted as a null value. diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/_resource.py b/opentelemetry-configuration/src/opentelemetry/configuration/_resource.py index 199324405a8..79196c0d97e 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/_resource.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/_resource.py @@ -113,6 +113,15 @@ def create_resource(config: ResourceConfig | None) -> Resource: if config.attributes: for attr in config.attributes: + # An unset ${VAR} with no default substitutes an empty value that + # the YAML parser reads as null. Skip such attributes rather than + # inserting a None (or coercing it into garbage like "None"/0). + if attr.value is None: + _logger.warning( + "Ignoring resource attribute '%s' with empty value", + attr.name, + ) + continue config_attrs[attr.name] = _coerce_attribute_value(attr) schema_url = config.schema_url diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/file/__init__.py b/opentelemetry-configuration/src/opentelemetry/configuration/file/__init__.py index 7be89f5ffd8..2c8c4ba48e6 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/file/__init__.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/file/__init__.py @@ -36,7 +36,6 @@ create_tracer_provider, ) from opentelemetry.configuration.file._env_substitution import ( - EnvSubstitutionError, substitute_env_vars, ) from opentelemetry.configuration.file._loader import load_config_file @@ -47,7 +46,6 @@ "substitute_env_vars", "ConfigurationError", "MissingDependencyError", - "EnvSubstitutionError", "create_resource", "create_propagator", "configure_propagator", diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py b/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py index 1e767214fa2..96696d8c395 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py @@ -7,15 +7,6 @@ import re -class EnvSubstitutionError(Exception): - """Error raised when environment variable substitution fails. - - Retained as part of the public API for backward compatibility. An unset - variable without a default is no longer an error (it is replaced with an - empty value per the configuration spec), so this is not currently raised. - """ - - def substitute_env_vars(text: str) -> str: """Substitute environment variables in configuration text. diff --git a/opentelemetry-configuration/tests/test_resource.py b/opentelemetry-configuration/tests/test_resource.py index 3a21872ce12..b822a37f8fa 100644 --- a/opentelemetry-configuration/tests/test_resource.py +++ b/opentelemetry-configuration/tests/test_resource.py @@ -224,6 +224,39 @@ def test_attribute_type_bool_array(self): resource = create_resource(config) self.assertEqual(list(resource.attributes["k"]), [True, False]) # type: ignore[arg-type] + def test_none_value_attribute_skipped_with_warning(self): + """An unset ${VAR} with no default yields a null value; it must be + skipped (not inserted as None or coerced) and a warning logged.""" + with self.assertLogs( + "opentelemetry.configuration._resource", level="WARNING" + ) as cm: + config = ResourceConfig( + attributes=[ + AttributeNameValue(name="empty", value=None), + AttributeNameValue(name="env", value="production"), + ] + ) + resource = create_resource(config) + self.assertNotIn("empty", resource.attributes) + self.assertEqual(resource.attributes["env"], "production") + self.assertTrue(any("empty" in msg for msg in cm.output)) + + def test_none_value_typed_attribute_skipped(self): + """A null value with a declared type must be skipped, not coerced + (int(None)/str(None) would raise or produce garbage).""" + with self.assertLogs( + "opentelemetry.configuration._resource", level="WARNING" + ): + config = ResourceConfig( + attributes=[ + AttributeNameValue( + name="k", value=None, type=AttributeType.int + ) + ] + ) + resource = create_resource(config) + self.assertNotIn("k", resource.attributes) + def test_attribute_type_bool_array_string_values(self): """bool_array must use _coerce_bool, not plain bool() — 'false' must be False.""" config = ResourceConfig(