diff --git a/.changelog/36.fixed b/.changelog/36.fixed new file mode 100644 index 0000000000..854b8818c1 --- /dev/null +++ b/.changelog/36.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 +`attribute_limits` field in `configure_sdk` diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/_sdk.py b/opentelemetry-configuration/src/opentelemetry/configuration/_sdk.py index 698c88cce5..d49a867b2b 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/_sdk.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/_sdk.py @@ -99,8 +99,16 @@ def configure_sdk(config: OpenTelemetryConfiguration) -> None: level = _SEVERITY_TO_LOGGING_LEVEL.get(config.log_level, INFO) getLogger("opentelemetry").setLevel(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 733691dc67..5ba7b44d05 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, ) @@ -357,41 +360,60 @@ 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 - else _DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT + 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 else _DEFAULT_OTEL_SPAN_EVENT_COUNT_LIMIT + config.event_count_limit + 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 else _DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT + config.link_count_limit + 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. @@ -402,6 +424,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. @@ -410,16 +435,9 @@ def create_tracer_provider( id_generator = ( _create_id_generator(config.id_generator) 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( @@ -439,6 +457,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. @@ -449,7 +468,9 @@ 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 8b5488233d..91b8f2edf4 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/file/_env_substitution.py @@ -13,7 +13,10 @@ def substitute_env_vars(text: str) -> str: Supports the following syntax: - ${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. + - ${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 $. Per the declarative configuration specification, a referenced environment @@ -32,14 +35,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: @@ -52,11 +59,20 @@ def replace_var(match) -> str: value = os.environ.get(var_name) - if value is None: - # An unset variable is replaced with its default if one is - # provided, otherwise with an empty value, per the spec. + # 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 applied above. Per the spec, an unset variable with no + # default is replaced with an empty value (which the YAML parser then + # interprets as null); a set-but-empty variable substitutes to its + # (empty) value. + if value is None: + return "" + # Per spec: "It MUST NOT be possible to inject YAML structures by # environment variables." Newlines are the primary injection vector — # a value like "legit\nmalicious_key: val" would create extra YAML diff --git a/opentelemetry-configuration/tests/file/test_env_substitution.py b/opentelemetry-configuration/tests/file/test_env_substitution.py index ff52b9d704..f52d8ac80d 100644 --- a/opentelemetry-configuration/tests/file/test_env_substitution.py +++ b/opentelemetry-configuration/tests/file/test_env_substitution.py @@ -37,6 +37,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_substitutes_empty(self): + """An unset ${env:VAR} without a default is replaced with an empty value.""" + with patch.dict(os.environ, {}, clear=True): + result = substitute_env_vars("name: ${env:MISSING_VAR}") + self.assertEqual(result, "name: ") + self.assertIsNone(yaml.safe_load(result)["name"]) + + 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_without_default_substitutes_empty(self): """An unset ${VAR} without a default is replaced with an empty value. diff --git a/opentelemetry-configuration/tests/test_sdk.py b/opentelemetry-configuration/tests/test_sdk.py index 6a6d2cf1be..adcd4b1171 100644 --- a/opentelemetry-configuration/tests/test_sdk.py +++ b/opentelemetry-configuration/tests/test_sdk.py @@ -9,6 +9,9 @@ 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, @@ -70,7 +73,7 @@ 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) @@ -214,5 +217,43 @@ def test_applies_tracer_provider_globally(self, mock_set_tracer): configure_sdk(config) - mock_set_tracer.assert_called_once() self.assertIsInstance(mock_set_tracer.call_args[0][0], SdkTracerProvider) + + +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)