From 12872cb2b10583c63d81ab3a2dc131b01ce371dd Mon Sep 17 00:00:00 2001 From: poshinchen Date: Fri, 13 Jun 2025 09:35:34 -0400 Subject: [PATCH] feat: initialized meter --- src/strands/agent/agent.py | 1 - src/strands/telemetry/__init__.py | 5 +- src/strands/telemetry/config.py | 33 +++++++++++++ src/strands/telemetry/metrics.py | 46 ++++++++++++++++++ src/strands/telemetry/metrics_constants.py | 3 ++ src/strands/telemetry/tracer.py | 15 ++---- tests/strands/telemetry/test_metrics.py | 56 ++++++++++++++++++++++ tests/strands/telemetry/test_tracer.py | 24 ++++------ 8 files changed, 153 insertions(+), 30 deletions(-) create mode 100644 src/strands/telemetry/config.py create mode 100644 src/strands/telemetry/metrics_constants.py diff --git a/src/strands/agent/agent.py b/src/strands/agent/agent.py index 56f5b92e7..2475b87e5 100644 --- a/src/strands/agent/agent.py +++ b/src/strands/agent/agent.py @@ -308,7 +308,6 @@ def __init__( # Initialize tracer instance (no-op if not configured) self.tracer = get_tracer() self.trace_span: Optional[trace.Span] = None - self.tool_caller = Agent.ToolCaller(self) @property diff --git a/src/strands/telemetry/__init__.py b/src/strands/telemetry/__init__.py index 159812162..21dd6ebf2 100644 --- a/src/strands/telemetry/__init__.py +++ b/src/strands/telemetry/__init__.py @@ -3,7 +3,8 @@ This module provides metrics and tracing functionality. """ -from .metrics import EventLoopMetrics, Trace, metrics_to_string +from .config import get_otel_resource +from .metrics import EventLoopMetrics, MetricsClient, Trace, metrics_to_string from .tracer import Tracer, get_tracer __all__ = [ @@ -12,4 +13,6 @@ "metrics_to_string", "Tracer", "get_tracer", + "MetricsClient", + "get_otel_resource", ] diff --git a/src/strands/telemetry/config.py b/src/strands/telemetry/config.py new file mode 100644 index 000000000..9f5a05fda --- /dev/null +++ b/src/strands/telemetry/config.py @@ -0,0 +1,33 @@ +"""OpenTelemetry configuration and setup utilities for Strands agents. + +This module provides centralized configuration and initialization functionality +for OpenTelemetry components and other telemetry infrastructure shared across Strands applications. +""" + +from importlib.metadata import version + +from opentelemetry.sdk.resources import Resource + + +def get_otel_resource() -> Resource: + """Create a standard OpenTelemetry resource with service information. + + This function implements a singleton pattern - it will return the same + Resource object for the same service_name parameter. + + Args: + service_name: Name of the service for OpenTelemetry. + + Returns: + Resource object with standard service information. + """ + resource = Resource.create( + { + "service.name": __name__, + "service.version": version("strands-agents"), + "telemetry.sdk.name": "opentelemetry", + "telemetry.sdk.language": "python", + } + ) + + return resource diff --git a/src/strands/telemetry/metrics.py b/src/strands/telemetry/metrics.py index cd70819b5..af9406430 100644 --- a/src/strands/telemetry/metrics.py +++ b/src/strands/telemetry/metrics.py @@ -6,6 +6,10 @@ from dataclasses import dataclass, field from typing import Any, Dict, Iterable, List, Optional, Set, Tuple +import opentelemetry.metrics as metrics_api +from opentelemetry.metrics import Counter, Meter + +from ..telemetry import metrics_constants as constants from ..types.content import Message from ..types.streaming import Metrics, Usage from ..types.tools import ToolUse @@ -355,3 +359,45 @@ def metrics_to_string(event_loop_metrics: EventLoopMetrics, allowed_names: Optio A formatted string representation of the metrics. """ return "\n".join(_metrics_summary_to_lines(event_loop_metrics, allowed_names or set())) + + +class MetricsClient: + """Singleton client for managing OpenTelemetry metrics instruments. + + The actual metrics export destination (console, OTLP endpoint, etc.) is configured + through OpenTelemetry SDK configuration by users, not by this client. + """ + + _instance: Optional["MetricsClient"] = None + meter: Meter + strands_agent_invocation_count: Counter + + def __new__(cls) -> "MetricsClient": + """Create or return the singleton instance of MetricsClient. + + Returns: + The single MetricsClient instance. + """ + if cls._instance is None: + cls._instance = super().__new__(cls) + return cls._instance + + def __init__(self) -> None: + """Initialize the MetricsClient. + + This method only runs once due to the singleton pattern. + Sets up the OpenTelemetry meter and creates metric instruments. + """ + if hasattr(self, "meter"): + return + + logger.info("Creating Strands MetricsClient") + meter_provider: metrics_api.MeterProvider = metrics_api.get_meter_provider() + self.meter = meter_provider.get_meter(__name__) + self.create_instruments() + + def create_instruments(self) -> None: + """Create and initialize all OpenTelemetry metric instruments.""" + self.strands_agent_invocation_count = self.meter.create_counter( + name=constants.STRANDS_AGENT_INVOCATION_COUNT, unit="Count" + ) diff --git a/src/strands/telemetry/metrics_constants.py b/src/strands/telemetry/metrics_constants.py new file mode 100644 index 000000000..d3d3e81fb --- /dev/null +++ b/src/strands/telemetry/metrics_constants.py @@ -0,0 +1,3 @@ +"""Metrics that are emitted in Strands-Agent.""" + +STRANDS_AGENT_INVOCATION_COUNT = "strands.agent.invocation_count" diff --git a/src/strands/telemetry/tracer.py b/src/strands/telemetry/tracer.py index e9a37a4aa..813c90e1b 100644 --- a/src/strands/telemetry/tracer.py +++ b/src/strands/telemetry/tracer.py @@ -8,20 +8,19 @@ import logging import os from datetime import date, datetime, timezone -from importlib.metadata import version from typing import Any, Dict, Mapping, Optional import opentelemetry.trace as trace_api from opentelemetry import propagate from opentelemetry.baggage.propagation import W3CBaggagePropagator from opentelemetry.propagators.composite import CompositePropagator -from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.trace import TracerProvider as SDKTracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor, ConsoleSpanExporter, SimpleSpanProcessor from opentelemetry.trace import Span, StatusCode from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator from ..agent.agent_result import AgentResult +from ..telemetry import get_otel_resource from ..types.content import Message, Messages from ..types.streaming import Usage from ..types.tools import ToolResult, ToolUse @@ -151,7 +150,6 @@ def __init__( self.otlp_headers = otlp_headers or {} self.tracer_provider: Optional[trace_api.TracerProvider] = None self.tracer: Optional[trace_api.Tracer] = None - propagate.set_global_textmap( CompositePropagator( [ @@ -173,15 +171,7 @@ def _initialize_tracer(self) -> None: self.tracer = self.tracer_provider.get_tracer(self.service_name) return - # Create resource with service information - resource = Resource.create( - { - "service.name": self.service_name, - "service.version": version("strands-agents"), - "telemetry.sdk.name": "opentelemetry", - "telemetry.sdk.language": "python", - } - ) + resource = get_otel_resource() # Create tracer provider self.tracer_provider = SDKTracerProvider(resource=resource) @@ -216,6 +206,7 @@ def _initialize_tracer(self) -> None: batch_processor = BatchSpanProcessor(otlp_exporter) self.tracer_provider.add_span_processor(batch_processor) logger.info("endpoint=<%s> | OTLP exporter configured with endpoint", endpoint) + except Exception as e: logger.exception("error=<%s> | Failed to configure OTLP exporter", e) elif self.otlp_endpoint and self.tracer_provider: diff --git a/tests/strands/telemetry/test_metrics.py b/tests/strands/telemetry/test_metrics.py index 4e84f0fdc..cafbd4bb7 100644 --- a/tests/strands/telemetry/test_metrics.py +++ b/tests/strands/telemetry/test_metrics.py @@ -1,9 +1,13 @@ import dataclasses import unittest +from unittest import mock import pytest +from opentelemetry.metrics._internal import _ProxyMeter +from opentelemetry.sdk.metrics import MeterProvider import strands +from strands.telemetry import MetricsClient from strands.types.streaming import Metrics, Usage @@ -117,6 +121,30 @@ def test_trace_end(mock_time, end_time, trace): assert tru_end_time == exp_end_time +@pytest.fixture +def mock_get_meter_provider(): + with mock.patch("strands.telemetry.metrics.metrics_api.get_meter_provider") as mock_get_meter_provider: + meter_provider_mock = mock.MagicMock(spec=MeterProvider) + mock_get_meter_provider.return_value = meter_provider_mock + + mock_meter = mock.MagicMock() + meter_provider_mock.get_meter.return_value = mock_meter + + yield mock_get_meter_provider + + +@pytest.fixture +def mock_sdk_meter_provider(): + with mock.patch("strands.telemetry.metrics.metrics_sdk.MeterProvider") as mock_meter_provider: + yield mock_meter_provider + + +@pytest.fixture +def mock_resource(): + with mock.patch("opentelemetry.sdk.resources.Resource") as mock_resource: + yield mock_resource + + def test_trace_add_child(child_trace, trace): trace.add_child(child_trace) @@ -379,3 +407,31 @@ def test_metrics_to_string(trace, child_trace, tool_metrics, exp_str, event_loop tru_str = strands.telemetry.metrics.metrics_to_string(event_loop_metrics) assert tru_str == exp_str + + +def test_setup_meter_if_meter_provider_is_set( + mock_get_meter_provider, + mock_resource, +): + """Test global meter_provider and meter are used""" + mock_resource_instance = mock.MagicMock() + mock_resource.create.return_value = mock_resource_instance + + metrics_client = MetricsClient() + + mock_get_meter_provider.assert_called() + mock_get_meter_provider.return_value.get_meter.assert_called() + + assert metrics_client is not None + + +def test_use_ProxyMeter_if_no_global_meter_provider(): + """Return _ProxyMeter""" + # Reset the singleton instance + strands.telemetry.metrics.MetricsClient._instance = None + + # Create a new instance which should use the real _ProxyMeter + metrics_client = MetricsClient() + + # Verify it's using a _ProxyMeter + assert isinstance(metrics_client.meter, _ProxyMeter) diff --git a/tests/strands/telemetry/test_tracer.py b/tests/strands/telemetry/test_tracer.py index 030dcd371..6ae3e1ad0 100644 --- a/tests/strands/telemetry/test_tracer.py +++ b/tests/strands/telemetry/test_tracer.py @@ -73,7 +73,9 @@ def mock_console_exporter(): @pytest.fixture def mock_resource(): - with mock.patch("strands.telemetry.tracer.Resource") as mock_resource: + with mock.patch("strands.telemetry.tracer.get_otel_resource") as mock_resource: + mock_resource_instance = mock.MagicMock() + mock_resource.return_value = mock_resource_instance yield mock_resource @@ -175,14 +177,12 @@ def test_initialize_tracer_with_console( ): """Test initializing the tracer with console exporter.""" mock_is_initialized.return_value = False - mock_resource_instance = mock.MagicMock() - mock_resource.create.return_value = mock_resource_instance # Initialize Tracer Tracer(enable_console_export=True) # Verify the tracer provider was created with correct resource - mock_tracer_provider.assert_called_once_with(resource=mock_resource_instance) + mock_tracer_provider.assert_called_once_with(resource=mock_resource.return_value) # Verify console exporter was added mock_console_exporter.assert_called_once() @@ -198,9 +198,6 @@ def test_initialize_tracer_with_otlp( """Test initializing the tracer with OTLP exporter.""" mock_is_initialized.return_value = False - mock_resource_instance = mock.MagicMock() - mock_resource.create.return_value = mock_resource_instance - # Initialize Tracer with ( mock.patch("strands.telemetry.tracer.HAS_OTEL_EXPORTER_MODULE", True), @@ -209,7 +206,7 @@ def test_initialize_tracer_with_otlp( Tracer(otlp_endpoint="http://test-endpoint") # Verify the tracer provider was created with correct resource - mock_tracer_provider.assert_called_once_with(resource=mock_resource_instance) + mock_tracer_provider.assert_called_once_with(resource=mock_resource.return_value) # Verify OTLP exporter was added with correct endpoint mock_otlp_exporter.assert_called_once() @@ -508,8 +505,6 @@ def test_initialize_tracer_with_invalid_otlp_endpoint( """Test initializing the tracer with an invalid OTLP endpoint.""" mock_is_initialized.return_value = False - mock_resource_instance = mock.MagicMock() - mock_resource.create.return_value = mock_resource_instance mock_otlp_exporter.side_effect = Exception("Connection error") # This should not raise an exception, but should log an error @@ -522,7 +517,7 @@ def test_initialize_tracer_with_invalid_otlp_endpoint( Tracer(otlp_endpoint="http://invalid-endpoint") # Verify the tracer provider was created with correct resource - mock_tracer_provider.assert_called_once_with(resource=mock_resource_instance) + mock_tracer_provider.assert_called_once_with(resource=mock_resource.return_value) # Verify OTLP exporter was attempted mock_otlp_exporter.assert_called_once() @@ -537,9 +532,6 @@ def test_initialize_tracer_with_missing_module( """Test initializing the tracer when the OTLP exporter module is missing.""" mock_is_initialized.return_value = False - mock_resource_instance = mock.MagicMock() - mock_resource.create.return_value = mock_resource_instance - # Initialize Tracer with OTLP endpoint but missing module with ( mock.patch("strands.telemetry.tracer.HAS_OTEL_EXPORTER_MODULE", False), @@ -552,13 +544,13 @@ def test_initialize_tracer_with_missing_module( assert "otel http exporting is currently DISABLED" in str(excinfo.value) # Verify the tracer provider was created with correct resource - mock_tracer_provider.assert_called_once_with(resource=mock_resource_instance) + mock_tracer_provider.assert_called_once_with(resource=mock_resource.return_value) # Verify set_tracer_provider was not called since an exception was raised mock_set_tracer_provider.assert_not_called() -def test_initialize_tracer_with_custom_tracer_provider(mock_get_tracer_provider, mock_resource): +def test_initialize_tracer_with_custom_tracer_provider(mock_is_initialized, mock_get_tracer_provider, mock_resource): """Test initializing the tracer with NoOpTracerProvider.""" mock_is_initialized.return_value = True tracer = Tracer(otlp_endpoint="http://invalid-endpoint")