From 70bd8450ca64b25edcbe9a9b9e91302eaa3c6ca5 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 22 Jul 2026 01:21:50 -0500 Subject: [PATCH 1/2] Fix declarative config env substitution and top-level fields - Support the ${env:VAR} prefixed environment variable reference form. - Apply ${VAR:-default} defaults when the referenced variable is set-but-empty, not only when it is unset (per the data-model spec). - Honor top-level log_level and attribute_limits in configure_sdk: log_level sets the SDK internal logger level; attribute_limits are threaded into the tracer provider as the default span attribute count/length limits. --- .changelog/0000.fixed | 4 + .../src/opentelemetry/configuration/_sdk.py | 57 +++++++++++- .../configuration/_tracer_provider.py | 63 ++++++++----- .../configuration/file/_env_substitution.py | 26 ++++-- .../tests/file/test_env_substitution.py | 37 ++++++++ opentelemetry-configuration/tests/test_sdk.py | 89 ++++++++++++++++++- 6 files changed, 247 insertions(+), 29 deletions(-) create mode 100644 .changelog/0000.fixed diff --git a/.changelog/0000.fixed b/.changelog/0000.fixed new file mode 100644 index 00000000000..ae04b974309 --- /dev/null +++ b/.changelog/0000.fixed @@ -0,0 +1,4 @@ +Declarative configuration: support the `${env:VAR}` prefixed environment +variable reference form, apply `${VAR:-default}` defaults when the referenced +variable is set-but-empty (not only when unset), and honor the top-level +`log_level` and `attribute_limits` fields in `configure_sdk` diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/_sdk.py b/opentelemetry-configuration/src/opentelemetry/configuration/_sdk.py index f7d8c9f16ad..f3e6243f555 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/_sdk.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/_sdk.py @@ -10,6 +10,7 @@ from __future__ import annotations +import logging from logging import getLogger from opentelemetry.configuration._logger_provider import ( @@ -26,10 +27,44 @@ from opentelemetry.configuration.instrumentation import ( configure_instrumentation, ) -from opentelemetry.configuration.models import OpenTelemetryConfiguration +from opentelemetry.configuration.models import ( + OpenTelemetryConfiguration, + SeverityNumber, +) _logger = getLogger(__name__) +# Maps the declarative-config ``log_level`` severity onto the closest Python +# stdlib logging level for the SDK's own internal logging. The config field +# uses the log severity vocabulary, so severities are bucketed into the five +# standard Python levels. +_SEVERITY_TO_LOGGING_LEVEL: dict[SeverityNumber, int] = { + SeverityNumber.trace: logging.DEBUG, + SeverityNumber.trace2: logging.DEBUG, + SeverityNumber.trace3: logging.DEBUG, + SeverityNumber.trace4: logging.DEBUG, + SeverityNumber.debug: logging.DEBUG, + SeverityNumber.debug2: logging.DEBUG, + SeverityNumber.debug3: logging.DEBUG, + SeverityNumber.debug4: logging.DEBUG, + SeverityNumber.info: logging.INFO, + SeverityNumber.info2: logging.INFO, + SeverityNumber.info3: logging.INFO, + SeverityNumber.info4: logging.INFO, + SeverityNumber.warn: logging.WARNING, + SeverityNumber.warn2: logging.WARNING, + SeverityNumber.warn3: logging.WARNING, + SeverityNumber.warn4: logging.WARNING, + SeverityNumber.error: logging.ERROR, + SeverityNumber.error2: logging.ERROR, + SeverityNumber.error3: logging.ERROR, + SeverityNumber.error4: logging.ERROR, + SeverityNumber.fatal: logging.CRITICAL, + SeverityNumber.fatal2: logging.CRITICAL, + SeverityNumber.fatal3: logging.CRITICAL, + SeverityNumber.fatal4: logging.CRITICAL, +} + def configure_sdk(config: OpenTelemetryConfiguration) -> None: """Configure the global SDK from a parsed declarative configuration. @@ -60,8 +95,26 @@ def configure_sdk(config: OpenTelemetryConfiguration) -> None: ) return + if config.log_level is not None: + # Apply the top-level ``log_level`` to the OpenTelemetry SDK's own + # internal logger so its diagnostic output honors the configured + # verbosity, per the spec's top-level ``log_level`` field. + logging.getLogger("opentelemetry").setLevel( + _SEVERITY_TO_LOGGING_LEVEL[config.log_level] + ) + + if config.attribute_limits is not None: + _logger.warning( + "Top-level attribute_limits are only applied to spans (via " + "SpanLimits); the Python SDK LoggerProvider and MeterProvider " + "constructors do not accept attribute limits, so they are ignored " + "for logs and metrics." + ) + resource = create_resource(config.resource) - configure_tracer_provider(config.tracer_provider, resource) + configure_tracer_provider( + config.tracer_provider, resource, config.attribute_limits + ) configure_meter_provider(config.meter_provider, resource) configure_logger_provider(config.logger_provider, resource) configure_propagator(config.propagator) diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/_tracer_provider.py b/opentelemetry-configuration/src/opentelemetry/configuration/_tracer_provider.py index c65b659f4c2..cfcf58ded54 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/_tracer_provider.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/_tracer_provider.py @@ -16,6 +16,9 @@ ConfigurationError, MissingDependencyError, ) +from opentelemetry.configuration.models import ( + AttributeLimits as AttributeLimitsConfig, +) from opentelemetry.configuration.models import ( ExperimentalComposableRuleBasedSampler as RuleBasedSamplerConfig, ) @@ -400,45 +403,64 @@ def _create_parent_based_sampler(config: ParentBasedSamplerConfig) -> Sampler: return ParentBased(**kwargs) -def _create_span_limits(config: SpanLimitsConfig) -> SpanLimits: +def _create_span_limits( + config: SpanLimitsConfig | None, + attribute_limits: AttributeLimitsConfig | None = None, +) -> SpanLimits: """Create SpanLimits from config. - Absent fields use the OTel spec defaults (128 for counts, unlimited for lengths). + Precedence for the generic per-attribute limits (``attribute_count_limit`` + and ``attribute_value_length_limit``): the tracer provider's own + ``limits`` override the top-level ``attribute_limits``, which in turn + override the OTel spec defaults (128 for counts, unlimited for lengths). Explicit values suppress env-var reading — matching Java SDK behavior. """ + span_attribute_count = None + span_attribute_length = None + if config is not None: + span_attribute_count = config.attribute_count_limit + span_attribute_length = config.attribute_value_length_limit + if span_attribute_count is None and attribute_limits is not None: + span_attribute_count = attribute_limits.attribute_count_limit + if span_attribute_length is None and attribute_limits is not None: + span_attribute_length = attribute_limits.attribute_value_length_limit + return SpanLimits( max_span_attributes=( - config.attribute_count_limit - if config.attribute_count_limit is not None + span_attribute_count + if span_attribute_count is not None else _DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT ), max_events=( config.event_count_limit - if config.event_count_limit is not None + if config is not None and config.event_count_limit is not None else _DEFAULT_OTEL_SPAN_EVENT_COUNT_LIMIT ), max_links=( config.link_count_limit - if config.link_count_limit is not None + if config is not None and config.link_count_limit is not None else _DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT ), max_event_attributes=( config.event_attribute_count_limit - if config.event_attribute_count_limit is not None + if config is not None + and config.event_attribute_count_limit is not None else _DEFAULT_OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT ), max_link_attributes=( config.link_attribute_count_limit - if config.link_attribute_count_limit is not None + if config is not None + and config.link_attribute_count_limit is not None else _DEFAULT_OTEL_LINK_ATTRIBUTE_COUNT_LIMIT ), - max_attribute_length=config.attribute_value_length_limit, + max_attribute_length=span_attribute_length, ) def create_tracer_provider( config: TracerProviderConfig | None, resource: Resource | None = None, + attribute_limits: AttributeLimitsConfig | None = None, ) -> TracerProvider: """Create an SDK TracerProvider from declarative config. @@ -449,6 +471,9 @@ def create_tracer_provider( Args: config: TracerProvider config from the parsed config file, or None. resource: Resource to attach to the provider. + attribute_limits: Top-level ``attribute_limits`` from the parsed config, + used as the default for span attribute count/length limits when the + tracer provider does not specify its own. Returns: A configured TracerProvider. @@ -463,16 +488,9 @@ def create_tracer_provider( if config is not None and config.id_generator is not None else None ) - span_limits = ( - _create_span_limits(config.limits) - if config is not None and config.limits is not None - else SpanLimits( - max_span_attributes=_DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, - max_events=_DEFAULT_OTEL_SPAN_EVENT_COUNT_LIMIT, - max_links=_DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT, - max_event_attributes=_DEFAULT_OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT, - max_link_attributes=_DEFAULT_OTEL_LINK_ATTRIBUTE_COUNT_LIMIT, - ) + span_limits = _create_span_limits( + config.limits if config is not None else None, + attribute_limits, ) provider = TracerProvider( @@ -492,6 +510,7 @@ def create_tracer_provider( def configure_tracer_provider( config: TracerProviderConfig | None, resource: Resource | None = None, + attribute_limits: AttributeLimitsConfig | None = None, ) -> None: """Configure the global TracerProvider from declarative config. @@ -502,7 +521,11 @@ def configure_tracer_provider( Args: config: TracerProvider config from the parsed config file, or None. resource: Resource to attach to the provider. + attribute_limits: Top-level ``attribute_limits`` from the parsed config, + applied as the default span attribute count/length limits. """ if config is None: return - trace.set_tracer_provider(create_tracer_provider(config, resource)) + trace.set_tracer_provider( + create_tracer_provider(config, resource, attribute_limits) + ) diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py b/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py index 9d3a1b815aa..c76d67e7493 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py @@ -23,7 +23,10 @@ def substitute_env_vars(text: str) -> str: Supports the following syntax: - ${VAR}: Substitute with environment variable VAR. Raises error if not found. - - ${VAR:-default}: Substitute with VAR if set, otherwise use default value. + - ${env:VAR}: Prefixed form of ${VAR}; the ``env:`` prefix is optional per + the spec grammar and behaves identically. + - ${VAR:-default}: Substitute with VAR if set and non-empty, otherwise use + the default value. - $$: Escape sequence for literal $. Args: @@ -39,14 +42,18 @@ def substitute_env_vars(text: str) -> str: >>> os.environ['SERVICE_NAME'] = 'my-service' >>> substitute_env_vars('name: ${SERVICE_NAME}') 'name: my-service' + >>> substitute_env_vars('name: ${env:SERVICE_NAME}') + 'name: my-service' >>> substitute_env_vars('name: ${MISSING:-default}') 'name: default' >>> substitute_env_vars('price: $$100') 'price: $100' """ - # Pattern matches $$ (escape sequence) or ${VAR_NAME} / ${VAR_NAME:-default_value} - # Handling both in a single pass ensures $$ followed by ${VAR} works correctly - pattern = r"\$\$|\$\{([A-Za-z_][A-Za-z0-9_]*)(:-([^}]*))?\}" + # Pattern matches $$ (escape sequence) or + # ${[env:]VAR_NAME} / ${[env:]VAR_NAME:-default_value}. + # The optional ``env:`` prefix is part of the spec grammar for references. + # Handling both in a single pass ensures $$ followed by ${VAR} works correctly. + pattern = r"\$\$|\$\{(?:env:)?([A-Za-z_][A-Za-z0-9_]*)(:-([^}]*))?\}" def replace_var(match) -> str: if match.group(1) is None: @@ -59,9 +66,16 @@ def replace_var(match) -> str: value = os.environ.get(var_name) + # Per spec (configuration/data-model.md): when a default value is + # provided via ``:-``, it applies when the referenced variable is + # null, empty, or undefined. Treat a set-but-empty variable the same + # as an unset one for the purpose of applying the default. + if has_default and (value is None or value == ""): + return default_value or "" + + # No default provided. An undefined variable is an error; a + # set-but-empty variable substitutes to its (empty) value. if value is None: - if has_default: - return default_value or "" _logger.error( "Environment variable '%s' not found and no default provided", var_name, diff --git a/opentelemetry-configuration/tests/file/test_env_substitution.py b/opentelemetry-configuration/tests/file/test_env_substitution.py index 14c79f20fd5..fcd545b8983 100644 --- a/opentelemetry-configuration/tests/file/test_env_substitution.py +++ b/opentelemetry-configuration/tests/file/test_env_substitution.py @@ -40,6 +40,43 @@ def test_substitution_with_default_override(self): result = substitute_env_vars("name: ${SERVICE_NAME:-default}") self.assertEqual(result, "name: actual") + def test_prefixed_reference_substitution(self): + """Test ${env:VAR} prefixed reference substitutes like ${VAR}.""" + with patch.dict(os.environ, {"SERVICE_NAME": "my-service"}): + result = substitute_env_vars("name: ${env:SERVICE_NAME}") + self.assertEqual(result, "name: my-service") + + def test_prefixed_reference_with_default(self): + """Test ${env:VAR:-default} prefixed reference honors the default.""" + with patch.dict(os.environ, {}, clear=True): + result = substitute_env_vars("name: ${env:MISSING:-default}") + self.assertEqual(result, "name: default") + + def test_prefixed_reference_missing_raises_error(self): + """Test ${env:VAR} without default raises when the variable is unset.""" + with patch.dict(os.environ, {}, clear=True): + with self.assertRaises(EnvSubstitutionError) as ctx: + substitute_env_vars("name: ${env:MISSING_VAR}") + self.assertIn("MISSING_VAR", str(ctx.exception)) + + def test_default_applied_when_variable_empty(self): + """Test ${VAR:-default} uses the default when VAR is set but empty.""" + with patch.dict(os.environ, {"SERVICE_NAME": ""}): + result = substitute_env_vars("name: ${SERVICE_NAME:-default}") + self.assertEqual(result, "name: default") + + def test_empty_variable_without_default_substitutes_empty(self): + """Test ${VAR} with VAR set-but-empty substitutes to empty, no error.""" + with patch.dict(os.environ, {"EMPTY_VAR": ""}): + result = substitute_env_vars("name: ${EMPTY_VAR}") + self.assertEqual(result, "name: ") + + def test_prefixed_default_applied_when_variable_empty(self): + """Test ${env:VAR:-default} uses the default when VAR is set but empty.""" + with patch.dict(os.environ, {"SERVICE_NAME": ""}): + result = substitute_env_vars("name: ${env:SERVICE_NAME:-default}") + self.assertEqual(result, "name: default") + def test_missing_variable_raises_error(self): """Test ${VAR} raises error when variable missing.""" with patch.dict(os.environ, {}, clear=True): diff --git a/opentelemetry-configuration/tests/test_sdk.py b/opentelemetry-configuration/tests/test_sdk.py index 1637eba8d7f..c3579272b83 100644 --- a/opentelemetry-configuration/tests/test_sdk.py +++ b/opentelemetry-configuration/tests/test_sdk.py @@ -4,12 +4,17 @@ # Tests access private members of SDK classes to assert correct configuration. # pylint: disable=protected-access +import logging import unittest from unittest.mock import patch from opentelemetry.configuration._sdk import configure_sdk +from opentelemetry.configuration.models import ( + AttributeLimits as AttributeLimitsConfig, +) from opentelemetry.configuration.models import ( OpenTelemetryConfiguration, + SeverityNumber, ) from opentelemetry.configuration.models import ( Propagator as PropagatorConfig, @@ -68,7 +73,9 @@ def test_calls_each_signal_with_resource( configure_sdk(config) mock_create_resource.assert_called_once_with(resource_cfg) - mock_tracer.assert_called_once_with(tracer_cfg, sentinel_resource) + mock_tracer.assert_called_once_with( + tracer_cfg, sentinel_resource, None + ) mock_meter.assert_called_once_with(None, sentinel_resource) mock_logger.assert_called_once_with(None, sentinel_resource) mock_propagator.assert_called_once_with(propagator_cfg) @@ -147,3 +154,83 @@ def test_applies_tracer_provider_globally(self, mock_set_tracer): self.assertIsInstance( mock_set_tracer.call_args[0][0], SdkTracerProvider ) + + +class TestConfigureSdkLogLevel(unittest.TestCase): + """Top-level ``log_level`` is applied to the SDK's internal logger.""" + + def setUp(self): + self._otel_logger = logging.getLogger("opentelemetry") + self._original_level = self._otel_logger.level + self.addCleanup(self._otel_logger.setLevel, self._original_level) + + @patch("opentelemetry.configuration._sdk.configure_propagator") + @patch("opentelemetry.configuration._sdk.configure_logger_provider") + @patch("opentelemetry.configuration._sdk.configure_meter_provider") + @patch("opentelemetry.configuration._sdk.configure_tracer_provider") + @patch("opentelemetry.configuration._sdk.create_resource") + def test_log_level_sets_internal_logger_level(self, *_mocks): + configure_sdk(_config(log_level=SeverityNumber.debug)) + self.assertEqual(self._otel_logger.level, logging.DEBUG) + + @patch("opentelemetry.configuration._sdk.configure_propagator") + @patch("opentelemetry.configuration._sdk.configure_logger_provider") + @patch("opentelemetry.configuration._sdk.configure_meter_provider") + @patch("opentelemetry.configuration._sdk.configure_tracer_provider") + @patch("opentelemetry.configuration._sdk.create_resource") + def test_log_level_error_maps_to_logging_error(self, *_mocks): + configure_sdk(_config(log_level=SeverityNumber.error)) + self.assertEqual(self._otel_logger.level, logging.ERROR) + + @patch("opentelemetry.configuration._sdk.configure_propagator") + @patch("opentelemetry.configuration._sdk.configure_logger_provider") + @patch("opentelemetry.configuration._sdk.configure_meter_provider") + @patch("opentelemetry.configuration._sdk.configure_tracer_provider") + @patch("opentelemetry.configuration._sdk.create_resource") + def test_log_level_absent_leaves_internal_logger_untouched(self, *_mocks): + self._otel_logger.setLevel(logging.WARNING) + configure_sdk(_config()) + self.assertEqual(self._otel_logger.level, logging.WARNING) + + +class TestConfigureSdkAttributeLimits(unittest.TestCase): + """Top-level ``attribute_limits`` are threaded to the tracer provider.""" + + @patch("opentelemetry.configuration._sdk.configure_propagator") + @patch("opentelemetry.configuration._sdk.configure_logger_provider") + @patch("opentelemetry.configuration._sdk.configure_meter_provider") + @patch("opentelemetry.configuration._sdk.configure_tracer_provider") + @patch("opentelemetry.configuration._sdk.create_resource") + def test_attribute_limits_passed_to_tracer_provider( + self, + mock_create_resource, + mock_tracer, + _mock_meter, + _mock_logger, + _mock_propagator, + ): + sentinel_resource = object() + mock_create_resource.return_value = sentinel_resource + limits = AttributeLimitsConfig( + attribute_count_limit=7, attribute_value_length_limit=42 + ) + + configure_sdk(_config(attribute_limits=limits)) + + mock_tracer.assert_called_once_with(None, sentinel_resource, limits) + + def test_attribute_limits_applied_to_span_limits(self): + from opentelemetry.configuration._tracer_provider import ( # noqa: PLC0415 + create_tracer_provider, + ) + + provider = create_tracer_provider( + None, + None, + AttributeLimitsConfig( + attribute_count_limit=9, attribute_value_length_limit=11 + ), + ) + span_limits = provider._span_limits + self.assertEqual(span_limits.max_span_attributes, 9) + self.assertEqual(span_limits.max_attribute_length, 11) From 530cfd6afa0ee9b82990219711990fa62fb5e365 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 22 Jul 2026 08:38:02 -0500 Subject: [PATCH 2/2] Rename changelog fragment to match PR number --- .changelog/{0000.fixed => 36.fixed} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{0000.fixed => 36.fixed} (100%) diff --git a/.changelog/0000.fixed b/.changelog/36.fixed similarity index 100% rename from .changelog/0000.fixed rename to .changelog/36.fixed