Implement code review improvements

- Move _initialized flag to __new__ method for better encapsulation
- Add type hints to _safe_telemetry_operation method
- Consolidate telemetry execution checks into _should_execute_telemetry helper
- Add pytest fixtures to reduce test setup redundancy
- Enhanced documentation for singleton behavior

Co-Authored-By: João <joao@crewai.com>

src/crewai/telemetry/telemetry.py

@@ -8,7 +8,7 @@ import platform
 import warnings
 from contextlib import contextmanager
 from importlib.metadata import version
-from typing import TYPE_CHECKING, Any, Optional
+from typing import TYPE_CHECKING, Any, Callable, Optional
 import threading
 
 from opentelemetry import trace
@@ -73,17 +73,18 @@ class Telemetry:
             with cls._lock:
                 if cls._instance is None:
                     cls._instance = super(Telemetry, cls).__new__(cls)
+                    cls._instance._initialized = False
         return cls._instance
 
     def __init__(self) -> None:
-        if hasattr(self, '_initialized'):
+        if self._initialized:
             return
         
         self.ready: bool = False
         self.trace_set: bool = False
 
         if self._is_telemetry_disabled():
-            self._initialized: bool = True
+            self._initialized = True
             return
 
         try:
@@ -119,6 +120,10 @@ class Telemetry:
             or os.getenv("CREWAI_DISABLE_TELEMETRY", "false").lower() == "true"
         )
 
+    def _should_execute_telemetry(self) -> bool:
+        """Check if telemetry operations should be executed."""
+        return self.ready and not self._is_telemetry_disabled()
+
     def set_tracer(self):
         if self.ready and not self.trace_set:
             try:
@@ -129,8 +134,9 @@ class Telemetry:
                 self.ready = False
                 self.trace_set = False
 
-    def _safe_telemetry_operation(self, operation):
-        if not self.ready or self._is_telemetry_disabled():
+    def _safe_telemetry_operation(self, operation: Callable[[], None]) -> None:
+        """Execute telemetry operation safely, checking both readiness and environment variables."""
+        if not self._should_execute_telemetry():
             return
         try:
             operation()

tests/telemetry/test_telemetry.py

@@ -9,6 +9,14 @@ from crewai.telemetry import Telemetry
 from opentelemetry import trace
 
 
+@pytest.fixture(autouse=True)
+def cleanup_telemetry():
+    """Automatically clean up Telemetry singleton between tests."""
+    Telemetry._instance = None
+    yield
+    Telemetry._instance = None
+
+
 @pytest.mark.parametrize(
     "env_var,value,expected_ready",
     [
@@ -22,7 +30,6 @@ from opentelemetry import trace
 )
 def test_telemetry_environment_variables(env_var, value, expected_ready):
     """Test telemetry state with different environment variable configurations."""
-    Telemetry._instance = None
     with patch.dict(os.environ, {env_var: value}):
         with patch("crewai.telemetry.telemetry.TracerProvider"):
             telemetry = Telemetry()
@@ -31,7 +38,6 @@ def test_telemetry_environment_variables(env_var, value, expected_ready):
 
 def test_telemetry_enabled_by_default():
     """Test that telemetry is enabled by default."""
-    Telemetry._instance = None
     with patch.dict(os.environ, {}, clear=True):
         with patch("crewai.telemetry.telemetry.TracerProvider"):
             telemetry = Telemetry()

tests/telemetry/test_telemetry_disable.py

@@ -6,6 +6,14 @@ import pytest
 from crewai.telemetry import Telemetry
 
 
+@pytest.fixture(autouse=True)
+def cleanup_telemetry():
+    """Automatically clean up Telemetry singleton between tests."""
+    Telemetry._instance = None
+    yield
+    Telemetry._instance = None
+
+
 @pytest.mark.parametrize("env_var,value,expected_ready", [
     ("OTEL_SDK_DISABLED", "true", False),
     ("OTEL_SDK_DISABLED", "TRUE", False),
@@ -16,7 +24,6 @@ from crewai.telemetry import Telemetry
 ])
 def test_telemetry_environment_variables(env_var, value, expected_ready):
     """Test telemetry state with different environment variable configurations."""
-    Telemetry._instance = None
     with patch.dict(os.environ, {env_var: value}):
         with patch("crewai.telemetry.telemetry.TracerProvider"):
             telemetry = Telemetry()
@@ -25,7 +32,6 @@ def test_telemetry_environment_variables(env_var, value, expected_ready):
 
 def test_telemetry_enabled_by_default():
     """Test that telemetry is enabled by default."""
-    Telemetry._instance = None
     with patch.dict(os.environ, {}, clear=True):
         with patch("crewai.telemetry.telemetry.TracerProvider"):
             telemetry = Telemetry()
@@ -34,8 +40,6 @@ def test_telemetry_enabled_by_default():
 
 def test_telemetry_disable_after_singleton_creation():
     """Test that telemetry operations are disabled when env var is set after singleton creation."""
-    Telemetry._instance = None
-    
     with patch.dict(os.environ, {}, clear=True):
         with patch("crewai.telemetry.telemetry.TracerProvider"):
             telemetry = Telemetry()
@@ -55,8 +59,6 @@ def test_telemetry_disable_after_singleton_creation():
 
 def test_telemetry_disable_with_multiple_instances():
     """Test that multiple telemetry instances respect dynamically changed env vars."""
-    Telemetry._instance = None
-    
     with patch.dict(os.environ, {}, clear=True):
         with patch("crewai.telemetry.telemetry.TracerProvider"):
             telemetry1 = Telemetry()
@@ -75,8 +77,6 @@ def test_telemetry_disable_with_multiple_instances():
 
 def test_telemetry_otel_sdk_disabled_after_creation():
     """Test that OTEL_SDK_DISABLED also works when set after singleton creation."""
-    Telemetry._instance = None
-    
     with patch.dict(os.environ, {}, clear=True):
         with patch("crewai.telemetry.telemetry.TracerProvider"):
             telemetry = Telemetry()