Uh oh!
There was an error while loading. Please reload this page.
feat(cloudflare): Support tracing for queue producer - #20529
Conversation
ef6442b to
b9b0533Comparesize-limit report 📦
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b9b0533. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
👋 @mydea, @logaretm, @andreiborza — Please review this PR when you get a chance! |
d4d646b to
124f4d9Compare| expect(constructorEnv).not.toBe(mockEnv); | ||
| }); | ||
| it('passes original env to the constructor when enableRpcTracePropagation is disabled', () => { |
There was a problem hiding this comment.
This test has been removed as before it only checked if the env was not changed to a Proxy. Because we now have another instrumentation we can simply remove it as it will always be a proxy
| /** | ||
| * Duck-type check for Queue producer bindings. | ||
| * Queue has `send` and `sendBatch` async methods. | ||
| */ | ||
| export function isQueue(item: unknown): item is Queue { | ||
| return item != null && isNotJSRPC(item) && typeof item.send === 'function' && typeof item.sendBatch === 'function'; | ||
| } |
There was a problem hiding this comment.
Is there a possibility that other APIs provide the same functions? Meaning: is this check safe enough?
There was a problem hiding this comment.
very unlikely that send and sendBatch are on it at the same time. There is a WebSocket, SendEmail and Pipeline that have send, but not sendBatch.
| if (!rpcPropagation) { | ||
| return item; | ||
| } |
There was a problem hiding this comment.
Can this be moved up to be an early-return? Then we wouldn't need the isQueue check and it's slightly more performant.
There was a problem hiding this comment.
no it can't as the queue will always be instrumented in case there is one, so rpcPropagation is already an early return for what comes next
arthurfiorette
commented
Jul 15, 2026
Hey @JPeer264@logaretm is this working? Latest docs says to manually instrument and i couldn't get automated instrumentation to work. https://docs.sentry.io/platforms/javascript/guides/cloudflare/tracing/instrumentation/queues-module/ |
closes#14387
closes#14387
We already had support for the consumer, but the producer was never instrumented. Which means we only knew when a queue was consumed, but never when it was produced.
Example trace (producer new): https://sentry-sdks.sentry.io/explore/traces/trace/7ee71fd6dc8b4ce1a22b1bb9f9610b26/
Example trace (consumer): https://sentry-sdks.sentry.io/explore/traces/trace/486eb86cd02146279c4d547efb4cabab/
In
wrangler.jsonca producer needs to be configured viaqueues.producers:{ "queues": { "producers": [ { "queue": "test-queue", "binding": "MY_QUEUE", }, ], }, }Once this happens the
MY_QUEUEis available in theenvvariable, which we already instrument. With that we only addisQueueinsideinstrumentEnv.tsand get instrument the producer:Additional info
We are using
messaging.batch.message_count, which is not yet in our semantic conventions: https://getsentry.github.io/sentry-conventions/attributes/messaging/But it was already used before in our queue consumer logic. So the only thing is that it needs to be added first: getsentry/sentry-conventions#338 to make it official