-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): Add Supabase Queues support #15921
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
size-limit report 📦
|
bbadd60 to
e7b3370
Compare
1bc2897 to
d387514
Compare
e7b3370 to
56a2d84
Compare
56a2d84 to
13d60e0
Compare
423397d to
556703c
Compare
13d60e0 to
74869e4
Compare
e63915a to
719c8b6
Compare
| }, | ||
| }, | ||
| async span => { | ||
| return (Reflect.apply(target, thisArg, argumentsList) as Promise<unknown>).then((res: unknown) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably also end the span when it throws/rejects? We can also set the status of the span then.
| name: 'supabase.db.rpc', | ||
| attributes: { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.db.supabase', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can add the messaging.system attribute to be 'supabase' as described in https://develop.sentry.dev/sdk/telemetry/traces/modules/queues/
| const isProducerSpan = argumentsList[0] === 'enqueue'; | ||
| const isConsumerSpan = argumentsList[0] === 'dequeue'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this recently changed, but here they show send and pop as rpc args: https://supabase.com/docs/guides/queues/quickstart#enqueueing-and-dequeueing-messages 🤔
fb8eeb1 to
a8da234
Compare
ff2f804 to
5ce5ee9
Compare
|
@sentry review |
|
On it! We are reviewing the PR and will provide feedback shortly. |
PR DescriptionThis pull request introduces instrumentation for Supabase queue operations using pgmq, enabling Sentry to capture spans and breadcrumbs for queue publishing and processing. This provides visibility into asynchronous task execution within Supabase applications. Click to see moreKey Technical ChangesThe key technical changes include: 1) Instrumenting the Architecture DecisionsThe primary architectural decision involves using Proxy objects to intercept calls to the Supabase client's Dependencies and InteractionsThis integration depends on the Risk ConsiderationsPotential risks include: 1) Performance overhead due to the instrumentation, although proxies are generally performant. 2) Incorrectly identifying queue operations, leading to spurious spans. 3) Failure to propagate trace context if consumers are not properly instrumented. 4) Security implications of modifying message bodies, although the injected data is limited to Sentry trace context. 5) The modification of the arguments list in place could lead to unexpected side effects. Notable Implementation DetailsNotable implementation details include: 1) The use of |
| if (traceparentData?.traceId && traceparentData?.parentSpanId) { | ||
| // Convert parentSampled boolean to traceFlags (W3C trace context spec) | ||
| // traceFlags bit 0 (LSB) = sampled flag: 1 if sampled, 0 if not sampled | ||
| const traceFlags = traceparentData.parentSampled ? 1 : 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Undefined parentSampled incorrectly converted to "not sampled"
When extracting producer trace context for span links, the code converts parentSampled to traceFlags using traceparentData.parentSampled ? 1 : 0. If parentSampled is undefined (meaning sampling decision was deferred/not yet made), this expression incorrectly sets traceFlags to 0 (not sampled). Per W3C trace context spec, undefined sampling state is semantically different from "not sampled". This could affect trace propagation for deferred sampling scenarios, though the impact is limited to span link attributes.
| scope.addEventProcessor(e => { | ||
| addExceptionMechanism(e, { | ||
| handled: false, | ||
| type: 'auto.db.supabase.rpc', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Mechanism types don't align with span origins per rules (Bugbot Rules)
Per the user's review rules, captureException mechanism types must align with the wrapping span's SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN. Multiple mismatches exist: generic RPC uses mechanism 'auto.db.supabase.rpc' but span origin 'auto.db.supabase'; queue operations use mechanism 'auto.db.supabase.queue' but origins are 'auto.db.supabase.queue.producer' and 'auto.db.supabase.queue.consumer'; auth uses mechanism 'auto.db.supabase.auth' but origin 'auto.db.supabase'; PostgREST uses mechanism 'auto.db.supabase.postgres' but origin 'auto.db.supabase'. Flagged because the rules file specifies mechanism types should match span origins.
Additional Locations (2)
|
|
||
| /** | ||
| * Instruments Supabase auth operations. | ||
| * | ||
| * Creates auto.db.supabase spans for auth operations (signIn, signUp, etc.) | ||
| * to track authentication performance and errors. | ||
| */ | ||
| function _instrumentAuthOperation(operation: AuthOperationFn, isAdmin = false): AuthOperationFn { | ||
| return new Proxy(operation, { | ||
| apply(target, thisArg, argumentsList) { | ||
| return startSpan( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
c3e2644 to
608b31d
Compare
|
@AbhiPrasad, @Lms24, can I have another round of reviews on this, please? I updated the sample events in the PR description. |
Resolves: #14611
This PR adds Supabase Queues support to the Supabase
Sample Events: Link
Instrumented Operations
Producer operations (
queue.publish):send- Send a single messagesend_batch- Send multiple messagesConsumer operations (
queue.process):pop- Read and delete messageread- Read message (with visibility timeout)receive- Alias forreadSpan Attributes
Follows OTEL Messaging Semantic Conventions and Sentry Queue Developer Docs:
messaging.destination.name- Queue namemessaging.message.id- Message ID (frommsg_id)messaging.message.retry.count- Retry count (from PGMQ'sread_ct)messaging.batch.message_count- Batch size forsend_batchDistributed Tracing
Producer spans inject
sentry-traceandbaggageinto the message payload under_sentrykey. Consumer spans create span links to the producer trace context.Notes
client.rpc()andclient.schema('pgmq_public').rpc()patternssend,send_batch,pop,read,receivemessaging.batch.message_count: 0