fix it in node-experimental

Author: mydea

packages/node-experimental/src/sdk/client.ts

@@ -5,7 +5,7 @@ import { trace } from '@opentelemetry/api';
 import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
 import { applySdkMetadata } from '@sentry/core';
 import type { CaptureContext, Event, EventHint } from '@sentry/types';
-import { Scope, getIsolationScope } from './scope';
+import { Scope } from './scope';
 
 /** A client for using Sentry with Node & OpenTelemetry. */
 export class NodeExperimentalClient extends NodeClient {
@@ -54,7 +54,7 @@ export class NodeExperimentalClient extends NodeClient {
     event: Event,
     hint: EventHint,
     scope?: Scope,
-    _isolationScope?: Scope,
+    isolationScope?: Scope,
   ): PromiseLike<Event | null> {
     let actualScope = scope;
 
@@ -64,8 +64,6 @@ export class NodeExperimentalClient extends NodeClient {
       delete hint.captureContext;
     }
 
-    const isolationScope = _isolationScope || (scope && scope.isolationScope) || getIsolationScope();
-
     return super._prepareEvent(event, hint, actualScope, isolationScope);
   }
 }

packages/node-experimental/src/sdk/scope.ts

@@ -72,9 +72,6 @@ export function isInitialized(): boolean {
 
 /** A fork of the classic scope with some otel specific stuff. */
 export class Scope extends OpenTelemetryScope implements ScopeInterface {
-  // Overwrite this if you want to use a specific isolation scope here
-  public isolationScope: Scope | undefined;
-
   protected _client: Client | undefined;
 
   protected _lastEventId: string | undefined;

packages/node-experimental/src/sdk/spanProcessor.ts

@@ -1,13 +1,11 @@
-import type { Context } from '@opentelemetry/api';
 import { SpanKind } from '@opentelemetry/api';
 import type { Span } from '@opentelemetry/sdk-trace-base';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
-import { SentrySpanProcessor, getClient, getSpanFinishScope } from '@sentry/opentelemetry';
+import { SentrySpanProcessor, getClient } from '@sentry/opentelemetry';
 
 import type { Http } from '../integrations/http';
 import type { NodeFetch } from '../integrations/node-fetch';
 import type { NodeExperimentalClient } from '../types';
-import { getIsolationScope } from './api';
 import { Scope } from './scope';
 
 /**
@@ -18,18 +16,6 @@ export class NodeExperimentalSentrySpanProcessor extends SentrySpanProcessor {
     super({ scopeClass: Scope });
   }
 
-  /** @inheritDoc */
-  public onStart(span: Span, parentContext: Context): void {
-    super.onStart(span, parentContext);
-
-    // We need to make sure that we use the correct isolation scope when finishing the span
-    // so we store it on the span finish scope for later use
-    const scope = getSpanFinishScope(span) as Scope | undefined;
-    if (scope) {
-      scope.isolationScope = getIsolationScope();
-    }
-  }
-
   /** @inheritDoc */
   protected _shouldSendSpanToSentry(span: Span): boolean {
     const client = getClient<NodeExperimentalClient>();

packages/node-experimental/src/sdk/types.ts

@@ -28,8 +28,6 @@ export interface ScopeData {
 }
 
 export interface Scope extends BaseScope {
-  // @ts-expect-error typeof this is what we want here
-  isolationScope: typeof this | undefined;
   // @ts-expect-error typeof this is what we want here
   clone(scope?: Scope): typeof this;
   /**

packages/node-experimental/test/integration/transactions.test.ts

@@ -99,7 +99,7 @@ describe('Integration | Transactions', () => {
         environment: 'production',
         event_id: expect.any(String),
         platform: 'node',
-        sdkProcessingMetadata: {
+        sdkProcessingMetadata: expect.objectContaining({
           dynamicSamplingContext: expect.objectContaining({
             environment: 'production',
             public_key: expect.any(String),
@@ -112,7 +112,7 @@ describe('Integration | Transactions', () => {
           source: 'task',
           spanMetadata: expect.any(Object),
           requestPath: 'test-path',
-        },
+        }),
         server_name: expect.any(String),
         // spans are circular (they have a reference to the transaction), which leads to jest choking on this
         // instead we compare them in detail below
@@ -329,6 +329,159 @@ describe('Integration | Transactions', () => {
     );
   });
 
+  it('correctly creates concurrent transaction & spans when using native OTEL tracer', async () => {
+    const beforeSendTransaction = jest.fn(() => null);
+
+    mockSdkInit({ enableTracing: true, beforeSendTransaction });
+
+    const client = Sentry.getClient<NodeExperimentalClient>();
+
+    Sentry.addBreadcrumb({ message: 'test breadcrumb 1', timestamp: 123456 });
+
+    Sentry.withIsolationScope(() => {
+      client.tracer.startActiveSpan('test name', span => {
+        Sentry.addBreadcrumb({ message: 'test breadcrumb 2', timestamp: 123456 });
+
+        span.setAttributes({
+          'test.outer': 'test value',
+        });
+
+        const subSpan = Sentry.startInactiveSpan({ name: 'inner span 1' });
+        subSpan.end();
+
+        Sentry.setTag('test.tag', 'test value');
+
+        client.tracer.startActiveSpan('inner span 2', innerSpan => {
+          Sentry.addBreadcrumb({ message: 'test breadcrumb 3', timestamp: 123456 });
+
+          innerSpan.setAttributes({
+            'test.inner': 'test value',
+          });
+
+          innerSpan.end();
+        });
+
+        span.end();
+      });
+    });
+
+    Sentry.withIsolationScope(() => {
+      client.tracer.startActiveSpan('test name b', span => {
+        Sentry.addBreadcrumb({ message: 'test breadcrumb 2b', timestamp: 123456 });
+
+        span.setAttributes({
+          'test.outer': 'test value b',
+        });
+
+        const subSpan = Sentry.startInactiveSpan({ name: 'inner span 1b' });
+        subSpan.end();
+
+        Sentry.setTag('test.tag', 'test value b');
+
+        client.tracer.startActiveSpan('inner span 2b', innerSpan => {
+          Sentry.addBreadcrumb({ message: 'test breadcrumb 3b', timestamp: 123456 });
+
+          innerSpan.setAttributes({
+            'test.inner': 'test value b',
+          });
+
+          innerSpan.end();
+        });
+
+        span.end();
+      });
+    });
+
+    await client.flush();
+
+    expect(beforeSendTransaction).toHaveBeenCalledTimes(2);
+    expect(beforeSendTransaction).toHaveBeenCalledWith(
+      expect.objectContaining({
+        breadcrumbs: [
+          { message: 'test breadcrumb 1', timestamp: 123456 },
+          { message: 'test breadcrumb 2', timestamp: 123456 },
+          { message: 'test breadcrumb 3', timestamp: 123456 },
+        ],
+        contexts: expect.objectContaining({
+          otel: expect.objectContaining({
+            attributes: {
+              'test.outer': 'test value',
+            },
+          }),
+          trace: {
+            data: {
+              'otel.kind': 'INTERNAL',
+              'sentry.origin': 'manual',
+            },
+            span_id: expect.any(String),
+            status: 'ok',
+            trace_id: expect.any(String),
+            origin: 'manual',
+          },
+        }),
+        spans: [
+          expect.objectContaining({
+            description: 'inner span 1',
+          }),
+          expect.objectContaining({
+            description: 'inner span 2',
+          }),
+        ],
+        start_timestamp: expect.any(Number),
+        tags: { 'test.tag': 'test value' },
+        timestamp: expect.any(Number),
+        transaction: 'test name',
+        type: 'transaction',
+      }),
+      {
+        event_id: expect.any(String),
+      },
+    );
+
+    expect(beforeSendTransaction).toHaveBeenCalledWith(
+      expect.objectContaining({
+        breadcrumbs: [
+          { message: 'test breadcrumb 1', timestamp: 123456 },
+          { message: 'test breadcrumb 2b', timestamp: 123456 },
+          { message: 'test breadcrumb 3b', timestamp: 123456 },
+        ],
+        contexts: expect.objectContaining({
+          otel: expect.objectContaining({
+            attributes: {
+              'test.outer': 'test value b',
+            },
+          }),
+          trace: {
+            data: {
+              'otel.kind': 'INTERNAL',
+              'sentry.origin': 'manual',
+            },
+            span_id: expect.any(String),
+            status: 'ok',
+            trace_id: expect.any(String),
+            origin: 'manual',
+          },
+        }),
+        spans: [
+          expect.objectContaining({
+            description: 'inner span 1b',
+          }),
+          expect.objectContaining({
+            description: 'inner span 2b',
+          }),
+        ],
+        start_timestamp: expect.any(Number),
+        tags: { 'test.tag': 'test value b' },
+        timestamp: expect.any(Number),
+        transaction: 'test name b',
+        type: 'transaction',
+      }),
+      {
+        event_id: expect.any(String),
+      },
+    );
+  });
+
   it('correctly creates transaction & spans with a trace header data', async () => {
     const beforeSendTransaction = jest.fn(() => null);
 

packages/opentelemetry/src/custom/hub.ts

@@ -13,7 +13,6 @@ export class OpenTelemetryHub extends Hub {
   public constructor(client?: Client, scope: Scope = new OpenTelemetryScope(), isolationScope?: Scope) {
     super(client, scope, isolationScope);
   }
-
 }
 
 /**

packages/opentelemetry/test/integration/transactions.test.ts

@@ -1,6 +1,6 @@
 import { TraceFlags, context, trace } from '@opentelemetry/api';
 import type { SpanProcessor } from '@opentelemetry/sdk-trace-base';
-import { addBreadcrumb, getClient, getIsolationScope, setTag, withIsolationScope } from '@sentry/core';
+import { addBreadcrumb, getClient, setTag, withIsolationScope } from '@sentry/core';
 import type { PropagationContext, TransactionEvent } from '@sentry/types';
 import { logger } from '@sentry/utils';