Skip to content

fix(telemetry): set error.type on spans that end in an error - #7495

Open
ankit2235 wants to merge 1 commit into
google:mainfrom
ankit2235:fix/telemetry-span-error-type
Open

ankit2235 wants to merge 1 commit into
google:mainfrom
ankit2235:fix/telemetry-span-error-type

Conversation

@ankit2235

Copy link
Copy Markdown

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
When a model call or tool fails, OpenTelemetry marks every span the exception passes through as ERROR, but only execute_tool sets error.type. The invocation / invoke_workflow, invoke_agent, call_llm and generate_content spans end in ERROR without saying which error it was, even though the metrics for the same operations record it (429, ValueError, ...). test_error_status_implies_error_type tracked this as a strict xfail for four cases.

Solution:

  • Added tracing.start_as_current_span(), a small wrapper around tracer.start_as_current_span() that sets error.type from resolve_error_type() when an exception escapes the span, then re-raises. It only catches Exception, the same cases where OpenTelemetry sets the ERROR status.
  • The invocation, invoke_agent, call_llm, generate_content, invoke_workflow and invoke_node spans are now opened through it.
  • In node_tracing.py, where a node hands its failure back as data and the workflow span is marked ERROR explicitly, error.type is set as well.
  • The functional test harness patched the tracer on both tracing and node_tracing, which is the same object. node_tracing no longer imports tracer, so the harness now patches tracing.tracer once.
  • Removed the four xfail entries and re-recorded the goldens with python -m tests.unittests.telemetry.regenerate.
  • The existing generate_content divergence for error.type was an adk_bug ("leaves error.type off the inference span"). ADK now reports 429 where the OTel instrumentor reports ClientError, so I changed it to desired_behavior with the same reasoning already used for the duration metric's error.type.

Failed spans gain one attribute; nothing is renamed or removed.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
python -m tests.unittests.telemetry.regenerate
65 golden(s) recorded.
32 divergence(s), all explained.

pytest tests/unittests/telemetry -n 4
880 passed, 3 xfailed

pytest tests/unittests -n 4
18450 passed, 86 skipped, 20 xfailed, 2 xpassed

test_error_status_implies_error_type now runs as a normal test for every case and passes.

The error.type entry in _FACTS_MISSING_FROM_SPANS stays as an xfail. What's left there are the skill-script cases: the failing scripts' spans already carry error.type, but the test pairs those metric points with the span of the script that succeeded, since the metric doesn't record which script failed. That's separate from this change.

Manual End-to-End (E2E) Tests:

Covered by the functional telemetry tests, which run real agent invocations against an in-memory exporter. The diff in the four re-recorded goldens shows the new error.type attribute on each span in the failing chain.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

When a model call or tool failed, OpenTelemetry marked every span the
exception passed through as ERROR, but only execute_tool said which
error it was. The invocation, invoke_workflow, invoke_agent, call_llm
and generate_content spans had no error.type, even though the metrics
for the same operations record it.

Open these spans through a small helper that sets error.type from
resolve_error_type() when an exception escapes, and set it where a
workflow node hands its failure back as data. The functional goldens
for the four error cases are re-recorded, and the generate_content
divergence from the OTel instrumentor is now a value difference (429
vs ClientError), the same one already accepted for the duration metric.

Fixes google#7494
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spans that end in ERROR don't set error.type (only tool spans do)

2 participants