feat(cloudflare): Support tracing for queue producer#20529
feat(cloudflare): Support tracing for queue producer#20529
Conversation
JPeer264
force-pushed
the
jp/cloudflare-queues
branch
from
ef6442b to
b9b0533
Compare
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
|
👋 @mydea, @logaretm, @andreiborza — Please review this PR when you get a chance! |
JPeer264
force-pushed
the
jp/cloudflare-queues
branch
from
d4d646b to
124f4d9
Compare
| 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
3 participants
closes #14387
closes JS-1813
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