Uh oh!
There was an error while loading. Please reload this page.
test(e2e): Port cloudflare-agent to span streaming - #23862
Merged
JPeer264 merged 1 commit intoSep 2, 2026
Merged
Conversation
Contributor
size-limit report 📦
|
JPeer264force-pushed
the
jp/e2e-stream-cloudflare-agent
branch
from
September 1, 2026 15:21
b150a0c to
110feb6CompareJPeer264force-pushed
the
jp/e2e-stream-cloudflare-agent
branch
from
September 2, 2026 06:21
110feb6 to
1af4495CompareJPeer264force-pushed
the
jp/e2e-stream-cloudflare-agent
branch
from
September 2, 2026 07:10
1af4495 to
1f6e74bCompareJPeer264force-pushed
the
jp/e2e-stream-cloudflare-agent
branch
from
September 2, 2026 07:46
1f6e74b to
c237b43CompareJPeer264
marked this pull request as ready for review
September 2, 2026 07:47
| transactionEvent.transaction === 'webSocketMessage' && | ||
| (transactionEvent.spans ?? []).some(span => span.description === 'durable_object_storage_put') | ||
| !!rpcSpan && | ||
| spans.some(span => span.is_segment && span.name === 'webSocketMessage' && span.trace_id === rpcSpan.trace_id) |
Member
There was a problem hiding this comment.
The trace id condition is no longer needed if you rebase develop.
| transactionEvent.transaction === 'webSocketMessage' && | ||
| (transactionEvent.spans ?? []).some(span => span.op === 'rpc' && span.description === 'greet') | ||
| !!rpcSpan && | ||
| spans.some(span => span.is_segment && span.name === 'webSocketMessage' && span.trace_id === rpcSpan.trace_id) |
| const rpcSpan = spans.find(span => getSpanOp(span) === 'rpc' && span.name === 'greet')!; | ||
| const rpcSpans = (transaction.spans ?? []).filter(span => span.op === 'rpc'); | ||
| const rpcSpans = spans.filter(span => getSpanOp(span) === 'rpc' && span.trace_id === rpcSpan.trace_id); |
| const genAiSpan = spans.find(span => getSpanOp(span) === 'gen_ai.chat'); | ||
| return ( | ||
| !!genAiSpan && | ||
| spans.some(span => span.is_segment && span.name === 'webSocketMessage' && span.trace_id === genAiSpan.trace_id) |
| transactionEvent.transaction === 'webSocketMessage' && | ||
| (transactionEvent.spans ?? []).some(span => span.op === 'rpc' && span.description === 'greet') | ||
| !!rpcSpan && | ||
| spans.some(span => span.is_segment && span.name === 'webSocketMessage' && span.trace_id === rpcSpan.trace_id) |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JPeer264force-pushed
the
jp/e2e-stream-cloudflare-agent
branch
from
September 2, 2026 11:30
c237b43 to
df60649Compare
Comment on lines
+127
to
+135
| const spansPromise = collectStreamedSpans('cloudflare-agent', spans => | ||
| spans.some( | ||
| span => | ||
| getSpanOp(span) === 'http.server' && | ||
| span.is_segment && | ||
| span.attributes['url.path']?.value === '/agents/my-agent/user-123' && | ||
| span.parent_span_id !== undefined, | ||
| ), | ||
| ); |
Contributor
There was a problem hiding this comment.
Bug: The test for internal db.query spans is ineffective because it monitors a trace that is unlikely to contain these spans, causing the test to always pass.
Severity: LOW
Suggested Fix
The test should be redesigned to capture spans from multiple, potentially unrelated traces simultaneously. Instead of resolving collectStreamedSpans on a single trace's predicate, consider a mechanism that can observe all spans received by the event proxy over a period of time, regardless of their trace_id, and then perform the assertion.
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/cloudflare-agent/tests/callable.test.ts#L127-L135
Potential issue: The test at `callable.test.ts` is intended to verify that internal
`db.query` spans from the agents runtime are not emitted. It uses `collectStreamedSpans`
to listen for spans on the `http.server` trace. However, the internal database queries
for bookkeeping tables like `cf_agents_*` likely execute in a separate trace context
from the main request. Because `collectStreamedSpans` groups spans by `trace_id`, it
will only capture spans from the `http.server` trace and will never see the internal
`db.query` spans from the other trace. This causes the test to pass trivially with zero
results, rendering it ineffective at detecting regressions where these internal spans
might start leaking.
Did we get this right? 👍 / 👎 to inform future reviews.
s1gr1d
approved these changes
Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports
cloudflare-agentto span streaming. The gen_ai assertions were already on streamed spans; the remainingwaitForTransactioncalls are replaced.GETsegment for the same URL. The DO segment is picked byurl.pathplus a definedparent_span_id(the worker propagates its trace over the RPC binding), same discriminator the old spec used.greetrpc span, so they are selected byparent_span_idand ordered bystart_timestamp; the count-based filter proof is unchanged.collectStreamedSpansgrouping by trace (test(e2e): Group collectStreamedSpans by trace #23912): the collect finishes once the owning segment has arrived, so no manualtrace_idmatching is needed.