fix(otel): Make sure we use correct hub on finish (#7577)

getsentry · Mar 23, 2023 · 1418582 · 1418582
1 parent 1121507
commit 1418582
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 25 deletions.
diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts
@@ -23,7 +23,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
   /**
    * The reference to the current hub.
    */
-  public readonly _hub: Hub;
+  public _hub: Hub;
 
   private _name: string;
 
@@ -280,4 +280,14 @@ export class Transaction extends SpanClass implements TransactionInterface {
 
     return dsc;
   }
+
+  /**
+   * Override the current hub with a new one.
+   * Used if you want another hub to finish the transaction.
+   *
+   * @internal
+   */
+  public setHub(hub: Hub): void {
+    this._hub = hub;
+  }
 }
diff --git a/packages/opentelemetry-node/src/spanprocessor.ts b/packages/opentelemetry-node/src/spanprocessor.ts
@@ -50,17 +50,6 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
    * @inheritDoc
    */
   public onStart(otelSpan: OtelSpan, parentContext: Context): void {
-    const hub = getCurrentHub();
-    if (!hub) {
-      __DEBUG_BUILD__ && logger.error('SentrySpanProcessor has triggered onStart before a hub has been setup.');
-      return;
-    }
-    const scope = hub.getScope();
-    if (!scope) {
-      __DEBUG_BUILD__ && logger.error('SentrySpanProcessor has triggered onStart before a scope has been setup.');
-      return;
-    }
-
     const otelSpanId = otelSpan.spanContext().spanId;
     const otelParentSpanId = otelSpan.parentSpanId;
 
@@ -79,7 +68,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
       SENTRY_SPAN_PROCESSOR_MAP.set(otelSpanId, sentryChildSpan);
     } else {
       const traceCtx = getTraceData(otelSpan, parentContext);
-      const transaction = hub.startTransaction({
+      const transaction = getCurrentHub().startTransaction({
         name: otelSpan.name,
         ...traceCtx,
         instrumenter: 'otel',
@@ -95,12 +84,6 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
    * @inheritDoc
    */
   public onEnd(otelSpan: OtelSpan): void {
-    const hub = getCurrentHub();
-    if (!hub) {
-      __DEBUG_BUILD__ && logger.error('SentrySpanProcessor has triggered onEnd before a hub has been setup.');
-      return;
-    }
-
     const otelSpanId = otelSpan.spanContext().spanId;
     const sentrySpan = SENTRY_SPAN_PROCESSOR_MAP.get(otelSpanId);
 
@@ -143,7 +126,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
         syntheticError.name = type;
       }
 
-      hub.captureException(syntheticError, {
+      getCurrentHub().captureException(syntheticError, {
         captureContext: {
           contexts: {
             otel: {
@@ -162,6 +145,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
 
     if (sentrySpan instanceof Transaction) {
       updateTransactionWithOtelData(sentrySpan, otelSpan);
+      sentrySpan.setHub(getCurrentHub());
     } else {
       updateSpanWithOtelData(sentrySpan, otelSpan);
     }

diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts
@@ -5,7 +5,15 @@ import type { Span as OtelSpan } from '@opentelemetry/sdk-trace-base';
 import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
 import { SemanticAttributes, SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
 import type { SpanStatusType } from '@sentry/core';
-import { addTracingExtensions, createTransport, Hub, makeMain, Span as SentrySpan, Transaction } from '@sentry/core';
+import {
+  addTracingExtensions,
+  createTransport,
+  Hub,
+  makeMain,
+  Scope,
+  Span as SentrySpan,
+  Transaction,
+} from '@sentry/core';
 import { NodeClient } from '@sentry/node';
 import { resolvedSyncPromise } from '@sentry/utils';
 
@@ -79,13 +87,9 @@ describe('SentrySpanProcessor', () => {
     expect(sentrySpanTransaction?.parentSpanId).toEqual(otelSpan.parentSpanId);
     expect(sentrySpanTransaction?.spanId).toEqual(otelSpan.spanContext().spanId);
 
-    expect(hub.getScope()?.getSpan()).toBeUndefined();
-
     otelSpan.end(endTime);
 
     expect(sentrySpanTransaction?.endTimestamp).toBe(endTimestampMs / 1000);
-
-    expect(hub.getScope()?.getSpan()).toBeUndefined();
   });
 
   it('creates a child span if there is a running transaction', () => {
@@ -789,6 +793,46 @@ describe('SentrySpanProcessor', () => {
       trace_id: otelSpan.spanContext().traceId,
     });
   });
+
+  // Regression test for https://github.com/getsentry/sentry-javascript/issues/7538
+  // Since otel context does not map to when Sentry hubs are cloned
+  // we can't rely on the original hub at transaction creation to contain all
+  // the scope information we want. Let's test to make sure that the information is
+  // grabbed from the new hub.
+  it('handles when a different hub creates the transaction', () => {
+    let sentryTransaction: any;
+
+    client = new NodeClient({
+      ...DEFAULT_NODE_CLIENT_OPTIONS,
+      tracesSampleRate: 1.0,
+    });
+
+    client.on('finishTransaction', transaction => {
+      sentryTransaction = transaction;
+    });
+
+    hub = new Hub(client);
+    makeMain(hub);
+
+    const newHub = new Hub(client, Scope.clone(hub.getScope()));
+    newHub.configureScope(scope => {
+      scope.setTag('foo', 'bar');
+    });
+
+    const tracer = provider.getTracer('default');
+
+    tracer.startActiveSpan('GET /users', parentOtelSpan => {
+      tracer.startActiveSpan('SELECT * FROM users;', child => {
+        makeMain(newHub);
+        child.end();
+      });
+
+      parentOtelSpan.end();
+    });
+
+    // @ts-ignore Accessing private attributes
+    expect(sentryTransaction._hub.getScope()._tags.foo).toEqual('bar');
+  });
 });
 
 // OTEL expects a custom date format