Skip to content

Commit 6396ce6

Browse files
GWealecopybara-github
authored andcommitted
fix(telemetry): mark failed tool spans as errors and fix provider naming
Co-authored-by: George Weale <gweale@google.com> PiperOrigin-RevId: 957285853
1 parent c12a025 commit 6396ce6

9 files changed

Lines changed: 589 additions & 22 deletions

File tree

src/google/adk/telemetry/_instrumentation.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ async def record_tool_execution(
209209
"""Unified context manager for consolidated tool execution telemetry."""
210210
start_time = time.monotonic()
211211
caught_error: Exception | None = None
212+
detected_error_type: str | None = None
212213
span: trace.Span | None = None
213214
span_name = f"execute_tool {tool.name}"
214215
try:
@@ -221,6 +222,7 @@ async def record_tool_execution(
221222
caught_error = e
222223
raise
223224
finally:
225+
detected_error_type = tel_ctx.error_type
224226
response_event = (
225227
tel_ctx.function_response_event if caught_error is None else None
226228
)
@@ -241,6 +243,7 @@ async def record_tool_execution(
241243
agent_name=agent.name,
242244
elapsed_s=_metrics.get_elapsed_s(span, start_time),
243245
error=caught_error,
246+
error_type=detected_error_type,
244247
)
245248
except Exception: # pylint: disable=broad-exception-caught
246249
logger.exception(

src/google/adk/telemetry/_metrics.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,15 +188,28 @@ def record_tool_execution_duration(
188188
agent_name: str,
189189
elapsed_s: float,
190190
error: Exception | None = None,
191+
error_type: str | None = None,
191192
):
192-
"""Records the duration of the tool execution."""
193+
"""Records the duration of the tool execution.
194+
195+
Args:
196+
tool_name: Name of the tool that ran.
197+
tool_type: Class name of the tool that ran.
198+
agent_name: Name of the agent that ran the tool.
199+
elapsed_s: Duration of the tool execution, in seconds.
200+
error: The exception raised by the tool, if any.
201+
error_type: An error type detected from a tool response that reported a
202+
failure without raising. Ignored when `error` is also set.
203+
"""
193204
attrs = {
194205
gen_ai_attributes.GEN_AI_AGENT_NAME: agent_name,
195206
gen_ai_attributes.GEN_AI_TOOL_NAME: tool_name,
196207
gen_ai_attributes.GEN_AI_TOOL_TYPE: tool_type,
197208
}
198209
if error is not None:
199210
attrs[error_attributes.ERROR_TYPE] = tracing.resolve_error_type(error)
211+
elif error_type is not None:
212+
attrs[error_attributes.ERROR_TYPE] = error_type
200213
_tool_execution_duration.record(elapsed_s, attributes=attrs)
201214

202215

@@ -212,7 +225,9 @@ def record_client_operation_duration(
212225
attrs = {
213226
gen_ai_attributes.GEN_AI_AGENT_NAME: agent_name,
214227
gen_ai_attributes.GEN_AI_OPERATION_NAME: "generate_content",
215-
gen_ai_attributes.GEN_AI_PROVIDER_NAME: _get_provider_name(),
228+
gen_ai_attributes.GEN_AI_PROVIDER_NAME: _get_provider_name(
229+
llm_request.model
230+
),
216231
}
217232
if llm_request.model:
218233
attrs[gen_ai_attributes.GEN_AI_REQUEST_MODEL] = llm_request.model
@@ -262,7 +277,9 @@ def record_client_token_usage(
262277
base_attrs = {
263278
gen_ai_attributes.GEN_AI_AGENT_NAME: agent_name,
264279
gen_ai_attributes.GEN_AI_OPERATION_NAME: "generate_content",
265-
gen_ai_attributes.GEN_AI_PROVIDER_NAME: _get_provider_name(),
280+
gen_ai_attributes.GEN_AI_PROVIDER_NAME: _get_provider_name(
281+
llm_request.model
282+
),
266283
}
267284
if llm_request.model:
268285
base_attrs[gen_ai_attributes.GEN_AI_REQUEST_MODEL] = llm_request.model
@@ -280,8 +297,8 @@ def record_client_token_usage(
280297
_client_token_usage.record(output_token_count, attributes=output_attrs)
281298

282299

283-
def _get_provider_name() -> str:
284-
return tracing._guess_gemini_system_name()
300+
def _get_provider_name(model: str | None) -> str:
301+
return tracing._resolve_gen_ai_system_name(model)
285302

286303

287304
def get_elapsed_s(

src/google/adk/telemetry/tracing.py

Lines changed: 104 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from contextlib import asynccontextmanager
3030
from contextlib import contextmanager
3131
import logging
32+
import re
3233
from typing import Final
3334
from typing import TYPE_CHECKING
3435

@@ -54,11 +55,14 @@
5455
from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
5556
from opentelemetry.semconv.schemas import Schemas
5657
from opentelemetry.trace import Span
58+
from opentelemetry.trace import Status
59+
from opentelemetry.trace import StatusCode
5760
from opentelemetry.util.types import AttributeValue
5861
from typing_extensions import deprecated
5962

6063
from .. import version
6164
from ..utils.env_utils import is_enterprise_mode_enabled
65+
from ..utils.model_name_utils import extract_model_name
6266
from ..utils.model_name_utils import is_gemini_model
6367
from ._experimental_semconv import maybe_log_completion_details
6468
from ._experimental_semconv import set_operation_details_attributes_from_request
@@ -98,6 +102,7 @@
98102
from ..models.llm_request import LlmRequest
99103
from ..models.llm_response import LlmResponse
100104
from ..tools.base_tool import BaseTool
105+
from ..workflow._base_node import BaseNode
101106

102107
tracer = trace.get_tracer(
103108
instrumenting_module_name="gcp.vertex.agent",
@@ -212,10 +217,19 @@ def trace_tool_call(
212217
):
213218
span.set_attribute(GEN_AI_AGENT_NAME, agent.name)
214219

220+
failure_type: str | None = None
215221
if error is not None:
216-
span.set_attribute(ERROR_TYPE, resolve_error_type(error))
222+
failure_type = resolve_error_type(error)
223+
span.record_exception(error)
217224
elif error_type is not None:
218-
span.set_attribute(ERROR_TYPE, error_type)
225+
failure_type = error_type
226+
if failure_type is not None:
227+
span.set_attribute(ERROR_TYPE, failure_type)
228+
# Without an explicit error status the span renders as successful, which
229+
# hides tools that reported a failure as a response dict instead of
230+
# raising. The description repeats the type rather than the error message
231+
# so no tool content lands in an attribute the content toggle cannot gate.
232+
span.set_status(Status(StatusCode.ERROR, failure_type))
219233

220234
# Special case for client side association with a remote tool call
221235
if (
@@ -742,14 +756,7 @@ def _use_extra_generate_content_attributes(
742756

743757

744758
def _is_gemini_agent(agent: BaseAgent) -> bool:
745-
from ..agents.llm_agent import LlmAgent
746-
747-
if not isinstance(agent, LlmAgent):
748-
return False
749-
750-
model = agent.model if agent.model != "" else agent._default_model
751-
model_name = model if isinstance(model, str) else model.model
752-
return is_gemini_model(model_name)
759+
return is_gemini_model(_agent_model_name(agent))
753760

754761

755762
def _set_common_generate_content_attributes(
@@ -770,10 +777,11 @@ def _use_native_generate_content_span_stable_semconv(
770777
telemetry_config: TelemetryConfig | None = None,
771778
) -> Iterator[GenerateContentSpan]:
772779
telemetry_config = telemetry_config or TelemetryConfig()
780+
system_name = _resolve_gen_ai_system_name(llm_request.model)
773781
with tracer.start_as_current_span(
774782
f"generate_content {llm_request.model or ''}"
775783
) as span:
776-
span.set_attribute(GEN_AI_SYSTEM, _guess_gemini_system_name())
784+
span.set_attribute(GEN_AI_SYSTEM, system_name)
777785
_set_common_generate_content_attributes(
778786
span, llm_request, common_attributes
779787
)
@@ -783,10 +791,10 @@ def _use_native_generate_content_span_stable_semconv(
783791
LogRecord(
784792
event_name=GEN_AI_SYSTEM_MESSAGE_EVENT,
785793
body=system_message_body(llm_request, telemetry_config),
786-
attributes={GEN_AI_SYSTEM: _guess_gemini_system_name()},
794+
attributes={GEN_AI_SYSTEM: system_name},
787795
)
788796
)
789-
user_message_attributes = {GEN_AI_SYSTEM: _guess_gemini_system_name()}
797+
user_message_attributes = {GEN_AI_SYSTEM: system_name}
790798
if (
791799
telemetry_config.should_add_content_to_logs
792800
and log_only_common_attributes
@@ -871,7 +879,9 @@ def trace_generate_content_result(span: Span | None, llm_response: LlmResponse):
871879
LogRecord(
872880
event_name=GEN_AI_CHOICE_EVENT,
873881
body=choice_body(llm_response, TelemetryConfig()),
874-
attributes={GEN_AI_SYSTEM: _guess_gemini_system_name()},
882+
attributes={
883+
GEN_AI_SYSTEM: _inference_system_name(None, llm_response)
884+
},
875885
)
876886
)
877887

@@ -916,7 +926,11 @@ def trace_inference_result(
916926
body=choice_body(
917927
llm_response, telemetry_config or TelemetryConfig()
918928
),
919-
attributes={GEN_AI_SYSTEM: _guess_gemini_system_name()},
929+
attributes={
930+
GEN_AI_SYSTEM: _inference_system_name(
931+
invocation_context, llm_response
932+
)
933+
},
920934
)
921935
)
922936

@@ -927,3 +941,78 @@ def _guess_gemini_system_name() -> str:
927941
if is_enterprise_mode_enabled()
928942
else GenAiSystemValues.GEMINI.name.lower()
929943
)
944+
945+
946+
# Anthropic models reach ADK either as a bare `claude-*` id (the built-in
947+
# Anthropic backend, and the Vertex `publishers/anthropic/models/...` path once
948+
# normalized) or behind a LiteLLM `anthropic/...` prefix, which the generic
949+
# prefix rule below already covers.
950+
_ANTHROPIC_MODEL_PATTERN: Final = re.compile(r"^claude[-.]", re.IGNORECASE)
951+
952+
# Leading segments of a resource-path model id, e.g. a Model Garden path like
953+
# `projects/<p>/locations/<l>/publishers/<pub>/models/<m>` or a tuned-model id
954+
# like `tunedModels/<id>`. They name a resource collection, never a provider,
955+
# so the provider-prefix rule must not read one as one.
956+
_RESOURCE_COLLECTION_SEGMENTS: Final = frozenset({
957+
"endpoints",
958+
"locations",
959+
"models",
960+
"projects",
961+
"publishers",
962+
"tunedmodels",
963+
})
964+
965+
966+
def _resolve_gen_ai_system_name(model: str | None) -> str:
967+
"""Returns the `gen_ai.system` / `gen_ai.provider.name` value for a model.
968+
969+
The name has to follow the model actually being served, otherwise every
970+
provider is reported as Gemini. A LiteLLM-style `<provider>/<model>` id
971+
carries the provider in its prefix, which semantic conventions allow as a
972+
lowercased name outside their well-known set. The prefix is read off the bare
973+
model name so that a resource path, whose leading segments describe where the
974+
model lives rather than who serves it, is not mistaken for one. When no model
975+
id is available, or the id names no provider, the deployment-derived
976+
Gemini/Vertex name is used, since Gemini is the backend ADK talks to
977+
natively.
978+
979+
Args:
980+
model: The model id the request is being served by, if known.
981+
"""
982+
if not model or is_gemini_model(model):
983+
return _guess_gemini_system_name()
984+
985+
model_name = extract_model_name(model)
986+
if _ANTHROPIC_MODEL_PATTERN.match(model_name):
987+
return GenAiSystemValues.ANTHROPIC.name.lower()
988+
989+
provider, separator, _ = model_name.partition("/")
990+
provider = provider.lower()
991+
if separator and provider and provider not in _RESOURCE_COLLECTION_SEGMENTS:
992+
return provider
993+
994+
return _guess_gemini_system_name()
995+
996+
997+
def _agent_model_name(agent: BaseAgent | BaseNode) -> str | None:
998+
"""Returns the model id configured on an agent, if it has one."""
999+
from ..agents.llm_agent import LlmAgent
1000+
1001+
if not isinstance(agent, LlmAgent):
1002+
return None
1003+
1004+
model = agent.model if agent.model != "" else agent._default_model
1005+
return model if isinstance(model, str) else model.model
1006+
1007+
1008+
def _inference_system_name(
1009+
invocation_context: InvocationContext | None,
1010+
llm_response: LlmResponse,
1011+
) -> str:
1012+
"""Returns the system name of the model that produced an inference result."""
1013+
model = llm_response.model_version
1014+
if not model and invocation_context is not None:
1015+
agent = invocation_context.agent
1016+
if agent is not None:
1017+
model = _agent_model_name(agent)
1018+
return _resolve_gen_ai_system_name(model)

0 commit comments

Comments
 (0)