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..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(); @@ -72,3 +73,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/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..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 @@ -66,6 +66,18 @@ expressApp.get('/test', async function (req, res) { res.send({ version: 'v1' }); }); +expressApp.get('/test-no-parent', async function (req, res) { + 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' }); +}); + 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..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,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, 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,26 @@ 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'); + }); + + const response = await fetch(`${baseURL}/test-no-parent`); + expect(response.ok).toBe(true); + await markerPromise; + + 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, + }), + ]); +}); 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 }, ); }