Skip to content

Commit

Permalink
change the way ClientTelemetry is initialized
Browse files Browse the repository at this point in the history
Signed-off-by: Teo <[email protected]>
  • Loading branch information
teocns committed Jan 7, 2025
1 parent 8cde6d9 commit 42adc09
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 23 deletions.
41 changes: 29 additions & 12 deletions agentops/telemetry/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,33 +30,50 @@ def __init__(self,client: "Client"):

def initialize(self, config: OTELConfig) -> None:
"""Initialize telemetry components"""
# Check for environment variables if no exporters configured
if not config.additional_exporters:
# Create a deep copy of the config
config_copy = OTELConfig(
additional_exporters=list(config.additional_exporters) if config.additional_exporters else None,
resource_attributes=dict(config.resource_attributes) if config.resource_attributes else None,
sampler=config.sampler,
retry_config=dict(config.retry_config) if config.retry_config else None,
custom_formatters=list(config.custom_formatters) if config.custom_formatters else None,
enable_metrics=config.enable_metrics,
metric_readers=list(config.metric_readers) if config.metric_readers else None,
enable_in_flight=config.enable_in_flight,
in_flight_interval=config.in_flight_interval,
max_queue_size=config.max_queue_size,
max_wait_time=config.max_wait_time,
endpoint=config.endpoint,
api_key=config.api_key
)

# Only check environment variables if no exporters are explicitly configured
if config_copy.additional_exporters is None:
endpoint = os.environ.get("OTEL_EXPORTER_OTLP_ENDPOINT")
service_name = os.environ.get("OTEL_SERVICE_NAME")

if service_name and not config.resource_attributes:
config.resource_attributes = {"service.name": service_name}
if service_name and not config_copy.resource_attributes:
config_copy.resource_attributes = {"service.name": service_name}

if endpoint:
from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter
config.additional_exporters = [OTLPSpanExporter(endpoint=endpoint)]
config_copy.additional_exporters = [OTLPSpanExporter(endpoint=endpoint)]
logger.info("Using OTEL configuration from environment variables")

# Validate exporters
if config.additional_exporters:
for exporter in config.additional_exporters:
if config_copy.additional_exporters:
for exporter in config_copy.additional_exporters:
if not isinstance(exporter, SpanExporter):
raise ValueError(f"Invalid exporter type: {type(exporter)}. Must be a SpanExporter")

# Create the OTEL manager instance
self._otel_manager = OTELManager(
config=config,
exporters=config.additional_exporters,
resource_attributes=config.resource_attributes,
sampler=config.sampler
config=config_copy,
exporters=config_copy.additional_exporters,
resource_attributes=config_copy.resource_attributes,
sampler=config_copy.sampler
)
self.config = config
self.config = config_copy

# Initialize the tracer provider with global service info
self._tracer_provider = self._otel_manager.initialize(
Expand Down
43 changes: 32 additions & 11 deletions tests/telemetry/test_telemetry_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ def test_configuration_with_otel():
config.configure(None, telemetry=otel_config)

assert config.telemetry == otel_config
assert config.telemetry.additional_exporters == [exporter]
assert len(config.telemetry.additional_exporters) == 1
assert isinstance(config.telemetry.additional_exporters[0], OTLPSpanExporter)


def test_init_accepts_telemetry_config():
Expand All @@ -34,9 +35,9 @@ def test_init_accepts_telemetry_config():
telemetry=telemetry
)

# Verify exporter was configured
client = agentops.Client()
assert client.telemetry.config.additional_exporters == [exporter]
assert len(client.telemetry.config.additional_exporters) == 1
assert isinstance(client.telemetry.config.additional_exporters[0], OTLPSpanExporter)


def test_init_with_env_var_endpoint(monkeypatch, instrumentation):
Expand All @@ -52,8 +53,9 @@ def test_init_with_env_var_endpoint(monkeypatch, instrumentation):
telemetry.initialize(config)

# Check the exporters were configured correctly
assert config.additional_exporters is not None
assert len(config.additional_exporters) == 1
assert config.additional_exporters is None # Original config should be unchanged
assert telemetry.config.additional_exporters is not None # New config should have exporters
assert len(telemetry.config.additional_exporters) == 1

# Create a test span
tracer = instrumentation.tracer_provider.get_tracer(__name__)
Expand All @@ -70,21 +72,40 @@ def test_init_with_env_var_endpoint(monkeypatch, instrumentation):
telemetry.shutdown()


@pytest.mark.skip
def test_telemetry_config_overrides_env_vars(instrumentation):
"""Test that explicit telemetry config takes precedence over env vars"""
custom_exporter = InMemorySpanExporter()
telemetry = OTELConfig(additional_exporters=[custom_exporter])
telemetry_config = OTELConfig(additional_exporters=[custom_exporter])

with patch('os.environ.get') as mock_env:
mock_env.return_value = "http://fromenv:4317"

# Create a mock environment getter that handles default values correctly
env_vars = {
"OTEL_EXPORTER_OTLP_ENDPOINT": "http://fromenv:4317",
"OTEL_SERVICE_NAME": "test-service",
"AGENTOPS_LOGGING_LEVEL": "INFO", # Add this to handle the logging level check
"AGENTOPS_API_KEY": None,
"AGENTOPS_PARENT_KEY": None,
"AGENTOPS_API_ENDPOINT": None,
"AGENTOPS_ENV_DATA_OPT_OUT": None
}
def mock_env_get(key, default=None):
return env_vars.get(key, default)

# Need to patch both os.environ.get and os.getenv
with patch('os.environ.get', side_effect=mock_env_get), \
patch('os.getenv', side_effect=mock_env_get):
# Initialize with our custom config
agentops.init(
api_key="test-key",
telemetry=telemetry
telemetry=telemetry_config
)

client = agentops.Client()
assert client.telemetry.config.additional_exporters == [custom_exporter]
# Verify we're using our custom exporter
assert len(client.telemetry.config.additional_exporters) == 1
assert isinstance(client.telemetry.config.additional_exporters[0], InMemorySpanExporter)
# Verify we're not using the environment variable
assert not isinstance(client.telemetry.config.additional_exporters[0], OTLPSpanExporter)


def test_multiple_exporters_in_config():
Expand Down

0 comments on commit 42adc09

Please sign in to comment.