From 9c93ffcba6f281d0a49374bac6a4be38e3698266 Mon Sep 17 00:00:00 2001 From: Tao Chen Date: Thu, 9 Jul 2026 12:47:53 -0700 Subject: [PATCH 1/5] Best effort to serialize tool def to Json --- .../core/agent_framework/observability.py | 87 +++++++++++++++++-- .../core/tests/core/test_observability.py | 47 ++++++++++ .../tests/foundry/test_foundry_chat_client.py | 26 ++++++ 3 files changed, 151 insertions(+), 9 deletions(-) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index cc6aa771f8a..89457dbc2dc 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -25,7 +25,17 @@ from collections.abc import Awaitable, Callable, Generator, Mapping, Sequence from enum import Enum from time import perf_counter, time_ns -from typing import TYPE_CHECKING, Any, ClassVar, Final, Generic, Literal, TypedDict, cast, overload +from typing import ( + TYPE_CHECKING, + Any, + ClassVar, + Final, + Generic, + Literal, + TypedDict, + cast, + overload, +) from dotenv import load_dotenv from opentelemetry import metrics, trace @@ -45,7 +55,9 @@ from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.trace.export import SpanExporter from opentelemetry.trace import Tracer - from opentelemetry.util._decorator import _AgnosticContextManager # type: ignore[reportPrivateUsage] + from opentelemetry.util._decorator import ( + _AgnosticContextManager, # type: ignore[reportPrivateUsage] + ) from pydantic import BaseModel from ._agents import SupportsAgentRun @@ -447,11 +459,15 @@ def _create_otlp_exporters( elif protocol in ("http/protobuf", "http"): # Import all HTTP exporters try: - from opentelemetry.exporter.otlp.proto.http._log_exporter import OTLPLogExporter as HTTPLogExporter + from opentelemetry.exporter.otlp.proto.http._log_exporter import ( + OTLPLogExporter as HTTPLogExporter, + ) from opentelemetry.exporter.otlp.proto.http.metric_exporter import ( OTLPMetricExporter as HTTPMetricExporter, ) - from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter as HTTPSpanExporter + from opentelemetry.exporter.otlp.proto.http.trace_exporter import ( + OTLPSpanExporter as HTTPSpanExporter, + ) except ImportError as exc: raise ImportError( "opentelemetry-exporter-otlp-proto-http is required for OTLP HTTP exporters. " @@ -901,9 +917,15 @@ def _configure_providers( try: from opentelemetry._logs import set_logger_provider from opentelemetry.sdk._logs import LoggerProvider, LoggingHandler - from opentelemetry.sdk._logs.export import BatchLogRecordProcessor, LogRecordExporter + from opentelemetry.sdk._logs.export import ( + BatchLogRecordProcessor, + LogRecordExporter, + ) from opentelemetry.sdk.metrics import MeterProvider - from opentelemetry.sdk.metrics.export import MetricExporter, PeriodicExportingMetricReader + from opentelemetry.sdk.metrics.export import ( + MetricExporter, + PeriodicExportingMetricReader, + ) from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor, SpanExporter except ModuleNotFoundError as ex: @@ -1467,7 +1489,11 @@ def get_response( function_invocation_kwargs: Keyword arguments forwarded only to tool invocation layers. client_kwargs: Additional client-specific keyword arguments for downstream chat clients. """ - from ._types import ChatResponse, ChatResponseUpdate, ResponseStream # type: ignore[reportUnusedImport] + from ._types import ( # type: ignore[reportUnusedImport] + ChatResponse, + ChatResponseUpdate, + ResponseStream, + ) global OBSERVABILITY_SETTINGS super_get_response = super().get_response # type: ignore[misc] @@ -2281,6 +2307,44 @@ def _get_instructions_from_options(options: Any) -> str | list[str] | None: _CACHE_MISS: Final = object() +def _serialize_tool_definitions(tools: Any) -> str | None: + """Serialize tools into the OTel GenAI tool-definitions JSON string. + + Returns ``None`` when no tool can be represented. Serialization is + best-effort: any failure is swallowed with a warning so telemetry never + raises into the caller. When a tool spec carries a value that is not + JSON-serializable, definitions are encoded individually so the offending + tool is skipped (and named in the warning) while the rest are still + captured. + """ + try: + tools_dict = _tools_to_dict(tools) + except Exception: + logger.warning("Failed to build tool definitions for telemetry; skipping attribute.", exc_info=True) + return None + if not tools_dict: + return None + try: + return json.dumps(tools_dict, ensure_ascii=False) + except Exception: + # A tool spec holds a value that is not JSON-serializable. Encode each + # definition on its own so the rest are still captured and the offending + # tool is named in the warning. + serializable: list[dict[str, Any]] = [] + for definition in tools_dict: + try: + json.dumps(definition, ensure_ascii=False) + except Exception: + logger.warning( + "Failed to serialize tool definition for telemetry; skipping tool %r.", + definition.get("name") or definition.get("type") or "", + exc_info=True, + ) + else: + serializable.append(definition) + return json.dumps(serializable, ensure_ascii=False) if serializable else None + + def _tools_to_dict( tools: Any, ) -> list[dict[str, Any]] | None: @@ -2366,7 +2430,12 @@ def _build_tool_otel_definition(tool_item: Any) -> dict[str, Any] | None: elif isinstance(tool_item, SerializationMixin): raw = tool_item.to_dict() elif isinstance(tool_item, Mapping): - raw = cast("Mapping[str, Any]", tool_item) + mapping_item = cast("Mapping[str, Any]", tool_item) + # Azure SDK tool models expose ``as_dict()``, which recursively converts + # the whole model (including nested non-dict Mapping values that are not + # JSON-serializable) into plain dicts; plain mappings are used as-is. + as_dict: Callable[[], Mapping[str, Any]] | None = getattr(mapping_item, "as_dict", None) + raw = as_dict() if callable(as_dict) else mapping_item if raw is None: logger.warning( @@ -2478,7 +2547,7 @@ def _otel_definition_from_mapping(raw: Mapping[str, Any]) -> dict[str, Any] | No # Tools with validation - returns None if no valid tools "tools": ( OtelAttr.TOOL_DEFINITIONS, - lambda tools: json.dumps(tools_dict, ensure_ascii=False) if (tools_dict := _tools_to_dict(tools)) else None, + _serialize_tool_definitions, True, None, ), diff --git a/python/packages/core/tests/core/test_observability.py b/python/packages/core/tests/core/test_observability.py index c5fc924a50c..4a863f59348 100644 --- a/python/packages/core/tests/core/test_observability.py +++ b/python/packages/core/tests/core/test_observability.py @@ -3197,6 +3197,53 @@ def test_get_span_attributes_omits_tool_definitions_when_unparseable() -> None: assert OtelAttr.TOOL_DEFINITIONS not in attrs +def test_get_span_attributes_skips_and_names_unserializable_tool(caplog: pytest.LogCaptureFixture) -> None: + """A tool whose definition isn't JSON-serializable is skipped by name; others survive.""" + import json as _json + + from agent_framework.observability import OtelAttr, _get_span_attributes + + class _Unserializable: + pass + + with caplog.at_level("WARNING", logger="agent_framework"): + attrs = _get_span_attributes( + operation_name="chat", + provider_name="openai", + model="gpt-4", + tools=[ + {"type": "web_search", "name": "good_tool"}, + {"type": "code_interpreter", "name": "bad_tool", "container": _Unserializable()}, + ], + ) + + # Serialization must not raise; the serializable tool is still captured and the bad one dropped. + definitions = _json.loads(attrs[OtelAttr.TOOL_DEFINITIONS]) + assert definitions == [{"type": "web_search", "name": "good_tool"}] + # The warning names the offending tool so customers can identify it. + assert any("bad_tool" in record.getMessage() for record in caplog.records) + + +def test_get_span_attributes_omits_tool_definitions_when_all_unserializable( + caplog: pytest.LogCaptureFixture, +) -> None: + """When every tool definition fails to serialize, the attribute is omitted (no raise).""" + from agent_framework.observability import OtelAttr, _get_span_attributes + + class _Unserializable: + pass + + with caplog.at_level("WARNING", logger="agent_framework"): + attrs = _get_span_attributes( + operation_name="chat", + provider_name="openai", + tools=[{"type": "code_interpreter", "name": "bad_tool", "container": _Unserializable()}], + ) + + assert OtelAttr.TOOL_DEFINITIONS not in attrs + assert any("bad_tool" in record.getMessage() for record in caplog.records) + + def test_tools_to_dict_supports_pydantic_tool_models() -> None: """Pydantic-based tool specs are reshaped into the OTel GenAI tool-definition shape.""" from pydantic import BaseModel diff --git a/python/packages/foundry/tests/foundry/test_foundry_chat_client.py b/python/packages/foundry/tests/foundry/test_foundry_chat_client.py index 571b3afbae2..8ccd37edbcb 100644 --- a/python/packages/foundry/tests/foundry/test_foundry_chat_client.py +++ b/python/packages/foundry/tests/foundry/test_foundry_chat_client.py @@ -957,6 +957,32 @@ def test_get_code_interpreter_tool_with_file_ids() -> None: assert tool_obj is not None +def test_code_interpreter_tool_serializes_to_otel_tool_definitions() -> None: + """Hosted code interpreter tools must serialize into OTel tool definitions. + + Regression test: ``CodeInterpreterTool`` is an Azure SDK model (a non-dict ``Mapping``) + whose nested ``container`` (``AutoCodeInterpreterToolParam``) is itself a non-dict + ``Mapping``. Capturing telemetry for a request carrying this tool previously raised + ``TypeError: Object of type AutoCodeInterpreterToolParam is not JSON serializable``. + """ + import json + + from agent_framework.observability import OtelAttr, _get_span_attributes + + tool_obj = RawFoundryChatClient.get_code_interpreter_tool(file_ids=["assistant-abc123"]) + + attributes = _get_span_attributes(operation_name="chat", provider_name="foundry", tools=tool_obj) + + definitions = json.loads(attributes[OtelAttr.TOOL_DEFINITIONS]) + assert definitions == [ + { + "type": "code_interpreter", + "name": "code_interpreter", + "container": {"type": "auto", "file_ids": ["assistant-abc123"]}, + } + ] + + def test_get_file_search_tool() -> None: """Test file search tool creation.""" From c21bfee03c3c455be557c9d4f0eab3f4cfec244b Mon Sep 17 00:00:00 2001 From: Tao Chen Date: Thu, 9 Jul 2026 13:40:55 -0700 Subject: [PATCH 2/5] Fix formatting --- .../core/agent_framework/observability.py | 78 ++++++++++++++----- 1 file changed, 58 insertions(+), 20 deletions(-) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index 89457dbc2dc..5ae0cd049f9 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -892,7 +892,11 @@ def _configure( from opentelemetry.sdk.metrics.export import ConsoleMetricExporter from opentelemetry.sdk.trace.export import ConsoleSpanExporter - exporters.extend([ConsoleSpanExporter(), ConsoleLogRecordExporter(), ConsoleMetricExporter()]) + exporters.extend([ + ConsoleSpanExporter(), + ConsoleLogRecordExporter(), + ConsoleMetricExporter(), + ]) # 4. Add VS Code extension exporters if port is specified if self.vs_code_extension_port: @@ -1489,12 +1493,6 @@ def get_response( function_invocation_kwargs: Keyword arguments forwarded only to tool invocation layers. client_kwargs: Additional client-specific keyword arguments for downstream chat clients. """ - from ._types import ( # type: ignore[reportUnusedImport] - ChatResponse, - ChatResponseUpdate, - ResponseStream, - ) - global OBSERVABILITY_SETTINGS super_get_response = super().get_response # type: ignore[misc] merged_client_kwargs = dict(client_kwargs) if client_kwargs is not None else {} @@ -1588,7 +1586,11 @@ async def _finalize_stream() -> None: # Stream errored; skip get_final_response() to avoid firing # result hooks such as after_run context providers on error # paths. Capture the error on the span before returning. - capture_exception(span=span, exception=result_stream._stream_error, timestamp=time_ns()) # pyright: ignore[reportPrivateUsage] + capture_exception( + span=span, + exception=result_stream._stream_error, # type: ignore + timestamp=time_ns(), + ) return response: ChatResponse[Any] = await result_stream.get_final_response() duration = duration_state.get("duration") @@ -1792,7 +1794,10 @@ def _trace_agent_invocation( merged_options: Mapping[str, Any], client_kwargs: Mapping[str, Any] | None, stream: bool, - execute: Callable[[], Awaitable[AgentResponse[Any]] | ResponseStream[AgentResponseUpdate, AgentResponse[Any]]], + execute: Callable[ + [], + Awaitable[AgentResponse[Any]] | ResponseStream[AgentResponseUpdate, AgentResponse[Any]], + ], ) -> Awaitable[AgentResponse[Any]] | ResponseStream[AgentResponseUpdate, AgentResponse[Any]]: """Trace an agent invocation while delegating execution to ``execute``.""" global OBSERVABILITY_SETTINGS @@ -1888,7 +1893,11 @@ async def _finalize_stream() -> None: # Stream errored; skip get_final_response() to avoid firing # result hooks such as after_run context providers on error # paths. Capture the error on the span before returning. - capture_exception(span=span, exception=result_stream._stream_error, timestamp=time_ns()) # pyright: ignore[reportPrivateUsage] + capture_exception( + span=span, + exception=result_stream._stream_error, # type: ignore + timestamp=time_ns(), + ) return response: AgentResponse[Any] = await result_stream.get_final_response() duration = duration_state.get("duration") @@ -1992,8 +2001,15 @@ async def _run() -> AgentResponse[Any]: INNER_USAGE_CAPTURED_FIELD not in inner_response_telemetry_captured_fields ), ) - _apply_accumulated_usage(response_attributes, inner_response_telemetry_captured_fields) - _capture_response(span=span, attributes=response_attributes, duration=duration) + _apply_accumulated_usage( + response_attributes, + inner_response_telemetry_captured_fields, + ) + _capture_response( + span=span, + attributes=response_attributes, + duration=duration, + ) if ( OBSERVABILITY_SETTINGS.SENSITIVE_DATA_ENABLED and response.messages @@ -2288,7 +2304,7 @@ def _get_instructions_from_options(options: Any) -> str | list[str] | None: instructions = cast(Mapping[str, Any], options).get("instructions") if isinstance(instructions, str): return instructions - if isinstance(instructions, list) and all(isinstance(item, str) for item in instructions): # type: ignore[reportUnknownVariableType] + if isinstance(instructions, list) and all(isinstance(item, str) for item in instructions): # type: ignore return instructions # type: ignore[reportUnknownVariableType] return None return None @@ -2320,7 +2336,10 @@ def _serialize_tool_definitions(tools: Any) -> str | None: try: tools_dict = _tools_to_dict(tools) except Exception: - logger.warning("Failed to build tool definitions for telemetry; skipping attribute.", exc_info=True) + logger.warning( + "Failed to build tool definitions for telemetry; skipping attribute.", + exc_info=True, + ) return None if not tools_dict: return None @@ -2564,7 +2583,12 @@ def _get_span_attributes(**kwargs: Any) -> dict[str, Any]: options = kwargs.get("all_options", kwargs.get("options")) options_mapping = cast(Mapping[str, Any], options) if isinstance(options, Mapping) else None - for source_keys, (otel_key, transform_func, check_options, default_value) in OTEL_ATTR_MAP.items(): + for source_keys, ( + otel_key, + transform_func, + check_options, + default_value, + ) in OTEL_ATTR_MAP.items(): # Normalize to tuple of keys keys = (source_keys,) if isinstance(source_keys, str) else source_keys @@ -2604,7 +2628,10 @@ def _capture_system_instructions(span: trace.Span, system_instructions: str | li otel_sys_instructions = [ {"type": "text", "content": instruction} for instruction in _normalize_instructions(system_instructions) ] - span.set_attribute(OtelAttr.SYSTEM_INSTRUCTIONS, json.dumps(otel_sys_instructions, ensure_ascii=False)) + span.set_attribute( + OtelAttr.SYSTEM_INSTRUCTIONS, + json.dumps(otel_sys_instructions, ensure_ascii=False), + ) def _capture_current_agent_system_instructions( @@ -2703,14 +2730,18 @@ def _capture_messages( if finish_reason: otel_messages[-1]["finish_reason"] = FINISH_REASON_MAP[finish_reason] span.set_attribute( - OtelAttr.OUTPUT_MESSAGES if output else OtelAttr.INPUT_MESSAGES, json.dumps(otel_messages, ensure_ascii=False) + OtelAttr.OUTPUT_MESSAGES if output else OtelAttr.INPUT_MESSAGES, + json.dumps(otel_messages, ensure_ascii=False), ) _capture_system_instructions(span, system_instructions) def _to_otel_message(message: Message) -> dict[str, Any]: """Create a otel representation of a message.""" - return {"role": message.role, "parts": [_to_otel_part(content) for content in message.contents]} + return { + "role": message.role, + "parts": [_to_otel_part(content) for content in message.contents], + } def _to_otel_part(content: Content) -> dict[str, Any] | None: @@ -2737,7 +2768,12 @@ def _to_otel_part(content: Content) -> dict[str, Any] | None: "modality": content.media_type.split("/")[0] if content.media_type else None, } case "function_call": - return {"type": "tool_call", "id": content.call_id, "name": content.name, "arguments": content.arguments} + return { + "type": "tool_call", + "id": content.call_id, + "name": content.name, + "arguments": content.arguments, + } case "function_result": return { "type": "tool_call_response", @@ -2751,7 +2787,9 @@ def _to_otel_part(content: Content) -> dict[str, Any] | None: return None -def _mark_inner_response_telemetry_captured(response: ChatResponse | AgentResponse) -> None: +def _mark_inner_response_telemetry_captured( + response: ChatResponse | AgentResponse, +) -> None: """Record when an inner chat telemetry span already captured response metadata.""" captured_fields = INNER_RESPONSE_TELEMETRY_CAPTURED_FIELDS.get() if captured_fields is None: From 2e005c5a20af3317df768a50e680de9ff1b26307 Mon Sep 17 00:00:00 2001 From: Tao Chen Date: Thu, 9 Jul 2026 13:55:09 -0700 Subject: [PATCH 3/5] Fix tests --- python/packages/core/agent_framework/observability.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index 5ae0cd049f9..5e4ab924cf3 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -1493,6 +1493,8 @@ def get_response( function_invocation_kwargs: Keyword arguments forwarded only to tool invocation layers. client_kwargs: Additional client-specific keyword arguments for downstream chat clients. """ + from ._types import ChatResponse, ChatResponseUpdate, ResponseStream # type: ignore[reportUnusedImport] + global OBSERVABILITY_SETTINGS super_get_response = super().get_response # type: ignore[misc] merged_client_kwargs = dict(client_kwargs) if client_kwargs is not None else {} From 111c21aa941bdd548f371a279b36f1e14ec8d36f Mon Sep 17 00:00:00 2001 From: Tao Chen Date: Fri, 10 Jul 2026 11:37:27 -0700 Subject: [PATCH 4/5] Optimize json serialization --- .../core/agent_framework/observability.py | 146 +++++++++--------- .../core/tests/core/test_observability.py | 118 +++++++------- 2 files changed, 130 insertions(+), 134 deletions(-) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index 5e4ab924cf3..7faa40f6721 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -2314,13 +2314,15 @@ def _get_instructions_from_options(options: Any) -> str | list[str] | None: # region OTel tool definitions -# Per-item in-memory cache of computed OTel tool definitions, keyed by the tool -# object's identity. Tool objects (e.g. ``FunctionTool``, ``MCPTool``) are often -# reused across runs, so caching their converted definitions avoids repeating the -# isinstance checks, schema generation, and dict construction on every invocation. -# A ``WeakKeyDictionary`` lets entries be garbage collected with their tools. -# Unhashable / non-weak-referenceable specs (e.g. plain dicts) bypass the cache. -_TOOL_OTEL_DEFINITION_CACHE: weakref.WeakKeyDictionary[Any, dict[str, Any] | None] = weakref.WeakKeyDictionary() +# Per-item in-memory cache of the *serialized* OTel tool-definition JSON fragment, +# keyed by the tool object's identity. Tool objects (e.g. ``FunctionTool``, +# ``MCPTool``) are often reused across runs, so caching the encoded string (rather +# than just the definition dict) lets repeated runs reuse the fragment without +# repeating the isinstance checks, schema generation, dict construction, and +# ``json.dumps``. A ``WeakKeyDictionary`` lets entries be garbage collected with +# their tools; unhashable / non-weak-referenceable specs (e.g. plain dicts) bypass +# the cache. +_TOOL_OTEL_JSON_CACHE: weakref.WeakKeyDictionary[Any, str | None] = weakref.WeakKeyDictionary() # Sentinel distinguishing "not cached" from a cached ``None`` (unparseable tool). _CACHE_MISS: Final = object() @@ -2330,100 +2332,92 @@ def _serialize_tool_definitions(tools: Any) -> str | None: Returns ``None`` when no tool can be represented. Serialization is best-effort: any failure is swallowed with a warning so telemetry never - raises into the caller. When a tool spec carries a value that is not - JSON-serializable, definitions are encoded individually so the offending - tool is skipped (and named in the warning) while the rest are still - captured. + raises into the caller. Each tool's JSON fragment is cached per tool object + (see :func:`_tool_to_otel_json`), so reused tool instances skip both the + definition build and ``json.dumps``; the fragments are joined into a JSON + array, and a tool that cannot be represented or serialized is skipped (and + named in the warning) while the rest are still captured. """ + from ._tools import normalize_tools + try: - tools_dict = _tools_to_dict(tools) + normalized_tools = normalize_tools(tools) except Exception: logger.warning( "Failed to build tool definitions for telemetry; skipping attribute.", exc_info=True, ) return None - if not tools_dict: - return None - try: - return json.dumps(tools_dict, ensure_ascii=False) - except Exception: - # A tool spec holds a value that is not JSON-serializable. Encode each - # definition on its own so the rest are still captured and the offending - # tool is named in the warning. - serializable: list[dict[str, Any]] = [] - for definition in tools_dict: - try: - json.dumps(definition, ensure_ascii=False) - except Exception: - logger.warning( - "Failed to serialize tool definition for telemetry; skipping tool %r.", - definition.get("name") or definition.get("type") or "", - exc_info=True, - ) - else: - serializable.append(definition) - return json.dumps(serializable, ensure_ascii=False) if serializable else None - - -def _tools_to_dict( - tools: Any, -) -> list[dict[str, Any]] | None: - """Convert tools into OpenTelemetry GenAI tool definitions. - - The output conforms to the OTel GenAI tool-definitions schema, where each - entry is either a ``FunctionToolDefinition`` (``type="function"`` with - ``name`` and optional ``description``/``parameters``) or a - ``GenericToolDefinition`` (any ``type`` plus a ``name``). See - https://github.com/open-telemetry/semantic-conventions-genai/blob/main/docs/gen-ai/gen-ai-tool-definitions.json. - - Args: - tools: The tools to parse. Can be a single tool or a sequence of tools. - - Returns: - A list of OTel-conformant tool-definition dicts, or ``None`` when - ``tools`` is empty or no tool can be represented. - """ - from ._tools import normalize_tools - - normalized_tools = normalize_tools(tools) if not normalized_tools: return None - results: list[dict[str, Any]] = [] + fragments: list[str] = [] for tool_item in normalized_tools: - otel_def = _tool_to_otel_definition(tool_item) - if otel_def is not None: - results.append(otel_def) - return results or None + fragment = _tool_to_otel_json(tool_item) + if fragment is not None: + fragments.append(fragment) + return f"[{','.join(fragments)}]" if fragments else None -def _tool_to_otel_definition(tool_item: Any) -> dict[str, Any] | None: - """Convert a single tool spec into an OTel GenAI tool-definition dict. +def _tool_to_otel_json(tool_item: Any) -> str | None: + """Serialize a single tool spec into its OTel tool-definition JSON fragment. - Results are cached per tool object (keyed by identity) so repeated runs that - reuse the same tool instances skip the conversion work. Specs that cannot be - weakly referenced (e.g. plain dicts) are converted without caching. + The encoded fragment is cached per tool object (keyed by identity) so that + repeated runs reuse the JSON without rebuilding the definition dict or + re-running ``json.dumps``. Specs that cannot be weakly referenced (e.g. + plain dicts) are serialized without caching. - Returns ``None`` and emits a warning when the input cannot be represented - as either a ``FunctionToolDefinition`` or a ``GenericToolDefinition``. + Returns ``None`` when the tool cannot be represented or serialized. """ try: - cached = _TOOL_OTEL_DEFINITION_CACHE.get(tool_item, _CACHE_MISS) + cached = _TOOL_OTEL_JSON_CACHE.get(tool_item, _CACHE_MISS) except TypeError: - # Unhashable spec (e.g. a plain dict); convert without caching. - return _build_tool_otel_definition(tool_item) + # Unhashable spec (e.g. a plain dict); serialize without caching. + return _build_tool_otel_json(tool_item) if cached is not _CACHE_MISS: - return cast("dict[str, Any] | None", cached) + return cast("str | None", cached) - definition = _build_tool_otel_definition(tool_item) + fragment = _build_tool_otel_json(tool_item) with contextlib.suppress(TypeError): # Object may not support weak references; skip caching when that is the case. - _TOOL_OTEL_DEFINITION_CACHE[tool_item] = definition - return definition + _TOOL_OTEL_JSON_CACHE[tool_item] = fragment + return fragment + + +def _build_tool_otel_json(tool_item: Any) -> str | None: + """Build and encode a single tool's OTel definition JSON fragment (uncached).""" + try: + definition = _build_tool_otel_definition(tool_item) + except Exception: + logger.warning( + "Failed to build tool definition for telemetry; skipping tool.", + exc_info=True, + ) + return None + if definition is None: + return None + try: + return json.dumps(definition, ensure_ascii=False) + except Exception: + logger.warning( + "Failed to serialize tool definition for telemetry; skipping tool %r.", + definition.get("name") or definition.get("type") or "", + exc_info=True, + ) + return None def _build_tool_otel_definition(tool_item: Any) -> dict[str, Any] | None: - """Convert a single tool spec into an OTel GenAI tool-definition dict (uncached).""" + """Convert a single tool spec into an OTel GenAI tool-definition dict (uncached). + + The output conforms to the OTel GenAI tool-definitions schema, where the + result is either a ``FunctionToolDefinition`` (``type="function"`` with + ``name`` and optional ``description``/``parameters``) or a + ``GenericToolDefinition`` (any ``type`` plus a ``name``). See + https://github.com/open-telemetry/semantic-conventions-genai/blob/main/docs/gen-ai/gen-ai-tool-definitions.json. + + Returns ``None`` and emits a warning when the input cannot be represented + as either a ``FunctionToolDefinition`` or a ``GenericToolDefinition``. + """ from pydantic import BaseModel from ._mcp import MCPTool diff --git a/python/packages/core/tests/core/test_observability.py b/python/packages/core/tests/core/test_observability.py index 4a863f59348..5cd28ff2f8b 100644 --- a/python/packages/core/tests/core/test_observability.py +++ b/python/packages/core/tests/core/test_observability.py @@ -3244,11 +3244,11 @@ class _Unserializable: assert any("bad_tool" in record.getMessage() for record in caplog.records) -def test_tools_to_dict_supports_pydantic_tool_models() -> None: +def test_build_tool_otel_definition_supports_pydantic_tool_models() -> None: """Pydantic-based tool specs are reshaped into the OTel GenAI tool-definition shape.""" from pydantic import BaseModel - from agent_framework.observability import _tools_to_dict + from agent_framework.observability import _build_tool_otel_definition class ProviderTool(BaseModel): type: str @@ -3256,33 +3256,31 @@ class ProviderTool(BaseModel): enabled: bool = True note: str | None = None - result = _tools_to_dict([ProviderTool(type="web_search", name="web_search")]) + result = _build_tool_otel_definition(ProviderTool(type="web_search", name="web_search")) - assert result == [{"type": "web_search", "name": "web_search", "enabled": True}] + assert result == {"type": "web_search", "name": "web_search", "enabled": True} -def test_tools_to_dict_returns_none_for_empty_input() -> None: - """``_tools_to_dict`` returns None when no tools are supplied.""" - from agent_framework.observability import _tools_to_dict +def test_serialize_tool_definitions_returns_none_for_empty_input() -> None: + """``_serialize_tool_definitions`` returns None when no tools are supplied.""" + from agent_framework.observability import _serialize_tool_definitions - assert _tools_to_dict(None) is None - assert _tools_to_dict([]) is None + assert _serialize_tool_definitions(None) is None + assert _serialize_tool_definitions([]) is None -def test_tools_to_dict_function_tool_uses_otel_function_definition() -> None: +def test_build_tool_otel_definition_function_tool_uses_otel_function_definition() -> None: """``FunctionTool`` instances are emitted as flat OTel FunctionToolDefinition dicts.""" from agent_framework import tool - from agent_framework.observability import _tools_to_dict + from agent_framework.observability import _build_tool_otel_definition @tool(name="add", description="Add two numbers") def add(x: int, y: int) -> int: return x + y - result = _tools_to_dict([add]) + definition = _build_tool_otel_definition(add) - assert result is not None - assert len(result) == 1 - definition = result[0] + assert definition is not None assert definition["type"] == "function" assert definition["name"] == "add" assert definition["description"] == "Add two numbers" @@ -3292,9 +3290,9 @@ def add(x: int, y: int) -> int: assert "function" not in definition -def test_tools_to_dict_flattens_openai_chat_completions_function_spec() -> None: +def test_build_tool_otel_definition_flattens_openai_chat_completions_function_spec() -> None: """OpenAI Chat Completions nested ``function`` spec is flattened to the OTel shape.""" - from agent_framework.observability import _tools_to_dict + from agent_framework.observability import _build_tool_otel_definition openai_spec = { "type": "function", @@ -3310,91 +3308,95 @@ def test_tools_to_dict_flattens_openai_chat_completions_function_spec() -> None: }, } - result = _tools_to_dict([openai_spec]) + result = _build_tool_otel_definition(openai_spec) - assert result == [ - { - "type": "function", - "name": "lookup_user", - "description": "Look up a user by id", - "parameters": { - "type": "object", - "properties": {"user_id": {"type": "string"}}, - "required": ["user_id"], - }, - "strict": True, - } - ] + assert result == { + "type": "function", + "name": "lookup_user", + "description": "Look up a user by id", + "parameters": { + "type": "object", + "properties": {"user_id": {"type": "string"}}, + "required": ["user_id"], + }, + "strict": True, + } -def test_tools_to_dict_passes_through_hosted_tool_dicts() -> None: +def test_build_tool_otel_definition_passes_through_hosted_tool_dicts() -> None: """Hosted-tool dicts pass through with the OTel required keys preserved.""" - from agent_framework.observability import _tools_to_dict + from agent_framework.observability import _build_tool_otel_definition - result = _tools_to_dict([{"type": "web_search", "name": "web_search", "max_results": 5}]) + result = _build_tool_otel_definition({"type": "web_search", "name": "web_search", "max_results": 5}) - assert result == [{"type": "web_search", "name": "web_search", "max_results": 5}] + assert result == {"type": "web_search", "name": "web_search", "max_results": 5} -def test_tools_to_dict_falls_back_to_type_when_name_missing() -> None: +def test_build_tool_otel_definition_falls_back_to_type_when_name_missing() -> None: """Hosted-tool dicts without ``name`` fall back to the ``type`` value.""" - from agent_framework.observability import _tools_to_dict + from agent_framework.observability import _build_tool_otel_definition - result = _tools_to_dict([{"type": "code_interpreter"}]) + result = _build_tool_otel_definition({"type": "code_interpreter"}) - assert result == [{"type": "code_interpreter", "name": "code_interpreter"}] + assert result == {"type": "code_interpreter", "name": "code_interpreter"} -def test_tools_to_dict_warns_when_type_missing(caplog: pytest.LogCaptureFixture) -> None: +def test_build_tool_otel_definition_warns_when_type_missing(caplog: pytest.LogCaptureFixture) -> None: """Tools without an extractable ``type`` are skipped with a warning.""" - from agent_framework.observability import _tools_to_dict + from agent_framework.observability import _build_tool_otel_definition with caplog.at_level("WARNING", logger="agent_framework"): - result = _tools_to_dict([{"kind": "not_an_otel_tool"}]) + result = _build_tool_otel_definition({"kind": "not_an_otel_tool"}) assert result is None assert any("missing 'type'" in rec.message for rec in caplog.records) -def test_tools_to_dict_warns_for_unknown_tool_object(caplog: pytest.LogCaptureFixture) -> None: +def test_build_tool_otel_definition_warns_for_unknown_tool_object(caplog: pytest.LogCaptureFixture) -> None: """Tools that are neither callable, mapping, BaseModel, nor known type are skipped.""" - from agent_framework.observability import _tools_to_dict + from agent_framework.observability import _build_tool_otel_definition class _Opaque: pass with caplog.at_level("WARNING", logger="agent_framework"): - result = _tools_to_dict([_Opaque()]) + result = _build_tool_otel_definition(_Opaque()) assert result is None assert any("OpenTelemetry tool definition" in rec.message for rec in caplog.records) -def test_tool_to_otel_definition_caches_per_tool_object() -> None: - """Converting the same tool object twice reuses the cached OTel definition.""" +def test_tool_to_otel_json_caches_per_tool_object() -> None: + """Serializing the same tool object twice reuses the cached OTel JSON fragment.""" from agent_framework import tool - from agent_framework.observability import _build_tool_otel_definition, _tool_to_otel_definition + from agent_framework.observability import _TOOL_OTEL_JSON_CACHE, _build_tool_otel_json, _tool_to_otel_json @tool(name="add", description="Add two numbers") def add(x: int, y: int) -> int: return x + y - first = _tool_to_otel_definition(add) - second = _tool_to_otel_definition(add) + first = _tool_to_otel_json(add) + second = _tool_to_otel_json(add) - # The cached result is returned as the same object on subsequent conversions. - assert first is second - # A fresh (uncached) build produces an equal but distinct object. - assert _build_tool_otel_definition(add) == first + # The encoded fragment is cached under the tool object and reused as-is. + assert first == second + assert add in _TOOL_OTEL_JSON_CACHE + assert _TOOL_OTEL_JSON_CACHE[add] == first + # A fresh (uncached) build produces an equal fragment. + assert _build_tool_otel_json(add) == first -def test_tool_to_otel_definition_skips_cache_for_unhashable_specs() -> None: - """Plain-dict tool specs are converted without raising despite being uncacheable.""" - from agent_framework.observability import _tool_to_otel_definition +def test_tool_to_otel_json_skips_cache_for_unhashable_specs() -> None: + """Plain-dict tool specs are serialized without raising despite being uncacheable.""" + import json + + from agent_framework.observability import _tool_to_otel_json spec = {"type": "web_search", "name": "web_search"} - assert _tool_to_otel_definition(spec) == {"type": "web_search", "name": "web_search"} + fragment = _tool_to_otel_json(spec) + assert fragment is not None + assert json.loads(fragment) == {"type": "web_search", "name": "web_search"} # region Test _capture_response From 405a2291cf254a33a0e2fe101bc9de4d34acfe7a Mon Sep 17 00:00:00 2001 From: Tao Chen Date: Fri, 10 Jul 2026 11:57:57 -0700 Subject: [PATCH 5/5] Best effort: Add secret filtering --- .../core/agent_framework/observability.py | 15 +++++ .../core/tests/core/test_observability.py | 64 +++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index 7faa40f6721..2be137989f4 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -2325,6 +2325,10 @@ def _get_instructions_from_options(options: Any) -> str | list[str] | None: _TOOL_OTEL_JSON_CACHE: weakref.WeakKeyDictionary[Any, str | None] = weakref.WeakKeyDictionary() # Sentinel distinguishing "not cached" from a cached ``None`` (unparseable tool). _CACHE_MISS: Final = object() +# Tool-spec fields that can carry secrets (e.g. an OAuth access token on an MCP +# tool, or injected auth headers) and must never be copied into emitted telemetry. +# Matched by field name so any MCP tool spec is covered, not just a specific class. +_SENSITIVE_TOOL_DEFINITION_KEYS: Final[frozenset[str]] = frozenset({"authorization", "headers"}) def _serialize_tool_definitions(tools: Any) -> str | None: @@ -2484,10 +2488,14 @@ def _otel_definition_from_mapping(raw: Mapping[str, Any]) -> dict[str, Any] | No if parameters: definition["parameters"] = parameters # Forward extra properties from both layers, preferring the inner spec. + # Sensitive fields (e.g. auth tokens/headers) are dropped so telemetry + # never carries secrets. for source in (nested, raw): for key, value in source.items(): if key in {"type", "function", "name", "description", "parameters"}: continue + if key in _SENSITIVE_TOOL_DEFINITION_KEYS: + continue definition.setdefault(key, value) return definition @@ -2513,13 +2521,20 @@ def _otel_definition_from_mapping(raw: Mapping[str, Any]) -> dict[str, Any] | No for key, value in raw.items(): if key in {"type", "name", "description", "parameters"}: continue + if key in _SENSITIVE_TOOL_DEFINITION_KEYS: + continue definition.setdefault(key, value) return definition + # Generic (non-function) tool definition, e.g. an MCP tool. Sensitive fields + # (e.g. an OAuth ``authorization`` token or auth ``headers``) are dropped so + # telemetry never carries secrets. definition = {"type": type_value, "name": name_value} for key, value in raw.items(): if key in {"type", "name"}: continue + if key in _SENSITIVE_TOOL_DEFINITION_KEYS: + continue definition[key] = value return definition diff --git a/python/packages/core/tests/core/test_observability.py b/python/packages/core/tests/core/test_observability.py index 5cd28ff2f8b..e2ec0082b5b 100644 --- a/python/packages/core/tests/core/test_observability.py +++ b/python/packages/core/tests/core/test_observability.py @@ -3341,6 +3341,70 @@ def test_build_tool_otel_definition_falls_back_to_type_when_name_missing() -> No assert result == {"type": "code_interpreter", "name": "code_interpreter"} +def test_build_tool_otel_definition_omits_secrets_from_mcp_tool_spec() -> None: + """Generic MCP tool specs must not leak secret fields (e.g. auth token/headers).""" + from agent_framework.observability import _build_tool_otel_definition + + mcp_spec = { + "type": "mcp", + "name": "github_mcp", + "server_url": "https://mcp.example.com", + "authorization": "super-secret-oauth-token", + "headers": {"Authorization": "Bearer super-secret-oauth-token"}, + } + + result = _build_tool_otel_definition(mcp_spec) + + assert result is not None + assert result["type"] == "mcp" + assert result["name"] == "github_mcp" + assert result["server_url"] == "https://mcp.example.com" + # Secrets are dropped. + assert "authorization" not in result + assert "headers" not in result + + +def test_build_tool_otel_definition_omits_secrets_from_mcp_tool_mapping() -> None: + """MCP tool objects exposing a mapping (e.g. Azure SDK models) also drop secrets.""" + from collections.abc import Mapping + + from agent_framework.observability import _build_tool_otel_definition + + class _AzureLikeMcpTool(Mapping): # type: ignore[type-arg] + """Minimal stand-in for an Azure SDK ``MCPTool`` model with an ``as_dict``.""" + + def __init__(self) -> None: + self._data = { + "type": "mcp", + "name": "azure_mcp", + "server_label": "azure_mcp", + "authorization": "super-secret-oauth-token", + "headers": {"Authorization": "Bearer super-secret-oauth-token"}, + } + + def as_dict(self) -> dict[str, Any]: + return dict(self._data) + + def __getitem__(self, key: str) -> Any: + return self._data[key] + + def __iter__(self): # type: ignore[no-untyped-def] + return iter(self._data) + + def __len__(self) -> int: + return len(self._data) + + result = _build_tool_otel_definition(_AzureLikeMcpTool()) + + assert result is not None + assert result["type"] == "mcp" + assert result["name"] == "azure_mcp" + assert result["server_label"] == "azure_mcp" + # Secrets are dropped. + assert "authorization" not in result + assert "headers" not in result + + def test_build_tool_otel_definition_warns_when_type_missing(caplog: pytest.LogCaptureFixture) -> None: """Tools without an extractable ``type`` are skipped with a warning.""" from agent_framework.observability import _build_tool_otel_definition