fix(otel): Ensure we properly set the context for transactions (#6159)

Previously, due to timing issues with async event processors the context was sometimes not set properly.

As a side effect, that also makes testing this way nicer, yayy!

Author: mydea

packages/opentelemetry-node/src/spanprocessor.ts

@@ -1,6 +1,6 @@
 import { Context } from '@opentelemetry/api';
 import { Span as OtelSpan, SpanProcessor as OtelSpanProcessor } from '@opentelemetry/sdk-trace-base';
-import { getCurrentHub, withScope } from '@sentry/core';
+import { getCurrentHub } from '@sentry/core';
 import { Transaction } from '@sentry/tracing';
 import { DynamicSamplingContext, Span as SentrySpan, TraceparentData, TransactionContext } from '@sentry/types';
 import { logger } from '@sentry/utils';
@@ -90,12 +90,12 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
 
     if (sentrySpan instanceof Transaction) {
       updateTransactionWithOtelData(sentrySpan, otelSpan);
-      finishTransactionWithContextFromOtelData(sentrySpan, otelSpan);
     } else {
       updateSpanWithOtelData(sentrySpan, otelSpan);
-      sentrySpan.finish(convertOtelTimeToSeconds(otelSpan.endTime));
     }
 
+    sentrySpan.finish(convertOtelTimeToSeconds(otelSpan.endTime));
+
     SENTRY_SPAN_PROCESSOR_MAP.delete(otelSpanId);
   }
 
@@ -141,17 +141,6 @@ function getTraceData(otelSpan: OtelSpan, parentContext: Context): Partial<Trans
   };
 }
 
-function finishTransactionWithContextFromOtelData(transaction: Transaction, otelSpan: OtelSpan): void {
-  withScope(scope => {
-    scope.setContext('otel', {
-      attributes: otelSpan.attributes,
-      resource: otelSpan.resource.attributes,
-    });
-
-    transaction.finish(convertOtelTimeToSeconds(otelSpan.endTime));
-  });
-}
-
 function updateSpanWithOtelData(sentrySpan: SentrySpan, otelSpan: OtelSpan): void {
   const { attributes, kind } = otelSpan;
 
@@ -169,6 +158,11 @@ function updateSpanWithOtelData(sentrySpan: SentrySpan, otelSpan: OtelSpan): voi
 }
 
 function updateTransactionWithOtelData(transaction: Transaction, otelSpan: OtelSpan): void {
+  transaction.setContext('otel', {
+    attributes: otelSpan.attributes,
+    resource: otelSpan.resource.attributes,
+  });
+
   transaction.setStatus(mapOtelStatus(otelSpan));
 
   const { op, description } = parseSpanDescription(otelSpan);

packages/opentelemetry-node/test/spanprocessor.test.ts

@@ -7,7 +7,6 @@ import { SemanticAttributes, SemanticResourceAttributes } from '@opentelemetry/s
 import { createTransport, Hub, makeMain } from '@sentry/core';
 import { NodeClient } from '@sentry/node';
 import { addExtensionMethods, Span as SentrySpan, SpanStatusType, Transaction } from '@sentry/tracing';
-import { Scope } from '@sentry/types';
 import { resolvedSyncPromise } from '@sentry/utils';
 
 import { SENTRY_SPAN_PROCESSOR_MAP, SentrySpanProcessor } from '../src/spanprocessor';
@@ -60,22 +59,6 @@ describe('SentrySpanProcessor', () => {
     return transactionWithContext._contexts;
   }
 
-  // monkey-patch finish to store the context at finish time
-  function monkeyPatchTransactionFinish(transaction: Transaction) {
-    const monkeyPatchedTransaction = transaction as Transaction;
-
-    // eslint-disable-next-line @typescript-eslint/unbound-method
-    const originalFinish = monkeyPatchedTransaction.finish;
-    // @ts-ignore accessing private property
-    monkeyPatchedTransaction._contexts = {};
-    monkeyPatchedTransaction.finish = function (endTimestamp?: number | undefined) {
-      // @ts-ignore accessing private property
-      monkeyPatchedTransaction._contexts = (transaction._hub.getScope() as unknown as Scope)._contexts;
-
-      return originalFinish.apply(monkeyPatchedTransaction, [endTimestamp]);
-    };
-  }
-
   it('creates a transaction', async () => {
     const startTimestampMs = 1667381672875;
     const endTimestampMs = 1667381672309;
@@ -173,7 +156,6 @@ describe('SentrySpanProcessor', () => {
     const otelSpan = provider.getTracer('default').startSpan('GET /users');
 
     const transaction = getSpanForOtelSpan(otelSpan) as Transaction;
-    monkeyPatchTransactionFinish(transaction);
 
     // context is only set after end
     expect(getContext(transaction)).toEqual({});
@@ -196,7 +178,6 @@ describe('SentrySpanProcessor', () => {
     const otelSpan2 = provider.getTracer('default').startSpan('GET /companies');
 
     const transaction2 = getSpanForOtelSpan(otelSpan2) as Transaction;
-    monkeyPatchTransactionFinish(transaction2);
 
     expect(getContext(transaction2)).toEqual({});