feat: add ADK mapper

[liramon2]

Description

The evals framework currently cannot evaluate agents built with Google's Agent Development Kit (ADK), which produce OpenTelemetry spans under the gcp.vertex.agent scope with a unique attribute layout. This PR adds ADKOtelSessionMapper to map ADK spans into Sessions.

Related Issues

#328

Documentation PR

Type of Change

New feature

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.

• I ran hatch run prepare

Checklist

• I have read the CONTRIBUTING document
• I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
• My change is focused and reasonably small; I have split unrelated work into separate PRs
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[strandly-the-agent]

TL;DR — Code-wise approve: no blockers remain at exact head bb158ee2e7ff80ad27b8d6e144251713213b5144.

✅ Focused + adjacent tests: 161 passed, with 11 existing warnings; local tracked diff is clean.
✅ Hosted lint, all 11 unit jobs, integration, CI Gate, and title checks are green; review trigger is waiting.
✅ The latest test-only delta fixes the fixture filename and expectations; the prior fixture blocker is closed.
ℹ️ Latest-only selection for parallel terminal tool results is an accepted, deliberate compatibility limitation—not a defect requiring further changes.
⚠️ GitHub cannot change this edited review's historical CHANGES_REQUESTED state; a maintainer must dismiss that state.
ℹ️ API-owner sign-off remains appropriate for the public behavior, although this repository has no API-review labels.

Final adjudication matrix and staged pass breakdown

Final adjudication

Item	Disposition	Basis
Fixture filename and stale expectations	Fixed	The test-only delta aligns the fixture loader and assertions; focused + adjacent tests pass 161/161.
Parallel terminal tool results	Accepted limitation; nonblocking	The author deliberately chose latest-only behavior for cross-mapper compatibility. agent_response is one string, sibling mappers select one terminal assistant response, every individual tool span remains in history, no evaluator consumes an aggregate, and aggregation would introduce a new schema convention.
Parallel behavior documentation	Accepted decision; optional clarity follow-up	Inline docs promise raw JSON but do not explicitly state latest-only selection for parallel terminal groups. Saying so would improve clarity, but it is not required for this PR.
System-prompt substring assertion	Suppressed	Exact matching would be stronger, but the current assertion exercises the intended behavior and does not create a substantive regression risk.
Prior closed decisions	Remain closed	Thought visibility, multipart extraction, merged bookkeeping exclusion, trace/span ordering, error.type, scalar raw-JSON wrapping, scope-only detection, the external-dict TracedHandler contract, and global tool-catalog policy are not re-raised. Multi-agent ownership remains follow-up #319.

The earlier triage issue comment (issuecomment-5084538480) was based on an incorrect characterization of the delta and is superseded by this exact-head review.

Staged-review disposition

• Correctness / adversarial: no remaining reproducible defect.
• API / DevX: accept latest-only parallel selection as the chosen compatibility contract; API-owner sign-off is still appropriate.
• Test quality: fixture defect fixed; focused + adjacent suite passes 161 tests.
• LLM-context: no blocker; all individual tool spans remain available in history.
• Docs: no required change; explicit parallel latest-only wording would be a nonblocking clarification.
• Aggregation: zero findings retained. The architecture disagreement is not continued as a blocker.

[liramon2]

@strandly-the-agent Review this PR. Consolidate your findings in a single comment (not scattered around). We are going with a threshold of 1e12 because it behaves the same and is aligned with the existing mappers.

[strandly-the-agent]

Verdict: ✅ approve — no blockers remain at 48f2b13. Both findings from my last review are fixed and each is now pinned by a test that fails when the fix is removed. Nothing new found; this supersedes all my earlier reviews on this PR.

✅ String-encoded nanosecond timestamps parse correctly again — cold repro twice: parse_timestamp("1700000000000000000") → 2023-11-14 22:13:20+00:00; a CloudWatch-parsed span with a string startTimeUnixNano now maps to year 2023; string-ns call_llm spans listed later-first select 'LATER', matching the int-ns control.
✅ The scope-filter test now fails when the filter is deleted (it previously could not); the three multi-agent mutation guards still fail as intended.
✅ The _parse_timestamp → parse_timestamp rename is complete — no stale references remain in src/ or tests/, and all six call sites across the ADK, LangChain and OpenInference mappers were updated.
✅ Full suite tests/strands_evals: 1708 passed (at dc1a4ee, delta since is a pure rename); focused mapper/extractor/provider suites at this head: 323 passed; ruff check and mypy -p src (212 files) clean; worktree clean.
✅ Hosted CI green at this SHA: lint, all 11 unit jobs, integration, CI Gate, PR title (only Trigger Strands Review waiting).

