Fix TracerProvider() crash on out-of-range OTEL_TRACES_SAMPLER_ARG (#5594)

* Fix TracerProvider() crash on out-of-range OTEL_TRACES_SAMPLER_ARG

_get_from_env_or_default() already caught ValueError/TypeError from
float(OTEL_TRACES_SAMPLER_ARG) to gracefully fall back to rate=1.0 with
a warning when the value is non-numeric. But TraceIdRatioBased.__init__
raises its own ValueError when the parsed rate is syntactically valid
yet outside [0.0, 1.0], and that exception was not covered by the same
guard, so TracerProvider() construction crashed instead of degrading
gracefully.

Per the SDK configuration spec, invalid OTEL_TRACES_SAMPLER_ARG values
MUST be logged and otherwise ignored, behaving as if the variable were
not set. An out-of-range numeric value is just as invalid as a
non-numeric one and should be handled the same way.

Moves the sampler construction inside the try block so both failure
modes share the existing fallback path.

* Rename 0.fixed to 5594.fixed

* Expand fallback warning to mention the rate that will be used

Per review feedback from @ocelotl: mention the fallback rate (1.0) in
the warning message so it's clear what value the sampler actually ends
up using when OTEL_TRACES_SAMPLER_ARG is invalid.

---------

Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>

Author: piyush182004

.changelog/5594.fixed

@@ -0,0 +1 @@
+`opentelemetry-sdk`: fix `TracerProvider()` raising `ValueError` when `OTEL_TRACES_SAMPLER_ARG` is a syntactically valid number outside the `[0.0, 1.0]` range, instead of logging a warning and falling back like other invalid values

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

@@ -471,10 +471,10 @@ def _get_from_env_or_default() -> Sampler:
     if trace_sampler in ("traceidratio", "parentbased_traceidratio"):
         try:
             rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG, ""))
+            return _KNOWN_SAMPLERS[trace_sampler](rate)
         except (ValueError, TypeError):
-            _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.")
-            rate = 1.0
-        return _KNOWN_SAMPLERS[trace_sampler](rate)
+            _logger.warning("Could not convert TRACES_SAMPLER_ARG to float. Using default rate 1.0.")
+            return _KNOWN_SAMPLERS[trace_sampler](1.0)
 
     return _KNOWN_SAMPLERS[trace_sampler]
 

opentelemetry-sdk/tests/trace/test_trace.py

@@ -51,6 +51,7 @@
     Decision,
     ParentBased,
     StaticSampler,
+    TraceIdRatioBased,
 )
 from opentelemetry.sdk.util import BoundedDict, BoundedList, ns_to_iso_str
 from opentelemetry.sdk.util.instrumentation import (
@@ -353,6 +354,68 @@ def test_ratio_sampler_with_env(self):
         self.assertIsInstance(tracer_provider.sampler, ParentBased)
         self.assertEqual(tracer_provider.sampler._root.rate, 0.25)
 
+    @mock.patch.dict(
+        "os.environ",
+        {
+            OTEL_TRACES_SAMPLER: "traceidratio",
+            OTEL_TRACES_SAMPLER_ARG: "5.0",
+        },
+    )
+    def test_ratio_sampler_with_out_of_range_env_arg_does_not_raise(self):
+        # A syntactically valid but out-of-range OTEL_TRACES_SAMPLER_ARG
+        # (outside [0.0, 1.0]) must not crash TracerProvider() construction.
+        # It should degrade gracefully, the same way a non-numeric value
+        # already does.
+        # pylint: disable=protected-access
+        reload(trace)
+        tracer_provider = trace.TracerProvider()
+        self.assertIsInstance(tracer_provider.sampler, TraceIdRatioBased)
+        self.assertEqual(tracer_provider.sampler.rate, 1.0)
+
+    @mock.patch.dict(
+        "os.environ",
+        {
+            OTEL_TRACES_SAMPLER: "traceidratio",
+            OTEL_TRACES_SAMPLER_ARG: "-0.5",
+        },
+    )
+    def test_ratio_sampler_with_negative_env_arg_does_not_raise(self):
+        # pylint: disable=protected-access
+        reload(trace)
+        tracer_provider = trace.TracerProvider()
+        self.assertIsInstance(tracer_provider.sampler, TraceIdRatioBased)
+        self.assertEqual(tracer_provider.sampler.rate, 1.0)
+
+    @mock.patch.dict(
+        "os.environ",
+        {
+            OTEL_TRACES_SAMPLER: "traceidratio",
+            OTEL_TRACES_SAMPLER_ARG: "0.0",
+        },
+    )
+    def test_ratio_sampler_with_env_arg_lower_boundary(self):
+        # 0.0 is a valid boundary value and must be used as-is, not
+        # treated as falsy/invalid and overridden by the fallback.
+        # pylint: disable=protected-access
+        reload(trace)
+        tracer_provider = trace.TracerProvider()
+        self.assertIsInstance(tracer_provider.sampler, TraceIdRatioBased)
+        self.assertEqual(tracer_provider.sampler.rate, 0.0)
+
+    @mock.patch.dict(
+        "os.environ",
+        {
+            OTEL_TRACES_SAMPLER: "traceidratio",
+            OTEL_TRACES_SAMPLER_ARG: "1.0",
+        },
+    )
+    def test_ratio_sampler_with_env_arg_upper_boundary(self):
+        # pylint: disable=protected-access
+        reload(trace)
+        tracer_provider = trace.TracerProvider()
+        self.assertIsInstance(tracer_provider.sampler, TraceIdRatioBased)
+        self.assertEqual(tracer_provider.sampler.rate, 1.0)
+
     def verify_default_sampler(self, tracer_provider):
         self.assertIsInstance(tracer_provider.sampler, ParentBased)
         # pylint: disable=protected-access