-
Notifications
You must be signed in to change notification settings - Fork 23
fix(streaming): aggregate model info per invocation, not per session #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d394612
1ebefbb
d392a65
db0c40b
fcae4a9
6c8edd7
2764a2c
e8e38f5
ca5792c
b30e477
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,10 @@ class ConversionResult: | |
| trace_id: str | ||
| invocations: list[Invocation] = field(default_factory=list) | ||
| warnings: list[str] = field(default_factory=list) | ||
| # LLM spans each invocation was built from, parallel to ``invocations``. | ||
| # Kept so callers can aggregate per-invocation model info (token counts, | ||
| # models, providers) without walking the whole trace for every invocation. | ||
| invocation_llm_spans: list[list[Span]] = field(default_factory=list) | ||
|
|
||
|
|
||
| def convert_trace(trace: Trace, format: str | None = None) -> ConversionResult: | ||
|
|
@@ -87,12 +91,39 @@ def _convert_adk_trace(trace: Trace) -> ConversionResult: | |
|
|
||
| for invoke_span in invoke_spans: | ||
| try: | ||
| invocation = _convert_invoke_span(invoke_span) | ||
| invocation, llm_spans = _convert_invoke_span(invoke_span) | ||
| result.invocations.append(invocation) | ||
| result.invocation_llm_spans.append(llm_spans) | ||
| except Exception as exc: | ||
| msg = f"Trace {trace.trace_id}: failed to convert invoke_agent span {invoke_span.span_id}: {exc}" | ||
| logger.warning(msg) | ||
| result.warnings.append(msg) | ||
| # Orchestrators like SequentialAgent don't call an LLM themselves, | ||
| # so after pruning the invocation has no LLM descendants and the | ||
| # converter raises. Dropping the whole invocation would silently | ||
| # shrink a 3-step trace to 2 rows and bias every per-invocation | ||
| # count downstream, so we keep it: the empty ``invocation_llm_spans`` | ||
| # slot ensures token totals stay honest, and we fall back | ||
| # ``user_content`` / ``final_response`` to the previous invocation | ||
| # so callers still get a renderable row. Without a previous | ||
| # invocation (the rare first-span-failed case) we emit an empty | ||
| # Content so the row is still well-formed. | ||
| prev = result.invocations[-1] if result.invocations else None | ||
| fallback = prev.user_content if prev is not None else genai_types.Content( | ||
| role="user", parts=[] | ||
| ) | ||
|
Comment on lines
+111
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two things here:
|
||
| fallback_response = prev.final_response if prev is not None else genai_types.Content( | ||
| role="model", parts=[] | ||
| ) | ||
| result.invocations.append( | ||
| Invocation( | ||
| invocation_id=invoke_span.get_tag(ADK_INVOCATION_ID, invoke_span.span_id), | ||
| user_content=fallback, | ||
| final_response=fallback_response, | ||
| creation_timestamp=invoke_span.start_time / 1_000_000.0, | ||
| ) | ||
| ) | ||
| result.invocation_llm_spans.append([]) | ||
|
|
||
| return result | ||
|
|
||
|
|
@@ -127,7 +158,7 @@ def _find_adk_spans(trace: Trace, operation: str) -> list[Span]: | |
| return matches | ||
|
|
||
|
|
||
| def _convert_invoke_span(invoke_span: Span) -> Invocation: | ||
| def _convert_invoke_span(invoke_span: Span) -> tuple[Invocation, list[Span]]: | ||
| llm_spans = find_adk_llm_spans_in(invoke_span) | ||
|
LeonxLJX marked this conversation as resolved.
|
||
| if not llm_spans: | ||
| raise ValueError( | ||
|
|
@@ -148,14 +179,16 @@ def _convert_invoke_span(invoke_span: Span) -> Invocation: | |
|
|
||
| invocation_id = invoke_span.get_tag(ADK_INVOCATION_ID, invoke_span.span_id) | ||
|
|
||
| return Invocation( | ||
| invocation = Invocation( | ||
| invocation_id=invocation_id, | ||
| user_content=user_content, | ||
| final_response=final_response, | ||
| intermediate_data=intermediate_data, | ||
| creation_timestamp=invoke_span.start_time / 1_000_000.0, | ||
| ) | ||
|
|
||
| return invocation, llm_spans | ||
|
|
||
|
|
||
| def _find_children_by_op(root: Span, op_prefix: str) -> list[Span]: | ||
| results: list[Span] = [] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -482,16 +482,22 @@ def collect(span: Span) -> None: | |
| elif is_adk_generate_content_llm_span(span): | ||
| generate_content_spans.append(span) | ||
|
|
||
| _walk_descendants(root, collect) | ||
| _walk_descendants(root, collect, skip_invoke_agents=True) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This loses an invocation. A |
||
| call_llm_spans.sort(key=lambda s: s.start_time) | ||
| generate_content_spans.sort(key=lambda s: s.start_time) | ||
| return call_llm_spans or generate_content_spans | ||
|
|
||
|
|
||
| def _walk_descendants(span: Span, visit) -> None: | ||
| def _walk_descendants(span: Span, visit, skip_invoke_agents: bool = False) -> None: | ||
| for child in span.children: | ||
| # When collecting the LLM spans that belong to one invocation, a nested | ||
| # invoke_agent span is a separate invocation: its subtree's LLM spans are | ||
| # attributed to that child invocation, so walking into it here would | ||
| # double-count them. | ||
| if skip_invoke_agents and child.operation_name.startswith("invoke_agent"): | ||
| continue | ||
| visit(child) | ||
| _walk_descendants(child, visit) | ||
| _walk_descendants(child, visit, skip_invoke_agents=skip_invoke_agents) | ||
|
|
||
|
|
||
| def is_llm_span(span: Span) -> bool: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.