fix: error raised for env checks in LogLimits and SpanLimits (open-telemetry#4458)

Current implementation raises a value error trying to create the error message.
Updated error message format to use positional instead of named params.
Added test cases to validate the correct errors are raised.

Co-authored-by: Riccardo Magliocchetti <[email protected]>

Author: jomcgi

CHANGELOG.md

@@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#4448](https://github.com/open-telemetry/opentelemetry-python/pull/4448))
 - Make `trace_api.use_span()` record `BaseException` as well as `Exception`
   ([#4406](https://github.com/open-telemetry/opentelemetry-python/pull/4406))
+- Fix env var error message for TraceLimits/SpanLimits
+  ([#4458](https://github.com/open-telemetry/opentelemetry-python/pull/4458))
 
 ## Version 1.30.0/0.51b0 (2025-02-03)
 

opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py

@@ -134,7 +134,7 @@ def _from_env_if_absent(
         if value == cls.UNSET:
             return None
 
-        err_msg = "{0} must be a non-negative integer but got {}"
+        err_msg = "{} must be a non-negative integer but got {}"
 
         # if no value is provided for the limit, try to load it from env
         if value is None:

opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

@@ -692,7 +692,7 @@ def _from_env_if_absent(
         if value == cls.UNSET:
             return None
 
-        err_msg = "{0} must be a non-negative integer but got {}"
+        err_msg = "{} must be a non-negative integer but got {}"
 
         # if no value is provided for the limit, try to load it from env
         if value is None:

opentelemetry-sdk/tests/logs/test_log_limits.py

@@ -13,11 +13,16 @@
 # limitations under the License.
 
 import unittest
+from unittest.mock import patch
 
 from opentelemetry.sdk._logs import LogLimits
 from opentelemetry.sdk._logs._internal import (
     _DEFAULT_OTEL_ATTRIBUTE_COUNT_LIMIT,
 )
+from opentelemetry.sdk.environment_variables import (
+    OTEL_ATTRIBUTE_COUNT_LIMIT,
+    OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
+)
 
 
 class TestLogLimits(unittest.TestCase):
@@ -38,3 +43,30 @@ def test_log_limits_max_attribute_length(self):
         limits = LogLimits(max_attribute_length=1)
 
         self.assertEqual(expected, limits.max_attribute_length)
+
+    def test_invalid_env_vars_raise(self):
+        env_vars = [
+            OTEL_ATTRIBUTE_COUNT_LIMIT,
+            OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
+        ]
+
+        bad_values = ["bad", "-1"]
+        test_cases = {
+            env_var: bad_value
+            for env_var in env_vars
+            for bad_value in bad_values
+        }
+
+        for env_var, bad_value in test_cases.items():
+            with self.subTest(f"Testing {env_var}={bad_value}"):
+                with self.assertRaises(ValueError) as error, patch.dict(
+                    "os.environ", {env_var: bad_value}, clear=True
+                ):
+                    LogLimits()
+
+                expected_msg = f"{env_var} must be a non-negative integer but got {bad_value}"
+                self.assertEqual(
+                    expected_msg,
+                    str(error.exception),
+                    f"Unexpected error message for {env_var}={bad_value}",
+                )

opentelemetry-sdk/tests/trace/test_trace.py

@@ -2007,6 +2007,37 @@ def _test_span_no_limits(self, tracer):
             for attr_val in root.attributes.values():
                 self.assertEqual(attr_val, self.long_val)
 
+    def test_invalid_env_vars_raise(self):
+        env_vars = [
+            OTEL_SPAN_EVENT_COUNT_LIMIT,
+            OTEL_SPAN_LINK_COUNT_LIMIT,
+            OTEL_ATTRIBUTE_COUNT_LIMIT,
+            OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT,
+            OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT,
+            OTEL_LINK_ATTRIBUTE_COUNT_LIMIT,
+            OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
+        ]
+        bad_values = ["bad", "-1"]
+        test_cases = {
+            env_var: bad_value
+            for env_var in env_vars
+            for bad_value in bad_values
+        }
+
+        for env_var, bad_value in test_cases.items():
+            with self.subTest(f"Testing {env_var}={bad_value}"):
+                with self.assertRaises(ValueError) as error, patch.dict(
+                    "os.environ", {env_var: bad_value}, clear=True
+                ):
+                    trace.SpanLimits()
+
+                expected_msg = f"{env_var} must be a non-negative integer but got {bad_value}"
+                self.assertEqual(
+                    expected_msg,
+                    str(error.exception),
+                    f"Unexpected error message for {env_var}={bad_value}",
+                )
+
 
 class TestTraceFlags(unittest.TestCase):
     def test_constant_default(self):