diff --git a/src/google/adk/flows/llm_flows/core/_model_call.py b/src/google/adk/flows/llm_flows/core/_model_call.py index 2eaa8715468..47c1ba779b9 100644 --- a/src/google/adk/flows/llm_flows/core/_model_call.py +++ b/src/google/adk/flows/llm_flows/core/_model_call.py @@ -22,6 +22,7 @@ from google.genai import types from opentelemetry import context as otel_context from opentelemetry import trace +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from ....agents._streaming_mode import StreamingMode from ....agents.invocation_context import InvocationContext @@ -33,6 +34,7 @@ from ....models.llm_request import LlmRequest from ....models.llm_response import LlmResponse from ....telemetry import _instrumentation +from ....telemetry.tracing import resolve_error_type from ....telemetry.tracing import trace_call_llm from ....telemetry.tracing import tracer from ....utils.context_utils import Aclosing @@ -318,6 +320,7 @@ async def _call_llm_with_tracing() -> AsyncGenerator[LlmResponse, None]: except Exception as e: # pylint: disable=broad-except if span.is_recording(): span.record_exception(e) + span.set_attribute(ERROR_TYPE, resolve_error_type(e)) span.set_status( trace.Status(trace.StatusCode.ERROR, f'{type(e).__name__}: {e}') ) diff --git a/src/google/adk/telemetry/_instrumentation.py b/src/google/adk/telemetry/_instrumentation.py index e0ab670f4b6..af028c33873 100644 --- a/src/google/adk/telemetry/_instrumentation.py +++ b/src/google/adk/telemetry/_instrumentation.py @@ -29,6 +29,7 @@ from opentelemetry import trace import opentelemetry.context as context_api +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from typing_extensions import assert_never from . import _adk_attributes @@ -99,6 +100,7 @@ def _recording_span(span: trace.Span) -> Iterator[trace.Span]: except Exception as exc: # pylint: disable=broad-except if span.is_recording(): span.record_exception(exc) + span.set_attribute(ERROR_TYPE, tracing.resolve_error_type(exc)) span.set_status( trace.Status(trace.StatusCode.ERROR, f"{type(exc).__name__}: {exc}") ) diff --git a/src/google/adk/telemetry/node_tracing.py b/src/google/adk/telemetry/node_tracing.py index 38f030a08f9..797568988a2 100644 --- a/src/google/adk/telemetry/node_tracing.py +++ b/src/google/adk/telemetry/node_tracing.py @@ -30,6 +30,7 @@ from opentelemetry import trace from opentelemetry.semconv._incubating.attributes.gen_ai_attributes import GEN_AI_CONVERSATION_ID from opentelemetry.semconv._incubating.attributes.gen_ai_attributes import GEN_AI_OPERATION_NAME +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.trace import set_span_in_context from opentelemetry.trace import Span from opentelemetry.trace import Status @@ -40,6 +41,8 @@ from ..workflow._base_node import BaseNode from .context import TelemetryConfig from .tracing import _telemetry_config_from_invocation_context +from .tracing import resolve_error_type +from .tracing import start_as_current_span from .tracing import tracer if TYPE_CHECKING: @@ -199,7 +202,7 @@ def _invoke_node_span( Holds the span attached across the yield, so the caller must only await inside the block. See `start_as_current_node_span`. """ - with tracer.start_as_current_span( + with start_as_current_span( f"invoke_node {node.name}", attributes={ GEN_AI_OPERATION_NAME: "invoke_node", @@ -323,6 +326,7 @@ def _use_invoke_workflow_span( recorded_error = get_recorded_error() if recorded_error is not None: span.record_exception(recorded_error) + span.set_attribute(ERROR_TYPE, resolve_error_type(recorded_error)) span.set_status(Status(StatusCode.ERROR, str(recorded_error))) finally: _metrics.record_workflow_invocation_duration( diff --git a/src/google/adk/telemetry/tracing.py b/src/google/adk/telemetry/tracing.py index adfe0b00c7d..18810ccac62 100644 --- a/src/google/adk/telemetry/tracing.py +++ b/src/google/adk/telemetry/tracing.py @@ -32,6 +32,7 @@ import logging import os import re +from typing import Any from typing import Final from typing import TYPE_CHECKING @@ -211,6 +212,21 @@ def resolve_error_type(error: BaseException) -> str: return type(error).__name__ +@contextmanager +def start_as_current_span(name: str, **kwargs: Any) -> Iterator[Span]: + """Opens a span with ``tracer`` that names the error escaping it, if any. + + OpenTelemetry marks such a span ERROR on its own, but only ``error.type`` + says which error it was, and the metric for the same operation records it. + """ + with tracer.start_as_current_span(name, **kwargs) as span: + try: + yield span + except Exception as e: + span.set_attribute(ERROR_TYPE, resolve_error_type(e)) + raise + + def trace_agent_invocation( span: trace.Span, agent: BaseAgent, ctx: InvocationContext ) -> None: @@ -1120,7 +1136,7 @@ def _use_native_generate_content_span_stable_semconv( ) -> Iterator[GenerateContentSpan]: telemetry_config = telemetry_config or TelemetryConfig() system_name = _resolve_gen_ai_system_name(llm_request.model) - with tracer.start_as_current_span( + with start_as_current_span( f"generate_content {llm_request.model or ''}".strip() ) as span: span.set_attribute(GEN_AI_SYSTEM, system_name) @@ -1174,7 +1190,7 @@ def _use_native_generate_content_span( yield gc_span return - with tracer.start_as_current_span( + with start_as_current_span( f"generate_content {llm_request.model or ''}".strip() ) as span: _set_common_generate_content_attributes( diff --git a/tests/unittests/telemetry/functional_divergences.json b/tests/unittests/telemetry/functional_divergences.json index 3af6842d58c..81b778c057a 100644 --- a/tests/unittests/telemetry/functional_divergences.json +++ b/tests/unittests/telemetry/functional_divergences.json @@ -75,6 +75,16 @@ "kind": "otel_bug", "reason": "The instrumentor reads `FunctionDeclaration.parameters`, which ADK leaves unset in favour of `parameters_json_schema`, so it advertises the tools by name and description but without their schema." }, + { + "path": [ + "attributes", + "error.type" + ], + "example_native_instrumentation_value": "429", + "example_otel_instrumentation_value": "ClientError", + "kind": "desired_behavior", + "reason": "ADK reports the provider's HTTP status code (`429`), the same value as on its duration metric; the instrumentor reports the exception class (`ClientError`), which `google.genai` collapses every 4xx into." + }, { "path": [ "attributes", @@ -99,18 +109,6 @@ "kind": "otel_bug", "reason": "On a call that failed the instrumentor records an empty finish-reason list for a response it never got." }, - { - "path": [ - "attributes", - "error.type" - ], - "example_native_instrumentation_value": { - "missing_on_this_side": true - }, - "example_otel_instrumentation_value": "ClientError", - "kind": "adk_bug", - "reason": "ADK reports a failed call on the enclosing spans and on the duration metric, but leaves `error.type` off the inference span itself." - }, { "path": [ "attributes", diff --git a/tests/unittests/telemetry/functional_goldens/agent/inference-error-resource-exhausted-schema-v1.json b/tests/unittests/telemetry/functional_goldens/agent/inference-error-resource-exhausted-schema-v1.json index 9a339aa31fa..35d0b261d07 100644 --- a/tests/unittests/telemetry/functional_goldens/agent/inference-error-resource-exhausted-schema-v1.json +++ b/tests/unittests/telemetry/functional_goldens/agent/inference-error-resource-exhausted-schema-v1.json @@ -1,7 +1,9 @@ { "root_span": { "name": "invocation", - "attributes": {}, + "attributes": { + "error.type": "429" + }, "status": "ERROR", "children": [ { @@ -10,13 +12,16 @@ "gen_ai.operation.name": "invoke_agent", "gen_ai.agent.description": "A sample root agent.", "gen_ai.agent.name": "some_root_agent", - "gen_ai.conversation.id": "PRESENT" + "gen_ai.conversation.id": "PRESENT", + "error.type": "429" }, "status": "ERROR", "children": [ { "name": "call_llm", - "attributes": {}, + "attributes": { + "error.type": "429" + }, "status": "ERROR", "children": [ { @@ -28,7 +33,8 @@ "gen_ai.agent.name": "some_root_agent", "gen_ai.conversation.id": "PRESENT", "gcp.vertex.agent.event_id": "PRESENT", - "gcp.vertex.agent.invocation_id": "PRESENT" + "gcp.vertex.agent.invocation_id": "PRESENT", + "error.type": "429" }, "status": "ERROR", "children": [], diff --git a/tests/unittests/telemetry/functional_goldens/agent/inference-error-resource-exhausted-schema-v2.json b/tests/unittests/telemetry/functional_goldens/agent/inference-error-resource-exhausted-schema-v2.json index 57cb14843f6..41611118c52 100644 --- a/tests/unittests/telemetry/functional_goldens/agent/inference-error-resource-exhausted-schema-v2.json +++ b/tests/unittests/telemetry/functional_goldens/agent/inference-error-resource-exhausted-schema-v2.json @@ -4,7 +4,8 @@ "attributes": { "gen_ai.operation.name": "invoke_workflow", "gen_ai.conversation.id": "PRESENT", - "gen_ai.workflow.name": "some_root_agent" + "gen_ai.workflow.name": "some_root_agent", + "error.type": "429" }, "status": "ERROR", "children": [ @@ -14,13 +15,16 @@ "gen_ai.operation.name": "invoke_agent", "gen_ai.agent.description": "A sample root agent.", "gen_ai.agent.name": "some_root_agent", - "gen_ai.conversation.id": "PRESENT" + "gen_ai.conversation.id": "PRESENT", + "error.type": "429" }, "status": "ERROR", "children": [ { "name": "call_llm", - "attributes": {}, + "attributes": { + "error.type": "429" + }, "status": "ERROR", "children": [ { @@ -32,7 +36,8 @@ "gen_ai.agent.name": "some_root_agent", "gen_ai.conversation.id": "PRESENT", "gcp.vertex.agent.event_id": "PRESENT", - "gcp.vertex.agent.invocation_id": "PRESENT" + "gcp.vertex.agent.invocation_id": "PRESENT", + "error.type": "429" }, "status": "ERROR", "children": [], diff --git a/tests/unittests/telemetry/functional_goldens/agent/inference-error-valueerror-schema-v2.json b/tests/unittests/telemetry/functional_goldens/agent/inference-error-valueerror-schema-v2.json index 632e77b7141..fe73c7cb282 100644 --- a/tests/unittests/telemetry/functional_goldens/agent/inference-error-valueerror-schema-v2.json +++ b/tests/unittests/telemetry/functional_goldens/agent/inference-error-valueerror-schema-v2.json @@ -4,7 +4,8 @@ "attributes": { "gen_ai.operation.name": "invoke_workflow", "gen_ai.conversation.id": "PRESENT", - "gen_ai.workflow.name": "some_root_agent" + "gen_ai.workflow.name": "some_root_agent", + "error.type": "ValueError" }, "status": "ERROR", "children": [ @@ -14,13 +15,16 @@ "gen_ai.operation.name": "invoke_agent", "gen_ai.agent.description": "A sample root agent.", "gen_ai.agent.name": "some_root_agent", - "gen_ai.conversation.id": "PRESENT" + "gen_ai.conversation.id": "PRESENT", + "error.type": "ValueError" }, "status": "ERROR", "children": [ { "name": "call_llm", - "attributes": {}, + "attributes": { + "error.type": "ValueError" + }, "status": "ERROR", "children": [ { @@ -32,7 +36,8 @@ "gen_ai.agent.name": "some_root_agent", "gen_ai.conversation.id": "PRESENT", "gcp.vertex.agent.event_id": "PRESENT", - "gcp.vertex.agent.invocation_id": "PRESENT" + "gcp.vertex.agent.invocation_id": "PRESENT", + "error.type": "ValueError" }, "status": "ERROR", "children": [], diff --git a/tests/unittests/telemetry/functional_goldens/agent/tool-error-valueerror-schema-v2.json b/tests/unittests/telemetry/functional_goldens/agent/tool-error-valueerror-schema-v2.json index eae27808d83..7c229f0300a 100644 --- a/tests/unittests/telemetry/functional_goldens/agent/tool-error-valueerror-schema-v2.json +++ b/tests/unittests/telemetry/functional_goldens/agent/tool-error-valueerror-schema-v2.json @@ -4,7 +4,8 @@ "attributes": { "gen_ai.operation.name": "invoke_workflow", "gen_ai.conversation.id": "PRESENT", - "gen_ai.workflow.name": "some_root_agent" + "gen_ai.workflow.name": "some_root_agent", + "error.type": "ValueError" }, "status": "ERROR", "children": [ @@ -14,7 +15,8 @@ "gen_ai.operation.name": "invoke_agent", "gen_ai.agent.description": "A sample root agent.", "gen_ai.agent.name": "some_root_agent", - "gen_ai.conversation.id": "PRESENT" + "gen_ai.conversation.id": "PRESENT", + "error.type": "ValueError" }, "status": "ERROR", "children": [ diff --git a/tests/unittests/telemetry/test_semconv_conformance.py b/tests/unittests/telemetry/test_semconv_conformance.py index 9f338d661ff..23318e38c83 100644 --- a/tests/unittests/telemetry/test_semconv_conformance.py +++ b/tests/unittests/telemetry/test_semconv_conformance.py @@ -140,18 +140,6 @@ "gen_ai.response.model": "spans carry the requested model only", } -# The cases whose spans are marked ERROR without naming the error. -_CASES_MISSING_ERROR_TYPE: frozenset[str] = frozenset({ - "agent-inference-error-resource-exhausted-schema-v1", - "agent-inference-error-resource-exhausted-schema-v2", - "agent-inference-error-valueerror-schema-v2", - "agent-tool-error-valueerror-schema-v2", -}) - -_MISSING_ERROR_TYPE_REASON: str = ( - "the metric for the same operation records error.type; the span does not" -) - def _case_id(case: FunctionalTestCase) -> str: """Names a case uniquely: the same test id is reused across scenarios.""" @@ -210,18 +198,7 @@ def _xfail(reason: str | None) -> list[pytest.MarkDecorator]: @pytest.mark.parametrize( "case", - [ - pytest.param( - case, - id=_case_id(case), - marks=_xfail( - _MISSING_ERROR_TYPE_REASON - if _case_id(case) in _CASES_MISSING_ERROR_TYPE - else None - ), - ) - for case in _EVERY_CASE - ], + [pytest.param(case, id=_case_id(case)) for case in _EVERY_CASE], ) def test_error_status_implies_error_type(case: FunctionalTestCase) -> None: """A span that ended in an error says which error, as semconv requires."""