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 a11a770b05..3eae4f80e8 100644 --- a/src/google/adk/flows/llm_flows/core/_model_call.py +++ b/src/google/adk/flows/llm_flows/core/_model_call.py @@ -33,8 +33,8 @@ from ....models.llm_request import LlmRequest from ....models.llm_response import LlmResponse from ....telemetry import _instrumentation +from ....telemetry.tracing import start_as_current_span from ....telemetry.tracing import trace_call_llm -from ....telemetry.tracing import tracer from ....utils._runner_utils import _with_caller_context from ....utils.context_utils import Aclosing from ._finalizer import has_meaningful_content @@ -184,7 +184,7 @@ async def call_llm_async( caller_context = otel_context.get_current() async def _call_llm_with_tracing() -> AsyncGenerator[LlmResponse, None]: - with tracer.start_as_current_span('call_llm') as span: + with start_as_current_span('call_llm') as span: # Runs before_model_callback inside the call_llm span so # plugins observe the same span as after/error callbacks. if response := await flow._handle_before_model_callback( diff --git a/src/google/adk/telemetry/_instrumentation.py b/src/google/adk/telemetry/_instrumentation.py index 2788b377ed..8779f95569 100644 --- a/src/google/adk/telemetry/_instrumentation.py +++ b/src/google/adk/telemetry/_instrumentation.py @@ -153,7 +153,7 @@ def record_invocation( Nothing; the span (if any) is active for the duration of the block. """ if resolve_schema_version() < SCHEMA_VERSION_SEMCONV_ALIGNED: - with tracing.tracer.start_as_current_span("invocation"): + with tracing.start_as_current_span("invocation"): yield return @@ -582,7 +582,7 @@ async def record_agent_invocation( context_api.set_value(_AGENT_INVOCATION_SCOPE_KEY, scope) ) try: - with tracing.tracer.start_as_current_span(span_name) as s: + with tracing.start_as_current_span(span_name) as s: span = s tracing.trace_agent_invocation(span, agent, ctx) yield scope diff --git a/src/google/adk/telemetry/node_tracing.py b/src/google/adk/telemetry/node_tracing.py index 66132378a4..00ffc0a4ef 100644 --- a/src/google/adk/telemetry/node_tracing.py +++ b/src/google/adk/telemetry/node_tracing.py @@ -28,6 +28,7 @@ from opentelemetry import context as context_api 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 Span from opentelemetry.trace import Status from opentelemetry.trace import StatusCode @@ -37,7 +38,8 @@ from ..workflow._base_node import BaseNode from .context import TelemetryConfig from .tracing import _telemetry_config_from_invocation_context -from .tracing import tracer +from .tracing import resolve_error_type +from .tracing import start_as_current_span if TYPE_CHECKING: from opentelemetry.util.types import AttributeValue @@ -153,7 +155,7 @@ def _invoke_node_span( context: Context, node: BaseNode ) -> Iterator[TelemetryContext]: """Opens an `invoke_node` span for a plain node.""" - with tracer.start_as_current_span( + with start_as_current_span( f"invoke_node {node.name}", attributes={ GEN_AI_OPERATION_NAME: "invoke_node", @@ -247,7 +249,7 @@ def _use_invoke_workflow_span( recorded_error: BaseException | None = None try: with ( - tracer.start_as_current_span( + start_as_current_span( name=span_name, attributes=attributes, context=otel_context, @@ -272,6 +274,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 adfe0b00c7..18810ccac6 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/scenarios/telemetry_setup.py b/tests/unittests/telemetry/functional/scenarios/telemetry_setup.py index edf1d7a408..36e3685701 100644 --- a/tests/unittests/telemetry/functional/scenarios/telemetry_setup.py +++ b/tests/unittests/telemetry/functional/scenarios/telemetry_setup.py @@ -24,7 +24,6 @@ from typing import NamedTuple from google.adk.telemetry import _metrics -from google.adk.telemetry import node_tracing from google.adk.telemetry import tracing from opentelemetry.sdk._logs import LoggerProvider from opentelemetry.sdk._logs.export import InMemoryLogRecordExporter @@ -235,13 +234,12 @@ def install_telemetry( tracer_provider.add_span_processor(SimpleSpanProcessor(span_exporter)) real_tracer = tracer_provider.get_tracer(__name__) - for module in (tracing, node_tracing): - monkeypatch.setattr( - module.tracer, - "start_as_current_span", - real_tracer.start_as_current_span, - ) - monkeypatch.setattr(module.tracer, "start_span", real_tracer.start_span) + monkeypatch.setattr( + tracing.tracer, + "start_as_current_span", + real_tracer.start_as_current_span, + ) + monkeypatch.setattr(tracing.tracer, "start_span", real_tracer.start_span) logger_provider = LoggerProvider() logger_provider.add_log_record_processor( diff --git a/tests/unittests/telemetry/functional_divergences.json b/tests/unittests/telemetry/functional_divergences.json index 695163127d..652cb67331 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 9a339aa31f..35d0b261d0 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 57cb14843f..41611118c5 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 632e77b714..fe73c7cb28 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 eae27808d8..7c229f0300 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 7f471e7fb1..e5e1bdd815 100644 --- a/tests/unittests/telemetry/test_semconv_conformance.py +++ b/tests/unittests/telemetry/test_semconv_conformance.py @@ -139,18 +139,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.""" @@ -209,18 +197,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."""