fix(telemetry): correct environment variable precedence for OTEL config (#86)

* fix(telemetry): correct environment variable precedence for OTEL configuration

* fix(telemetry): update get_tracer function to adapt the changes

* fix(telemetry): fix tracer initialization

---------

Co-authored-by: Jack Yuan <[email protected]>

Author: JackYPCOnline

src/strands/telemetry/tracer.py

@@ -93,7 +93,7 @@ def __init__(
         service_name: str = "strands-agents",
         otlp_endpoint: Optional[str] = None,
         otlp_headers: Optional[Dict[str, str]] = None,
-        enable_console_export: bool = False,
+        enable_console_export: Optional[bool] = None,
     ):
         """Initialize the tracer.
 
@@ -105,13 +105,17 @@ def __init__(
         """
         # Check environment variables first
         env_endpoint = os.environ.get("OTEL_EXPORTER_OTLP_ENDPOINT")
-        env_console_export = os.environ.get("STRANDS_OTEL_ENABLE_CONSOLE_EXPORT", "").lower() in ("true", "1", "yes")
+        env_console_export_str = os.environ.get("STRANDS_OTEL_ENABLE_CONSOLE_EXPORT")
 
-        # Environment variables take precedence over constructor parameters
-        if env_endpoint:
-            otlp_endpoint = env_endpoint
-        if env_console_export:
-            enable_console_export = True
+        # Constructor parameters take precedence over environment variables
+        self.otlp_endpoint = otlp_endpoint or env_endpoint
+
+        if enable_console_export is not None:
+            self.enable_console_export = enable_console_export
+        elif env_console_export_str:
+            self.enable_console_export = env_console_export_str.lower() in ("true", "1", "yes")
+        else:
+            self.enable_console_export = False
 
         # Parse headers from environment if available
         env_headers = os.environ.get("OTEL_EXPORTER_OTLP_HEADERS")
@@ -128,14 +132,11 @@ def __init__(
                 logger.warning("error=<%s> | failed to parse OTEL_EXPORTER_OTLP_HEADERS", e)
 
         self.service_name = service_name
-        self.otlp_endpoint = otlp_endpoint
         self.otlp_headers = otlp_headers or {}
-        self.enable_console_export = enable_console_export
-
         self.tracer_provider: Optional[TracerProvider] = None
         self.tracer: Optional[trace.Tracer] = None
 
-        if otlp_endpoint or enable_console_export:
+        if self.otlp_endpoint or self.enable_console_export:
             self._initialize_tracer()
 
     def _initialize_tracer(self) -> None:
@@ -547,7 +548,7 @@ def get_tracer(
     service_name: str = "strands-agents",
     otlp_endpoint: Optional[str] = None,
     otlp_headers: Optional[Dict[str, str]] = None,
-    enable_console_export: bool = False,
+    enable_console_export: Optional[bool] = None,
 ) -> Tracer:
     """Get or create the global tracer.
 

tests/strands/telemetry/test_tracer.py

@@ -60,6 +60,50 @@ def mock_resource():
         yield mock_resource
 
 
+@pytest.fixture
+def clean_env():
+    """Fixture to provide a clean environment for each test."""
+    with mock.patch.dict(os.environ, {}, clear=True):
+        yield
+
+
+@pytest.fixture
+def env_with_otlp():
+    """Fixture with OTLP environment variables."""
+    with mock.patch.dict(
+        os.environ,
+        {
+            "OTEL_EXPORTER_OTLP_ENDPOINT": "http://env-endpoint",
+        },
+    ):
+        yield
+
+
+@pytest.fixture
+def env_with_console():
+    """Fixture with console export environment variables."""
+    with mock.patch.dict(
+        os.environ,
+        {
+            "STRANDS_OTEL_ENABLE_CONSOLE_EXPORT": "true",
+        },
+    ):
+        yield
+
+
+@pytest.fixture
+def env_with_both():
+    """Fixture with both OTLP and console export environment variables."""
+    with mock.patch.dict(
+        os.environ,
+        {
+            "OTEL_EXPORTER_OTLP_ENDPOINT": "http://env-endpoint",
+            "STRANDS_OTEL_ENABLE_CONSOLE_EXPORT": "true",
+        },
+    ):
+        yield
+
+
 def test_init_default():
     """Test initializing the Tracer with default parameters."""
     tracer = Tracer()
@@ -681,3 +725,50 @@ def test_serialize_vs_json_dumps():
     custom_result = serialize({"text": japanese_text})
     assert japanese_text in custom_result
     assert "\\u" not in custom_result
+
+
+def test_init_with_no_env_or_param(clean_env):
+    """Test initializing with neither environment variable nor constructor parameter."""
+    tracer = Tracer()
+    assert tracer.otlp_endpoint is None
+    assert tracer.enable_console_export is False
+
+    tracer = Tracer(otlp_endpoint="http://param-endpoint")
+    assert tracer.otlp_endpoint == "http://param-endpoint"
+
+    tracer = Tracer(enable_console_export=True)
+    assert tracer.enable_console_export is True
+
+
+def test_constructor_params_with_otlp_env(env_with_otlp):
+    """Test constructor parameters precedence over OTLP environment variable."""
+    # Constructor parameter should take precedence
+    tracer = Tracer(otlp_endpoint="http://constructor-endpoint")
+    assert tracer.otlp_endpoint == "http://constructor-endpoint"
+
+    # Without constructor parameter, should use env var
+    tracer = Tracer()
+    assert tracer.otlp_endpoint == "http://env-endpoint"
+
+
+def test_constructor_params_with_console_env(env_with_console):
+    """Test constructor parameters precedence over console environment variable."""
+    # Constructor parameter should take precedence
+    tracer = Tracer(enable_console_export=False)
+    assert tracer.enable_console_export is False
+
+    # Without explicit constructor parameter, should use env var
+    tracer = Tracer()
+    assert tracer.enable_console_export is True
+
+
+def test_fallback_to_env_vars(env_with_both):
+    """Test fallback to environment variables when no constructor parameters."""
+    tracer = Tracer()
+    assert tracer.otlp_endpoint == "http://env-endpoint"
+    assert tracer.enable_console_export is True
+
+    # Constructor parameters should still take precedence
+    tracer = Tracer(otlp_endpoint="http://constructor-endpoint", enable_console_export=False)
+    assert tracer.otlp_endpoint == "http://constructor-endpoint"
+    assert tracer.enable_console_export is False