Repository navigation
Do not pass completion hook to retrieval and embedding invocations - #980
DylanRussell wants to merge 16 commits into
Conversation
Ensures that local agent, workflow, retrieval, and embedding invocations do not trigger the completion/upload hook, avoiding duplicate uploads of messages handled by child inference spans. Remote agent invocations still call the completion hook because there are no client-side inference spans in that scenario.
Pull request dashboard statusWaiting on maintainers · refreshed 2026-11-09 18:37 UTC Merge when ready. Also blocked by: 1 required status check is failing. Status above doesn't look right?
|
There was a problem hiding this comment.
🟢 Approval recommended
The scoped hook changes match the stated behavior, with regression coverage and no identified blocking issues.
No open issues 🎉
What changed in this PR
Updates shared GenAI utilities to prevent duplicate completion uploads from local agent and workflow invocations. Retrieval and embedding invocations also bypass the hook, while inference and remote-agent hooks remain enabled.
Changes:
- Routes excluded invocation types through no-op completion hooks.
- Moves agent completion-hook execution into remote-agent finalization.
- Updates regression tests for hook suppression and preserved remote-agent behavior.
| File | Description |
|---|---|
util/opentelemetry-util-genai/tests/test_handler_completion_hook.py |
Verifies hook suppression and remote-agent hook behavior. |
util/opentelemetry-util-genai/src/opentelemetry/util/genai/handler.py |
Supplies no-op hooks for excluded invocation types. |
util/opentelemetry-util-genai/src/opentelemetry/util/genai/_workflow_invocation.py |
Disables workflow completion hooks. |
util/opentelemetry-util-genai/src/opentelemetry/util/genai/_retrieval_invocation.py |
Disables retrieval hooks, including suppressed invocations. |
util/opentelemetry-util-genai/src/opentelemetry/util/genai/_embedding_invocation.py |
Uses a no-op hook for embeddings. |
util/opentelemetry-util-genai/src/opentelemetry/util/genai/_agent_invocation.py |
Restricts agent completion hooks to remote invocations. |
Review effort: Balanced
lmolkova
left a comment
lmolkova
left a comment
There was a problem hiding this comment.
Agent and workflow input/output aren't duplicates of the inference messages. From the inference spans alone, we can't tell which inference output became the final answer, or what the agent got as input before it built the prompts. Frameworks post-process, merge, retry, and pick between candidates. The agent span is the only place that has the real request and answer, which is why we record them there too. Some turns also skip the LLM entirely (like the qwenpaw /stop test), so nothing would reach the hook.
I think the hook should run wherever we capture messages and be skipped only where there are none. Could we scope this PR to retrieval and embedding and keep the hook on local agent and workflow? I opened open-telemetry/semantic-conventions-genai#649 to get this documented per operation in semconv.
|
OK that makes sense. Reverted the changes to agent/workflow. Added a comment to the bug |
c141481 to
df2c829
Compare
lmolkova
left a comment
lmolkova
left a comment
There was a problem hiding this comment.
The PR title/description also still mention local agent and workflow
DylanRussell
changed the title
DylanRussell
added
Skip Changelog
3f2cd12 to
52137ab
Compare
This comment has been minimized.
This comment has been minimized.
|
/dashboard route:reviewers |
|
@DylanRussell, your reviewer-routing request was recorded; this pull request has the approvals it needs and is now waiting on maintainers. The handoff remains active across pushes until newer actionable human feedback arrives. Top-level feedback through this request will not return; unresolved review threads remain open. |
Retrieval / Embedding do not currently use the hook, so no need to pass it.
Tests
updated unit tests.
Type of change
Checklist