What landed and how I verified it

dc1a4ee fixed both open findings:

1. session_mapper.py:97-98 restores if value.isdigit(): return datetime.fromtimestamp(int(value) / 1e9, tz=timezone.utc) as the first statement of the string branch — the shape OTLP JSON actually delivers (uint64 as a decimal string), which cloudwatch_parser.py:173 passes through unchanged.
2. test_string_nanosecond_epoch now asserts year == 2023 and month == 11 instead of blessing the fallback to now(). Removing the restored branch makes it fail (assert 2026 == 2023).
3. TestScopeFiltering::test_foreign_scope_spans_are_dropped now uses a foreign-scope execute_tool span with full gen_ai.tool.* / gcp.vertex.agent.tool_* attributes — one that would convert unconditionally if unfiltered — instead of a parentless generate_content span that _convert_inference_span discarded anyway.

48f2b13 then took poshinchen's naming request: the shared helper is now parse_timestamp (session_mapper.py:77), with call sites updated at adk_otel_session_mapper.py:139,265,302,569,570, langchain_otel_session_mapper.py:454,455 and openinference_session_mapper.py:602,603, plus the test class. I grepped src/ and tests/ for leftovers — clean apart from stale __pycache__ artifacts.

Mutation results (throwaway copies, baseline 250 in tests/strands_evals/mappers): deleting the scope filter → 1 failed (was 250 unchanged before dc1a4ee); forcing the single-trace branch → 2 failed; stripping the stop_at_agents pruning → 1 failed; removing the unclaimed-span fallback → 1 failed; removing the isdigit branch → 1 failed. Every behavior I raised across this series is now guarded.

Accepted, and two things I looked at but am not raising

1e12 threshold — accepted. Agreed with your reasoning: real nanosecond values are ~1.7e18, so 1e12 and 1e15 behave identically for every input any supported path produces, and matching the existing mappers is the right call. Not reopening it.

Short/float digit strings — theoretical only. isdigit() claims any all-digit string, so "2026" → 1970-01-01T00:00:00.000002 and "1700000000" (seconds as a string) → 1970, while "1.7e18" isn't isdigit() and still falls back to now(). I traced every producer feeding this helper: readable_spans_to_dicts (utils.py:230-236) always yields ints from the OTel SDK, and cloudwatch_parser.py:173-174 always yields full ~19-digit nanosecond strings per the OTLP JSON mapping. No supported path can produce those shapes, so this is a docstring caveat at most — your call, and fine to ignore.

Coverage nit. The string-nanosecond path is pinned at the unit level but no map_to_session-level fixture uses a string-typed start_time. One fixture would close it if you're already touching these tests; not worth a round trip on its own.

Housekeeping and closed items

My earlier reviews are now stale CHANGES_REQUESTED states from older heads with no remaining findings: PRR_kwDOPWQYlM8AAAABHh-C6Q and PRR_kwDOPWQYlM8AAAABHhnPqQ (b9854bc), PRR_kwDOPWQYlM8AAAABHaA_CQ (cf40376), PRR_kwDOPWQYlM8AAAABHZI9jg (ae0caee). Please dismiss them so the PR's review state reflects reality — GitHub won't clear them automatically and I can't dismiss my own.

