fix parent span/trace loss for ignored child spans in otel

Author: Lms24

dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/children/instrument.mjs

@@ -7,6 +7,7 @@ Sentry.init({
   tracesSampleRate: 1.0,
   transport: loggingTransport,
   traceLifecycle: 'stream',
-  ignoreSpans: ['middleware - expressInit', 'custom-to-drop', { op: 'ignored-op' }],
+  ignoreSpans: ['middleware - expressInit', /custom-to-drop/, { op: 'ignored-op' }],
   clientReportFlushInterval: 1_000,
+  debug: true,
 });

dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/children/server.mjs

@@ -19,16 +19,24 @@ app.get('/test/express', (_req, res) => {
           name: 'custom',
           op: 'custom',
         },
-        () => {},
+        () => {
+          Sentry.startSpan({ name: 'custom-grandchild', op: 'custom' }, () => {
+            Sentry.startSpan({ name: 'custom-to-drop-grand-grandchild', op: 'custom' }, () => {
+              Sentry.startSpan({ name: 'custom-grand-grand-grandchild', op: 'custom' }, () => {});
+            });
+          });
+          Sentry.startSpan({ name: 'custom-grandchild-2', op: 'custom' }, () => {});
+        },
       );
     },
-    Sentry.startSpan({ name: 'name-passes-but-op-not-span-1', op: 'ignored-op' }, () => {}),
-    Sentry.startSpan(
-      // sentry.op attribute has precedence over top op argument
-      { name: 'name-passes-but-op-not-span-2', op: 'keep', attributes: { 'sentry.op': 'ignored-op' } },
-      () => {},
-    ),
   );
+
+  Sentry.startSpan({ name: 'name-passes-but-op-not-span-1', op: 'ignored-op' }, () => {}),
+  Sentry.startSpan(
+    // sentry.op attribute has precedence over top op argument
+    { name: 'name-passes-but-op-not-span-2', /*op: 'keep',*/ attributes: { 'sentry.op': 'ignored-op' } },
+    () => {},
+  ),
   res.send({ response: 'response 1' });
 });
 

dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/children/test.ts

@@ -16,7 +16,7 @@ describe('filtering child spans with ignoreSpans (streaming)', () => {
             discarded_events: [
               {
                 category: 'span',
-                quantity: 4,
+                quantity: 5,
                 reason: 'ignored',
               },
             ],
@@ -26,7 +26,7 @@ describe('filtering child spans with ignoreSpans (streaming)', () => {
           span: container => {
             // 5 spans: 1 root, 2 middleware, 1 request handler, 1 custom
             // Would be 7 if we didn't ignore the 'middleware - expressInit' and 'custom-to-drop' spans
-            expect(container.items).toHaveLength(5);
+            expect(container.items).toHaveLength(8);
             const getSpan = (name: string, op: string) =>
               container.items.find(
                 item => item.name === name && item.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_OP]?.value === op,
@@ -36,13 +36,22 @@ describe('filtering child spans with ignoreSpans (streaming)', () => {
             const requestHandlerSpan = getSpan('/test/express', 'request_handler.express');
             const httpServerSpan = getSpan('GET /test/express', 'http.server');
             const customSpan = getSpan('custom', 'custom');
+            const customGrandchildSpan = getSpan('custom-grandchild', 'custom');
+            const customGrandchild2Span = getSpan('custom-grandchild-2', 'custom');
+            const customGrandGrandGrandChildSpan = getSpan('custom-grand-grand-grandchild', 'custom');
 
             expect(queryMiddlewareSpan).toBeDefined();
             expect(corsMiddlewareSpan).toBeDefined();
             expect(requestHandlerSpan).toBeDefined();
             expect(httpServerSpan).toBeDefined();
             expect(customSpan).toBeDefined();
+            expect(customGrandchildSpan).toBeDefined();
+            expect(customGrandchild2Span).toBeDefined();
+            expect(customGrandGrandGrandChildSpan).toBeDefined();
 
+            expect(customGrandchildSpan?.parent_span_id).toBe(customSpan?.span_id);
+            expect(customGrandchild2Span?.parent_span_id).toBe(customSpan?.span_id);
+            expect(customGrandGrandGrandChildSpan?.parent_span_id).toBe(customGrandchildSpan?.span_id);
             expect(customSpan?.parent_span_id).toBe(requestHandlerSpan?.span_id);
             expect(requestHandlerSpan?.parent_span_id).toBe(httpServerSpan?.span_id);
             expect(queryMiddlewareSpan?.parent_span_id).toBe(httpServerSpan?.span_id);

packages/opentelemetry/src/contextManager.ts

@@ -59,12 +59,19 @@ export function wrapContextManagerClass<ContextManagerInstance extends ContextMa
       thisArg?: ThisParameterType<F>,
       ...args: A
     ): ReturnType<F> {
-      // Remove ignored spans from context so children naturally parent to the grandparent
+      // Remove ignored spans from context and restore the parent span so children
+      // naturally parent to the grandparent instead of starting a new trace.
+      // At this point, this.active() still holds the outer context (before super.with()
+      // updates AsyncLocalStorage), which has the grandparent span we want to restore.
       const span = trace.getSpan(context);
-      const effectiveContext =
-        span?.spanContext().traceState?.get(SENTRY_TRACE_STATE_CHILD_IGNORED) === '1'
-          ? trace.deleteSpan(context)
-          : context;
+      let effectiveContext: Context;
+      if (span?.spanContext().traceState?.get(SENTRY_TRACE_STATE_CHILD_IGNORED) === '1') {
+        const contextWithoutSpan = trace.deleteSpan(context);
+        const parentSpan = trace.getSpan(this.active());
+        effectiveContext = parentSpan ? trace.setSpan(contextWithoutSpan, parentSpan) : contextWithoutSpan;
+      } else {
+        effectiveContext = context;
+      }
 
       const currentScopes = getScopesFromContext(effectiveContext);
       const currentScope = currentScopes?.scope || getCurrentScope();