Conversation
Contributor
size-limit report 📦
|
JPeer264
force-pushed
the
jp/cf-int-tests-bindings-streaming
branch
from
September 19, 2026 15:46
bfc18d9 to
4b27b89
Compare
JPeer264
marked this pull request as ready for review
September 21, 2026 06:45
JPeer264
requested review from
isaacs,
msonnb and
mydea
and removed request for
a team
September 21, 2026 06:45
Contributor
JPeer264
force-pushed
the
jp/cf-int-tests-bindings-streaming
branch
from
September 27, 2026 09:54
4b27b89 to
31da3d8
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
9
to
14
| export default Sentry.withSentry( | ||
| (env: Env) => ({ | ||
| dsn: env.SENTRY_DSN, | ||
| traceLifecycle: 'static', | ||
| tracesSampleRate: 1, | ||
| }), | ||
| { |
Contributor
There was a problem hiding this comment.
Bug: The D1 batch test can fail due to a race condition where parent and child spans arrive in separate envelopes, but the test asserts they are in the same one.
Severity: MEDIUM
Suggested Fix
The test should be updated to handle spans arriving in separate envelopes. Use collectStreamedSpans() to wait for all spans before making assertions, which is the pattern used in the Prisma and Vercel AI tests. Alternatively, restructure the assertions so they do not depend on finding parent and child spans within the same envelope.
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/cloudflare-integration-tests/suites/d1/index.ts#L9-L14
Potential issue: The D1 batch integration test is susceptible to a race condition. The
test expects a parent segment span and its child 'D1 batch' span to be present in the
same envelope when using `.unordered()` mode. However, due to the test runner's
streaming nature and timed buffer flushes, these spans can arrive in separate envelopes.
When this occurs, the logic attempting to find the child span fails because it's not in
the same envelope as the parent. Consequently, a later assertion,
`expect(directBatchSpan).toBeDefined()`, fails, leading to flaky test outcomes in CI
environments.
Also affects:
dev-packages/cloudflare-integration-tests/suites/d1/test.ts:85~110
Did we get this right? 👍 / 👎 to inform future reviews.
Removes the `traceLifecycle: 'static'` pin from `suites/d1`, `suites/r2`,
`suites/queue`, `suites/prisma`, `suites/durableobject/error`,
`suites/workflows/step-context` and `suites/vite/diagnostics-channel/vercelai-6`,
and rewrites the assertions from transaction envelopes to span v2.
`durableobject/error` and `workflows/step-context` only assert on error events,
so the pin is all that goes. `cache-client` and `durableobject-scope` were
already unpinned; they drop `transaction` from their ignore list, which is now an
envelope type nothing emits.
The binding spans keep the shape they had, and only the encoding changes: a span
carries its name in `name` rather than `description`, its op and origin as
attributes, and every attribute as a `{ type, value }` pair. The r2 and queue
suites therefore keep one envelope expectation per request.
Two things do change.
D1 and Prisma spans are `db.query`, so streaming names them after
`db.query.summary`. `SELECT * FROM users WHERE id = ?` becomes `SELECT users`.
The Prisma suite loses the description-based split between its two `SELECT`
spans, which now share the name `SELECT main.User`, so the D1 one is picked by its
op and the traceparent comment is asserted on `db.query.text` instead.
`prisma` and `vercelai-6` collect until the whole trace is in hand, seventeen spans
and three. Both assert on the complete child set of one request, and the span
buffer flushes on a timer, so reading a single envelope would be a race. Waiting
for the segment span would be one too: it ends last, but each envelope is its own
request to the mock server, so it can arrive before its children do.
`vercelai-6` also drops the separate span container it used to read next to the
transaction item: a streamed gen_ai span is an ordinary item of the one span
envelope.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
JPeer264
force-pushed
the
jp/cf-int-tests-bindings-streaming
branch
from
September 27, 2026 13:28
31da3d8 to
6c87954
Compare
Comment on lines
+99
to
+100
| 'db.system.name': { type: 'string', value: 'cloudflare-d1' }, | ||
| 'db.operation.name': { type: 'string', value: 'exec' }, |
Contributor
There was a problem hiding this comment.
Bug: D1 integration tests will fail because they assert parent-child span relationships in a single envelope, which is not guaranteed with streaming span emission.
Severity: MEDIUM
Suggested Fix
Update the D1 integration tests to handle spans arriving across multiple, separate envelopes. Use a utility like collectStreamedSpans() to accumulate all spans from the test run before performing assertions that rely on relationships between different spans.
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/cloudflare-integration-tests/suites/d1/test.ts#L99-L100
Potential issue: The removal of `traceLifecycle: 'static'` enables streaming span
emission for D1 integration tests. However, the tests for `prepare().all()`, `exec()`,
and `batch()` assert parent-child span relationships within a single-envelope callback,
for example `expect(querySpan?.parent_span_id).toBe(segmentSpan?.span_id)`. With
streaming, the parent `segmentSpan` and child `querySpan` can arrive in separate
envelopes. When this happens, one of the spans will be `undefined` during the check,
causing the test assertion to fail and making the test suite flaky.
Also affects:
dev-packages/cloudflare-integration-tests/suites/d1/test.ts:129~130
This branch has not been deployed
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 the binding suites off the
traceLifecycle: 'static'pin:d1,r2,queue,prisma,durableobject/error,workflows/step-contextandvite/diagnostics-channel/vercelai-6.durableobject/errorandworkflows/step-contextonly assert on error events, so the pin is all that goes.cache-clientanddurableobject-scopewere already unpinned and keep ignoring spans, so they are untouched.Most binding spans keep the shape they had and only the encoding changes, so the r2 and queue suites keep one envelope expectation per request. Two things do change.
D1 and Prisma spans carry the
db.queryop, so streaming names them afterdb.query.summary:SELECT * FROM users WHERE id = ?becomesSELECT users. The Prisma suite loses the description-based split between its twoSELECTspans, which now share the nameSELECT main.User. The D1 one is picked by its op instead, and the traceparent comment that used to be matched on the description is asserted ondb.query.text.prismaandvercelai-6switch tocollectStreamedSpansUntilSegment. Both assert on the complete child set of one request and the span buffer flushes on a timer, so reading a single envelope would be a race.vercelai-6also drops the separate span container it used to read next to the transaction item: a streamed gen_ai span is an ordinary item of the one span envelope./keeps itsGET /name under streaming (the source isroute, noturl), sovercelai-6waits on that rather than the bare method the raw-URL suites see.Part of #24148
🤖 Generated with Claude Code