From 5f6dcbf93550d5dcd50aca629aa631fd34390030 Mon Sep 17 00:00:00 2001 From: lazerg Date: Thu, 29 Oct 2026 11:56:23 +0500 Subject: [PATCH 1/4] fix(server-utils): Require a parent span for Firestore operations Co-Authored-By: Claude Opus 5.5 --- .../suites/orchestrion-firebase/test.ts | 46 +++++++++++++++++++ .../integrations/firebase/instrumentation.ts | 21 +++++---- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/dev-packages/deno-integration-tests/suites/orchestrion-firebase/test.ts b/dev-packages/deno-integration-tests/suites/orchestrion-firebase/test.ts index cb131997dca9..ce98e177dbd4 100644 --- a/dev-packages/deno-integration-tests/suites/orchestrion-firebase/test.ts +++ b/dev-packages/deno-integration-tests/suites/orchestrion-firebase/test.ts @@ -72,3 +72,49 @@ Deno.test('firebase instrumentation: orchestrion @firebase/firestore:add-doc cha assertEquals(fsSpan.attributes['firebase.firestore.options.projectId']?.value, 'demo-project'); assertEquals(fsSpan.attributes['sentry.origin']?.value, 'auto.db.firebase.firestore'); }); + +Deno.test('firebase instrumentation: only creates firestore db spans with an active parent span', async () => { + resetGlobals(); + const sink = spanSink(); + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + transport: sink.transport, + }); + + const channel = tracingChannel('orchestrion:@firebase/firestore:add-doc'); + + function addDoc(path: string): void { + const reference = { + path, + type: 'collection', + firestore: { + app: { name: '[DEFAULT]', options: { projectId: 'demo-project', appId: 'demo-app' } }, + toJSON: () => ({ settings: {} }), + }, + }; + const ctx: Record = { arguments: [reference] }; + channel.start.runStores(ctx, () => undefined); + channel.end.publish(ctx); + ctx.result = {}; + channel.asyncStart.runStores(ctx, () => undefined); + channel.asyncEnd.publish(ctx); + } + + addDoc('before_parent'); + startSpan({ name: 'parent', op: 'test' }, () => addDoc('inside_parent')); + addDoc('after_parent'); + + await flush(); + + const parent = await withTimeout( + sink.waitFor(span => span.is_segment && span.name === 'parent'), + 5000, + "'parent' segment span", + ); + const dbSpans = sink.spans.filter(span => getSpanOp(span) === 'db.query'); + assertEquals( + dbSpans.map(span => ({ name: span.name, parentSpanId: span.parent_span_id, isSegment: span.is_segment })), + [{ name: 'addDoc inside_parent', parentSpanId: parent.span_id, isSegment: false }], + ); +}); diff --git a/packages/server-utils/src/integrations/firebase/instrumentation.ts b/packages/server-utils/src/integrations/firebase/instrumentation.ts index b10534aea30a..246ebd9895d8 100644 --- a/packages/server-utils/src/integrations/firebase/instrumentation.ts +++ b/packages/server-utils/src/integrations/firebase/instrumentation.ts @@ -37,15 +37,18 @@ const NOOP = (): void => {}; export function instrumentFirebase() { for (const { channel, spanName, useParent } of FIRESTORE_OPERATIONS) { - bindTracingChannelToSpan(diagnosticsChannel.tracingChannel(channel), data => - safeChannelCallback(() => { - const reference = data.arguments[0] as FirestoreReference | undefined; - if (!reference) { - return undefined; - } - const spanReference = useParent ? reference.parent || reference : reference; - return startFirestoreSpan(spanName, spanReference); - }), + bindTracingChannelToSpan( + diagnosticsChannel.tracingChannel(channel), + data => + safeChannelCallback(() => { + const reference = data.arguments[0] as FirestoreReference | undefined; + if (!reference) { + return undefined; + } + const spanReference = useParent ? reference.parent || reference : reference; + return startFirestoreSpan(spanName, spanReference); + }), + { requiresParentSpan: true }, ); } From 1609ff1ce2946e38f4ead3ae5957a6c7dbb7130c Mon Sep 17 00:00:00 2001 From: lazerg Date: Thu, 29 Oct 2026 12:32:44 +0500 Subject: [PATCH 2/4] test(deno): Use DB_QUERY constant in Firestore parent span test Co-Authored-By: Claude Opus 5.5 --- .../deno-integration-tests/suites/orchestrion-firebase/test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-packages/deno-integration-tests/suites/orchestrion-firebase/test.ts b/dev-packages/deno-integration-tests/suites/orchestrion-firebase/test.ts index ce98e177dbd4..abc063529480 100644 --- a/dev-packages/deno-integration-tests/suites/orchestrion-firebase/test.ts +++ b/dev-packages/deno-integration-tests/suites/orchestrion-firebase/test.ts @@ -7,6 +7,7 @@ import { assert } from 'https://deno.land/std@0.212.0/assert/assert.ts'; import { assertExists } from 'https://deno.land/std@0.212.0/assert/assert_exists.ts'; import { assertEquals } from 'https://deno.land/std@0.212.0/assert/assert_equals.ts'; import { getSpanOp, resetGlobals, spanSink, withTimeout } from '../../src/index.ts'; +import { DB_QUERY } from '@sentry/conventions/op'; Deno.test('firebase instrumentation: included in default integrations (Deno 2.8.0+)', () => { resetGlobals(); @@ -112,7 +113,7 @@ Deno.test('firebase instrumentation: only creates firestore db spans with an act 5000, "'parent' segment span", ); - const dbSpans = sink.spans.filter(span => getSpanOp(span) === 'db.query'); + const dbSpans = sink.spans.filter(span => getSpanOp(span) === DB_QUERY); assertEquals( dbSpans.map(span => ({ name: span.name, parentSpanId: span.parent_span_id, isSegment: span.is_segment })), [{ name: 'addDoc inside_parent', parentSpanId: parent.span_id, isSegment: false }], From c8543446518d9ceb65b8547b51499536671c6e68 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 29 Oct 2026 10:13:33 +0100 Subject: [PATCH 3/4] test(node): Assert no Firestore spans without a parent span Add a Node E2E check next to the Deno test: a Firestore call outside any active span must not produce a root db.query span. Co-Authored-By: Claude Opus 5.5 --- .../node-firebase/firestore-app/src/app.ts | 7 +++++++ .../node-firebase/tests/spans.test.ts | 17 ++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-firebase/firestore-app/src/app.ts b/dev-packages/e2e-tests/test-applications/node-firebase/firestore-app/src/app.ts index 486aa06b5ffc..254dd7ce7749 100644 --- a/dev-packages/e2e-tests/test-applications/node-firebase/firestore-app/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-firebase/firestore-app/src/app.ts @@ -66,6 +66,13 @@ expressApp.get('/test', async function (req, res) { res.send({ version: 'v1' }); }); +expressApp.get('/test-no-parent', async function (req, res) { + await addCity(); + Sentry.startSpan({ name: 'No Parent Marker' }, () => {}); + await Sentry.flush(); + res.send({ version: 'v1' }); +}); + expressApp.listen(port, () => { console.log(`Example app listening on port ${port}`); }); diff --git a/dev-packages/e2e-tests/test-applications/node-firebase/tests/spans.test.ts b/dev-packages/e2e-tests/test-applications/node-firebase/tests/spans.test.ts index cd9717ccdea0..b51b7455f1fb 100644 --- a/dev-packages/e2e-tests/test-applications/node-firebase/tests/spans.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-firebase/tests/spans.test.ts @@ -1,5 +1,6 @@ import { expect, test } from '@playwright/test'; -import { collectStreamedSpansUntilSegment } from '@sentry-internal/test-utils'; +import type { SerializedStreamedSpan } from '@sentry-internal/test-utils'; +import { collectStreamedSpansUntilSegment, getSpanOp, waitForStreamedSpans } from '@sentry-internal/test-utils'; // The orchestrion spans are Sentry-native, so they carry no span-kind attribute (`sentry.kind`). const origin = 'auto.db.firebase.firestore'; @@ -49,3 +50,17 @@ test('should add, set, get and delete document', async ({ baseURL }) => { expect(children).toHaveLength(4); expect(children).toEqual(expect.arrayContaining([spanAddDoc, spanSetDocs, spanGetDocs, spanDeleteDoc])); }); + +test('does not create firestore spans without a parent span', async ({ baseURL }) => { + const seen: SerializedStreamedSpan[] = []; + // A span that is never sent cannot be awaited, so wait for a marker span sent after the Firestore call. + const markerPromise = waitForStreamedSpans('node-firebase', spans => { + seen.push(...spans); + return spans.some(span => span.is_segment && span.name === 'No Parent Marker'); + }); + + await fetch(`${baseURL}/test-no-parent`); + await markerPromise; + + expect(seen.filter(span => span.is_segment && getSpanOp(span) === 'db.query')).toEqual([]); +}); From 8ae0eb52df7d7b2913bacd7a0dbdcf4c39e8840b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 29 Oct 2026 10:23:03 +0100 Subject: [PATCH 4/4] test(node): Make Firestore no-parent span test order-safe Flush before the marker span so an unparented Firestore span would reach the proxy first, and assert on all spans of a dedicated collection instead of only root spans. A parented call inside the marker proves the instrumentation is active. Co-Authored-By: Claude Opus 5.5 --- .../node-firebase/firestore-app/src/app.ts | 9 +++++++-- .../node-firebase/tests/spans.test.ts | 15 ++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-firebase/firestore-app/src/app.ts b/dev-packages/e2e-tests/test-applications/node-firebase/firestore-app/src/app.ts index 254dd7ce7749..83d04bebd136 100644 --- a/dev-packages/e2e-tests/test-applications/node-firebase/firestore-app/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-firebase/firestore-app/src/app.ts @@ -67,8 +67,13 @@ expressApp.get('/test', async function (req, res) { }); expressApp.get('/test-no-parent', async function (req, res) { - await addCity(); - Sentry.startSpan({ name: 'No Parent Marker' }, () => {}); + const noParentRef = collection(db, 'no-parent'); + await addDoc(noParentRef, { name: 'Without Parent' }); + // Flush before the marker so a span from the call above would reach the proxy before it. + await Sentry.flush(); + await Sentry.startSpan({ name: 'No Parent Marker' }, async () => { + await addDoc(noParentRef, { name: 'With Parent' }); + }); await Sentry.flush(); res.send({ version: 'v1' }); }); diff --git a/dev-packages/e2e-tests/test-applications/node-firebase/tests/spans.test.ts b/dev-packages/e2e-tests/test-applications/node-firebase/tests/spans.test.ts index b51b7455f1fb..836238fc3add 100644 --- a/dev-packages/e2e-tests/test-applications/node-firebase/tests/spans.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-firebase/tests/spans.test.ts @@ -1,6 +1,6 @@ import { expect, test } from '@playwright/test'; import type { SerializedStreamedSpan } from '@sentry-internal/test-utils'; -import { collectStreamedSpansUntilSegment, getSpanOp, waitForStreamedSpans } from '@sentry-internal/test-utils'; +import { collectStreamedSpansUntilSegment, waitForStreamedSpans } from '@sentry-internal/test-utils'; // The orchestrion spans are Sentry-native, so they carry no span-kind attribute (`sentry.kind`). const origin = 'auto.db.firebase.firestore'; @@ -59,8 +59,17 @@ test('does not create firestore spans without a parent span', async ({ baseURL } return spans.some(span => span.is_segment && span.name === 'No Parent Marker'); }); - await fetch(`${baseURL}/test-no-parent`); + const response = await fetch(`${baseURL}/test-no-parent`); + expect(response.ok).toBe(true); await markerPromise; - expect(seen.filter(span => span.is_segment && getSpanOp(span) === 'db.query')).toEqual([]); + const marker = seen.find(span => span.is_segment && span.name === 'No Parent Marker')!; + // Only the call inside the marker span may produce a span, as its child. + expect(seen.filter(span => span.name === 'addDoc no-parent')).toEqual([ + expect.objectContaining({ + is_segment: false, + parent_span_id: marker.span_id, + trace_id: marker.trace_id, + }), + ]); });