test(e2e/node-eve): Assert a manual span nests under gen_ai.execute_tool - #24330
test(e2e/node-eve): Assert a manual span nests under gen_ai.execute_tool#24330mydea wants to merge 1 commit into
Conversation
size-limit report 📦
|
Wrap the `get_weather` tool's work in a manual `Sentry.startSpan` and assert in the eve e2e test that the resulting user span nests directly under the SDK's `gen_ai.execute_tool` span (same trace, parent = the tool span). This exercises the manual-instrumentation-inside-a-tool path and proves user spans slot into the active gen_ai context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3951e4d to
9936ac2
Compare
| // directly under it — this is the manual-instrumentation-inside-a-tool case. | ||
| const manualSpan = traceSpans.find(span => getSpanOp(span) === 'gen_ai.tool.manual'); | ||
| expect(manualSpan?.name).toBe('resolve-weather'); | ||
| expect(manualSpan?.attributes?.['weather.city']?.value).toBe('Paris'); |
There was a problem hiding this comment.
Bug: The test assertion toBe('Paris') for the LLM-derived weather.city is too strict and will cause flakiness, as LLM output is non-deterministic.
Severity: MEDIUM
Suggested Fix
Change the assertion from expect(weather.city).toBe('Paris') to expect(weather.city).toContain('Paris'). This makes the test more robust against non-deterministic LLM output variations, consistent with the existing test on line 64.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: dev-packages/e2e-tests/test-applications/node-eve/tests/eve.test.ts#L74
Potential issue: The test at line 74 asserts that the LLM-derived `weather.city`
attribute is exactly `'Paris'`. However, LLM output is non-deterministic and may produce
variations like `'paris'` or `'Paris, France'`. This strict `toBe('Paris')` assertion
introduces flakiness, as it will fail on valid but differently formatted outputs. A
similar, pre-existing assertion on line 64 correctly uses `toContain('Paris')` to handle
this variability, indicating the new assertion is overly strict.
Did we get this right? 👍 / 👎 to inform future reviews.
| ['gen_ai.invoke_agent', 'gen_ai.generate_content', 'gen_ai.execute_tool', 'gen_ai.tool.manual'].every(op => | ||
| spansOfTrace.some(span => getSpanOp(span) === op), | ||
| ) && spansOfTrace.some(isAgentServerSpan), | ||
| ); |
There was a problem hiding this comment.
Bug: The test may hang indefinitely if the 'gen_ai.tool.manual' span is not emitted, as the completion predicate in collectStreamedSpans will never resolve.
Severity: MEDIUM
Suggested Fix
Add a timeout to the collectStreamedSpans utility or the specific test case. This will ensure the test fails quickly with a clear error message if the required spans are not received within a reasonable time, rather than hanging.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: dev-packages/e2e-tests/test-applications/node-eve/tests/eve.test.ts#L32-L35
Potential issue: The test's completion predicate in `collectStreamedSpans` now requires
the presence of a `'gen_ai.tool.manual'` span. The underlying `waitForStreamedSpans`
function has no built-in timeout. If the `'gen_ai.tool.manual'` span is not emitted for
any reason (e.g., a flushing or context issue), the predicate will never be satisfied.
This causes the test to hang until the outer Playwright timeout is reached, masking the
specific failure (the missing span) with a generic timeout error.
Did we get this right? 👍 / 👎 to inform future reviews.
Extends the
node-evee2e app so a tool call (get_weather) wraps its work in a manualSentry.startSpan, then asserts the resulting user span lands as a direct child of the SDK'sgen_ai.execute_toolspan (same trace,parent_span_id= the tool span'sspan_id).The point is to prove that manual instrumentation added inside a tool slots into the active gen_ai context rather than floating off as its own segment — i.e. eve runs
executewhile the tool span is active. We fold the check into the existing weather turn instead of adding a new agent turn to avoid a second LLM call.Stacked on #24254.
🤖 Generated with Claude Code