Previously adjudicated items stay closed and are not re-raised: latest-only parallel terminal tool-result selection, the raw {"result": ...} scalar telemetry contract, thought filtering, multipart concatenation, (merged tools) exclusion, error.type with UNSET, scope-only mapper detection, the external-dict/TracedHandler contract, the global tool catalog, and multi-agent ownership (originally deferred to #319, implemented in-PR and reviewed as new code). Two design calls remain yours rather than defects: per-agent Trace splitting is a new cross-mapper pattern (others collapse to root or stay flat), and a pure-delegation coordinator turn yields an AgentInvocationSpan with an empty agent_response while session-level output stays correct.

Routing: triage GO on a small, well-targeted delta → correctness and test-quality (with mutation testing) run independently; API, LLM-context and docs passes skipped as not applicable. As always, this is solid work for a human to approve — the merge call is yours.

[poshinchen]

@strandly-the-agent review the PR before I approve it.

[github-actions]

Assessment: Comment (non-blocking) — code is functionally solid; main concern is PR scope.

The ADK mapper itself is well-structured, mirrors the existing SessionMapper pattern, and is thoroughly tested (159 mapper tests pass locally, ruff clean). The shared-helper refactor (parse_timestamp/safe_json_parse lifted into the base class + utils) is a clean deduplication, and the new multi-agent trace splitting is well covered.

Review themes

• Scope / focus: This PR is billed as "add ADK mapper" but also bundles (1) smolagents support in OpenInferenceSessionMapper (~+279 lines + a new 206-line fixture — a distinct feature), (2) a behavioral change to Strands CloudWatch split-format detection in detect_otel_mapper, and (3) unrelated actions/setup-python v6→v7 workflow bumps. At ~2661 insertions this sits well past the "focused and reasonably small" bar the checklist claims. The smolagents work and the workflow bumps in particular would be easier to review and revert independently as separate PRs.
• Correctness / side effects: One Important item flagged inline — smolagents normalization mutates the caller's span dicts in place, making map_to_session non-idempotent on its input. See the inline comment on openinference_session_mapper.py.
• API surface: This adds a new public export (ADKOtelSessionMapper). The repo has no API-review labels, but since it follows the established mapper contract exactly, an API-owner sign-off is likely sufficient rather than a full API review.
• Minor: Inconsistent defensive attribute access across ADK helpers (span.get("attributes") or {} vs span.get("attributes", {})); currently masked by the surrounding try/except.

Nice work handling the ADK-specific span hierarchy and the Gemini 2.x vs 3+ tool-call-id differences.

[strandly-the-agent]

Verdict: ✅ approve — no blockers at 1473ea4, and the force-push introduced no new risk. Re-reviewed from scratch on the rebased head; everything I previously blocked on stays fixed and every guard is still live.

⚠️ One mea culpa first: my answer to your SCOPE_ADK question sat in an unsubmitted pending review since yesterday, so you never saw it — it was stale against the old head and I've cleared it. Substance below; you'd already reached the same conclusion ("nvm, it's correct"), and you were right.

✅ Source is unchanged from what I approved at 48f2b13 — all seven PR-owned source/test files are byte-identical; the only new PR content is the thought_signature → MOCK_SIGNATURE fixture redaction. The rest of git diff 48f2b13..HEAD is main moving (merge-base is now c5461d0).
✅ Full suite tests/strands_evals: 1715 passed (with the langfuse/otel extras); focused mapper/extractor/provider suites: 323 passed; ADK mapper file alone: 34 passed.
✅ ruff check src tests clean; ruff format --check clean on the PR files; mypy -p src reports only 2 import-not-found errors for optional extras, identical on the pre-PR merge-base — environmental, not yours.
✅ Cold repros at this head, run twice: string-nanosecond timestamps → 2023 and order correctly; the multi-agent split gives one Trace per invoke_agent with no cross-agent tool leakage and no dropped orphan tool span; a fully-populated foreign-scope tool span is dropped; the committed fixture maps end to end (tool id uwpsprd2, tokens 157+32 / 222+20).
✅ Hosted CI green at this SHA: lint, all 11 unit jobs, integration, CI Gate, PR title.

The fixture redaction is inert — verified, not assumed

thought_signature appears nowhere in src/ or tests/ outside the fixture JSON: the mapper only reads the thought boolean marker (adk_otel_session_mapper.py:452,481,502,518,544), and no test asserts on the signature, its length, or the surrounding part shape.

Stronger check than reading the code: I mapped the old fixture (git show 48f2b13:…adk_live_spans.json, real base64 signatures) and the new one through map_to_session() and diffed the entire resulting Session tree — byte-identical. All three occurrences sit in that ignored sibling field (turn-1 llm_response, the embedded history in turn-2's llm_request, turn-2's llm_response). The load-bearing thought: true filtering keeps its own synthetic coverage at test_adk_otel_session_mapper.py:311-316, untouched. The fixture still parses as valid ADK 2.5 shape — seven spans, one scope, embedded JSON payloads all well-formed — so the redaction cost nothing but the secret-looking blob.

Rebase interactions and mutation guards

Rebase. The main-side delta the force-push pulled in (output_evaluator.py, trajectory_evaluator.py, the multimodal evaluators, a pyproject.toml bump) is additive tools: list[Any] | None plumbing. It touches neither AgentInvocationSpan.agent_response, nor Trace/Session shape, nor anything keyed on trace_id — so the two ADK-specific shapes worth worrying about (an empty agent_response on a pure-delegation coordinator turn, and multiple Trace objects sharing one trace_id) don't meet any new assumption. The green full suite confirms it.

Mutation guards, all four still live (run against throwaway copies via PYTHONPATH, since the editable install otherwise resolves back to the pristine tree and the mutation silently no-ops):

Mutation	Result
Force len(agent_spans_raw) <= 1 always true	2 failed (TestMultiAgentSplitting)
Delete the scope-filter line in map_to_session	1 failed (test_foreign_scope_spans_are_dropped)
Strip the stop_at_agents pruning in _get_descendants	1 failed (specialist-tool-leak assertion)
Remove the isdigit branch in parse_timestamp	1 failed (test_string_nanosecond_epoch)

Every behaviour I raised across this review series fails at least one test when broken — that's what makes this safe to land.

The SCOPE_ADK answer that got stuck (for the record)

The value is correct: verified against google-adk==2.5.0, where ADK's tracer registers with instrumenting_module_name="gcp.vertex.agent" (google/adk/telemetry/tracing.py:102-105), and the same string is used for its logger (:109) and meter (telemetry/_metrics.py:43). That's the scope name landing on every span this mapper consumes.

On the name, SCOPE_ADK is right: these constants are keyed by producing framework, not by the value's namespace (SCOPE_STRANDS = "strands.telemetry.tracer"), and the _OTEL/_SMOLAGENTS suffixes exist only to disambiguate multiple instrumentation libraries for one framework. There's a single ADK-emitted scope, so _GCP would disambiguate nothing and would read as "the GCP flavour of ADK".

Worth knowing anyway: ADK also emits under google.adk.plugins.bigquery_agent_analytics (google/adk/plugins/bigquery_agent_analytics_plugin.py:85) — an opt-in analytics plugin, not the invoke_agent/call_llm/execute_tool hierarchy, and the scope filter correctly drops it. No change needed; it just means "ADK spans" isn't one scope forever, so this constant is mildly version-sensitive if a future release moves the core tracer to a google.adk.* name.

Closed items and the agreed follow-up

Not re-raised, all settled earlier: latest-only parallel terminal tool-result selection; the raw {"result": …} scalar telemetry contract; thought filtering; multipart concatenation; (merged tools) exclusion; error.type with UNSET; scope-only mapper detection; the external-dict/TracedHandler contract; the global tool catalog; the 1e12 threshold (your call, and I agree — identical behaviour for real ~1.7e18 values); and short/float digit-string timestamps (theoretical, no producer emits them).

Agreed follow-up, deliberately out of scope here: TraceExtractor only picking the first AgentInvocationSpan per Trace. liramon2 reproduced the same low-score symptom with the LangChain OTel mapper, so it's framework-agnostic rather than ADK-specific, and you both landed on fixing it separately — which I think is right: the per-agent Trace split here is the pragmatic workaround, and reverting it to wait on the extractor fix would just ship known-bad multi-agent scores meanwhile. Worth filing that follow-up before this merges so the workaround doesn't quietly become the convention; the shape is roughly "attribute per agent span instead of per trace" in extractors/trace_extractor.py:87-133.

Routing: triage GO on a rebase-plus-fixture delta → one independent correctness/test pass with mutation testing; API, LLM-context and docs skipped as not applicable. As ever, solid work — the merge call is yours.