fix: error raised for env checks in LogLimits and SpanLimits

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.

Author: jomcgi

CHANGELOG.md

@@ -26,6 +26,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,53 @@ def test_log_limits_max_attribute_length(self):
         limits = LogLimits(max_attribute_length=1)
 
         self.assertEqual(expected, limits.max_attribute_length)
+
+    def test_non_integer_env_vars_raise(self):
+        test_cases = [
+            (env_val, "bad")
+            for env_val in [
+                OTEL_ATTRIBUTE_COUNT_LIMIT,
+                OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
+            ]
+        ]
+
+        for env_var, bad_value in test_cases:
+            with self.subTest(env_var=env_var, value=bad_value):
+                env = {env_var: bad_value}
+
+                with self.assertRaises(ValueError) as error, patch.dict(
+                    "os.environ", env
+                ):
+                    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}",
+                )
+
+    def test_negative_integer_env_vars_raise(self):
+        test_cases = [
+            (env_val, "-1")
+            for env_val in [
+                OTEL_ATTRIBUTE_COUNT_LIMIT,
+                OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
+            ]
+        ]
+
+        for env_var, invalid_value in test_cases:
+            with self.subTest(env_var=env_var, value=invalid_value):
+                env = {env_var: invalid_value}
+
+                with self.assertRaises(ValueError) as error, patch.dict(
+                    "os.environ", env
+                ):
+                    LogLimits()
+
+                expected_msg = f"{env_var} must be a non-negative integer but got {invalid_value}"
+                self.assertEqual(
+                    expected_msg,
+                    str(error.exception),
+                    f"Unexpected error message for {env_var}={invalid_value}",
+                )

opentelemetry-sdk/tests/trace/test_span_limits.py

@@ -0,0 +1,91 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import unittest
+from unittest.mock import patch
+
+from opentelemetry.sdk.environment_variables import (
+    OTEL_ATTRIBUTE_COUNT_LIMIT,
+    OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
+    OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT,
+    OTEL_LINK_ATTRIBUTE_COUNT_LIMIT,
+    OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT,
+    OTEL_SPAN_EVENT_COUNT_LIMIT,
+    OTEL_SPAN_LINK_COUNT_LIMIT,
+)
+from opentelemetry.sdk.trace import (
+    SpanLimits,
+)
+
+
+class TestSpanLimits(unittest.TestCase):
+    def test_non_integer_env_vars_raise(self):
+        test_cases = [
+            (env_val, "bad")
+            for env_val in [
+                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,
+            ]
+        ]
+
+        for env_var, bad_value in test_cases:
+            with self.subTest(env_var=env_var, value=bad_value):
+                env = {env_var: bad_value}
+
+                with self.assertRaises(ValueError) as error, patch.dict(
+                    "os.environ", env
+                ):
+                    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}",
+                )
+
+    def test_negative_integer_env_vars_raise(self):
+        test_cases = [
+            (env_val, "-1")
+            for env_val in [
+                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,
+            ]
+        ]
+
+        for env_var, invalid_value in test_cases:
+            with self.subTest(env_var=env_var, value=invalid_value):
+                env = {env_var: invalid_value}
+
+                with self.assertRaises(ValueError) as error, patch.dict(
+                    "os.environ", env
+                ):
+                    SpanLimits()
+
+                expected_msg = f"{env_var} must be a non-negative integer but got {invalid_value}"
+                self.assertEqual(
+                    expected_msg,
+                    str(error.exception),
+                    f"Unexpected error message for {env_var}={invalid_value}",
+                )