mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-01-09 08:08:32 +00:00
Fix telemetry singleton pattern to respect dynamic environment variables
- Modified Telemetry.__init__ to prevent re-initialization with _initialized flag - Updated _safe_telemetry_operation to check _is_telemetry_disabled() dynamically - Added comprehensive tests for environment variables set after singleton creation - Fixed singleton contamination in existing tests by adding proper reset - Resolves issue #2945 where CREWAI_DISABLE_TELEMETRY=true was ignored when set after import Co-Authored-By: João <joao@crewai.com>
This commit is contained in:
@@ -76,10 +76,14 @@ class Telemetry:
|
|||||||
return cls._instance
|
return cls._instance
|
||||||
|
|
||||||
def __init__(self) -> None:
|
def __init__(self) -> None:
|
||||||
|
if hasattr(self, '_initialized'):
|
||||||
|
return
|
||||||
|
|
||||||
self.ready: bool = False
|
self.ready: bool = False
|
||||||
self.trace_set: bool = False
|
self.trace_set: bool = False
|
||||||
|
|
||||||
if self._is_telemetry_disabled():
|
if self._is_telemetry_disabled():
|
||||||
|
self._initialized: bool = True
|
||||||
return
|
return
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -105,6 +109,8 @@ class Telemetry:
|
|||||||
):
|
):
|
||||||
raise # Re-raise the exception to not interfere with system signals
|
raise # Re-raise the exception to not interfere with system signals
|
||||||
self.ready = False
|
self.ready = False
|
||||||
|
finally:
|
||||||
|
self._initialized = True
|
||||||
|
|
||||||
def _is_telemetry_disabled(self) -> bool:
|
def _is_telemetry_disabled(self) -> bool:
|
||||||
"""Check if telemetry should be disabled based on environment variables."""
|
"""Check if telemetry should be disabled based on environment variables."""
|
||||||
@@ -124,7 +130,7 @@ class Telemetry:
|
|||||||
self.trace_set = False
|
self.trace_set = False
|
||||||
|
|
||||||
def _safe_telemetry_operation(self, operation):
|
def _safe_telemetry_operation(self, operation):
|
||||||
if not self.ready:
|
if not self.ready or self._is_telemetry_disabled():
|
||||||
return
|
return
|
||||||
try:
|
try:
|
||||||
operation()
|
operation()
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ from opentelemetry import trace
|
|||||||
)
|
)
|
||||||
def test_telemetry_environment_variables(env_var, value, expected_ready):
|
def test_telemetry_environment_variables(env_var, value, expected_ready):
|
||||||
"""Test telemetry state with different environment variable configurations."""
|
"""Test telemetry state with different environment variable configurations."""
|
||||||
|
Telemetry._instance = None
|
||||||
with patch.dict(os.environ, {env_var: value}):
|
with patch.dict(os.environ, {env_var: value}):
|
||||||
with patch("crewai.telemetry.telemetry.TracerProvider"):
|
with patch("crewai.telemetry.telemetry.TracerProvider"):
|
||||||
telemetry = Telemetry()
|
telemetry = Telemetry()
|
||||||
@@ -30,6 +31,7 @@ def test_telemetry_environment_variables(env_var, value, expected_ready):
|
|||||||
|
|
||||||
def test_telemetry_enabled_by_default():
|
def test_telemetry_enabled_by_default():
|
||||||
"""Test that telemetry is enabled by default."""
|
"""Test that telemetry is enabled by default."""
|
||||||
|
Telemetry._instance = None
|
||||||
with patch.dict(os.environ, {}, clear=True):
|
with patch.dict(os.environ, {}, clear=True):
|
||||||
with patch("crewai.telemetry.telemetry.TracerProvider"):
|
with patch("crewai.telemetry.telemetry.TracerProvider"):
|
||||||
telemetry = Telemetry()
|
telemetry = Telemetry()
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import os
|
import os
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch, MagicMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
@@ -16,6 +16,7 @@ from crewai.telemetry import Telemetry
|
|||||||
])
|
])
|
||||||
def test_telemetry_environment_variables(env_var, value, expected_ready):
|
def test_telemetry_environment_variables(env_var, value, expected_ready):
|
||||||
"""Test telemetry state with different environment variable configurations."""
|
"""Test telemetry state with different environment variable configurations."""
|
||||||
|
Telemetry._instance = None
|
||||||
with patch.dict(os.environ, {env_var: value}):
|
with patch.dict(os.environ, {env_var: value}):
|
||||||
with patch("crewai.telemetry.telemetry.TracerProvider"):
|
with patch("crewai.telemetry.telemetry.TracerProvider"):
|
||||||
telemetry = Telemetry()
|
telemetry = Telemetry()
|
||||||
@@ -24,7 +25,70 @@ def test_telemetry_environment_variables(env_var, value, expected_ready):
|
|||||||
|
|
||||||
def test_telemetry_enabled_by_default():
|
def test_telemetry_enabled_by_default():
|
||||||
"""Test that telemetry is enabled by default."""
|
"""Test that telemetry is enabled by default."""
|
||||||
|
Telemetry._instance = None
|
||||||
with patch.dict(os.environ, {}, clear=True):
|
with patch.dict(os.environ, {}, clear=True):
|
||||||
with patch("crewai.telemetry.telemetry.TracerProvider"):
|
with patch("crewai.telemetry.telemetry.TracerProvider"):
|
||||||
telemetry = Telemetry()
|
telemetry = Telemetry()
|
||||||
assert telemetry.ready is True
|
assert telemetry.ready is True
|
||||||
|
|
||||||
|
|
||||||
|
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()
|
||||||
|
assert telemetry.ready is True
|
||||||
|
|
||||||
|
mock_operation = MagicMock()
|
||||||
|
telemetry._safe_telemetry_operation(mock_operation)
|
||||||
|
mock_operation.assert_called_once()
|
||||||
|
|
||||||
|
mock_operation.reset_mock()
|
||||||
|
|
||||||
|
os.environ['CREWAI_DISABLE_TELEMETRY'] = 'true'
|
||||||
|
|
||||||
|
telemetry._safe_telemetry_operation(mock_operation)
|
||||||
|
mock_operation.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
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()
|
||||||
|
assert telemetry1.ready is True
|
||||||
|
|
||||||
|
os.environ['CREWAI_DISABLE_TELEMETRY'] = 'true'
|
||||||
|
|
||||||
|
telemetry2 = Telemetry()
|
||||||
|
assert telemetry2 is telemetry1
|
||||||
|
assert telemetry2.ready is True
|
||||||
|
|
||||||
|
mock_operation = MagicMock()
|
||||||
|
telemetry2._safe_telemetry_operation(mock_operation)
|
||||||
|
mock_operation.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
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()
|
||||||
|
assert telemetry.ready is True
|
||||||
|
|
||||||
|
mock_operation = MagicMock()
|
||||||
|
telemetry._safe_telemetry_operation(mock_operation)
|
||||||
|
mock_operation.assert_called_once()
|
||||||
|
|
||||||
|
mock_operation.reset_mock()
|
||||||
|
|
||||||
|
os.environ['OTEL_SDK_DISABLED'] = 'true'
|
||||||
|
|
||||||
|
telemetry._safe_telemetry_operation(mock_operation)
|
||||||
|
mock_operation.assert_not_called()
|
||||||
|
|||||||
Reference in New Issue
Block a user