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

@@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#4402](https://github.com/open-telemetry/opentelemetry-python/pull/4402))
 - Tolerates exceptions when loading resource detectors via `OTEL_EXPERIMENTAL_RESOURCE_DETECTORS`
   ([#4373](https://github.com/open-telemetry/opentelemetry-python/pull/4373))
+- Disconnect gRPC client stub when shutting down `OTLPSpanExporter`
+  ([#4370](https://github.com/open-telemetry/opentelemetry-python/pull/4370))
 - opentelemetry-sdk: fix OTLP exporting of Histograms with explicit buckets advisory
   ([#4434](https://github.com/open-telemetry/opentelemetry-python/pull/4434))
 - opentelemetry-exporter-otlp-proto-grpc: better dependency version range for Python 3.13
@@ -24,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)
 

exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py

@@ -243,8 +243,8 @@ def __init__(
         ) or Compression.NoCompression
 
         if insecure:
-            self._client = self._stub(
-                insecure_channel(self._endpoint, compression=compression)
+            self._channel = insecure_channel(
+                self._endpoint, compression=compression
             )
         else:
             credentials = _get_credentials(
@@ -253,11 +253,10 @@ def __init__(
                 OTEL_EXPORTER_OTLP_CLIENT_KEY,
                 OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,
             )
-            self._client = self._stub(
-                secure_channel(
-                    self._endpoint, credentials, compression=compression
-                )
+            self._channel = secure_channel(
+                self._endpoint, credentials, compression=compression
             )
+        self._client = self._stub(self._channel)
 
         self._export_lock = threading.Lock()
         self._shutdown = False
@@ -360,6 +359,7 @@ def shutdown(self, timeout_millis: float = 30_000, **kwargs) -> None:
         # wait for the last export if any
         self._export_lock.acquire(timeout=timeout_millis / 1e3)
         self._shutdown = True
+        self._channel.close()
         self._export_lock.release()
 
     @property

exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_metrics_exporter.py

@@ -874,6 +874,21 @@ def test_shutdown_wait_last_export(self):
         finally:
             export_thread.join()
 
+    def test_export_over_closed_grpc_channel(self):
+        # pylint: disable=protected-access
+
+        add_MetricsServiceServicer_to_server(
+            MetricsServiceServicerSUCCESS(), self.server
+        )
+        self.exporter.export(self.metrics["sum_int"])
+        self.exporter.shutdown()
+        data = self.exporter._translate_data(self.metrics["sum_int"])
+        with self.assertRaises(ValueError) as err:
+            self.exporter._client.Export(request=data)
+        self.assertEqual(
+            str(err.exception), "Cannot invoke RPC on closed channel!"
+        )
+
     def test_aggregation_temporality(self):
         # pylint: disable=protected-access
 

exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py

@@ -1017,6 +1017,21 @@ def test_shutdown_wait_last_export(self):
         finally:
             export_thread.join()
 
+    def test_export_over_closed_grpc_channel(self):
+        # pylint: disable=protected-access
+
+        add_TraceServiceServicer_to_server(
+            TraceServiceServicerSUCCESS(), self.server
+        )
+        self.exporter.export([self.span])
+        self.exporter.shutdown()
+        data = self.exporter._translate_data([self.span])
+        with self.assertRaises(ValueError) as err:
+            self.exporter._client.Export(request=data)
+        self.assertEqual(
+            str(err.exception), "Cannot invoke RPC on closed channel!"
+        )
+
 
 def _create_span_with_status(status: SDKStatus):
     span = _Span(

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):