get rid of setSpanScope

Author: mydea

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

@@ -1,5 +1,5 @@
 import { getCurrentScope, setGlobalScope } from '@sentry/core';
-import { getClient, getSpanScope } from '@sentry/opentelemetry';
+import { getClient, getSpanScopes } from '@sentry/opentelemetry';
 
 import * as Sentry from '../../src/';
 import type { NodeExperimentalClient } from '../../src/types';
@@ -41,7 +41,11 @@ describe('Integration | Scope', () => {
           scope2.setTag('tag3', 'val3');
 
           Sentry.startSpan({ name: 'outer' }, span => {
-            expect(getSpanScope(span)).toBe(enableTracing ? scope2 : undefined);
+            // TODO: This is "incorrect" until we stop cloning the current scope for setSpanScopes
+            // Once we change this, the scopes _should_ be the same again
+            if (enableTracing) {
+              expect(getSpanScopes(span)?.scope).not.toBe(scope2);
+            }
 
             spanId = span.spanContext().spanId;
             traceId = span.spanContext().traceId;

packages/opentelemetry/src/asyncContextStrategy.ts

@@ -31,6 +31,10 @@ export function setOpenTelemetryContextAsyncContextStrategy(): void {
     const ctx = api.context.active();
 
     // We depend on the otelContextManager to handle the context/hub
+    // We set the `SENTRY_FORK_ISOLATION_SCOPE_CONTEXT_KEY` context value, which is picked up by
+    // the OTEL context manager, which uses the presence of this key to determine if it should
+    // fork the isolation scope, or not
+    // as by default, we don't want to fork this, unless triggered explicitly by `runWithAsyncContext`
     return api.context.with(ctx.setValue(SENTRY_FORK_ISOLATION_SCOPE_CONTEXT_KEY, true), () => {
       return callback();
     });

packages/opentelemetry/src/index.ts

@@ -10,9 +10,8 @@ export {
   getSpanHub,
   getSpanMetadata,
   getSpanParent,
-  getSpanScope,
   setSpanMetadata,
-  getSpanFinishScopes,
+  getSpanScopes,
 } from './utils/spanData';
 
 export { getPropagationContextFromContext, setPropagationContextOnContext, setHubOnContext } from './utils/contextData';

packages/opentelemetry/src/propagator.ts

@@ -7,7 +7,7 @@ import { SENTRY_BAGGAGE_KEY_PREFIX, generateSentryTraceHeader, propagationContex
 
 import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER } from './constants';
 import { getPropagationContextFromContext, setPropagationContextOnContext } from './utils/contextData';
-import { getSpanScope } from './utils/spanData';
+import { getSpanScopes } from './utils/spanData';
 
 /**
  * Injects and extracts `sentry-trace` and `baggage` headers from carriers.
@@ -91,7 +91,7 @@ function getDsc(
   // Else, we try to generate a new one
   const client = getClient();
   const activeSpan = trace.getSpan(context);
-  const scope = activeSpan ? getSpanScope(activeSpan) : undefined;
+  const { scope } = (activeSpan && getSpanScopes(activeSpan)) || {};
 
   if (client) {
     return getDynamicSamplingContextFromClient(traceId || propagationContext.traceId, client, scope);

packages/opentelemetry/src/spanExporter.ts

@@ -24,7 +24,7 @@ import type { SpanNode } from './utils/groupSpansWithParents';
 import { groupSpansWithParents } from './utils/groupSpansWithParents';
 import { mapStatus } from './utils/mapStatus';
 import { parseSpanDescription } from './utils/parseSpanDescription';
-import { getSpanFinishScopes, getSpanHub, getSpanMetadata, getSpanScope } from './utils/spanData';
+import { getSpanHub, getSpanMetadata, getSpanScopes } from './utils/spanData';
 
 type SpanNodeCompleted = SpanNode & { span: ReadableSpan };
 
@@ -130,7 +130,7 @@ function maybeSend(spans: ReadableSpan[]): ReadableSpan[] {
 function getScopesForTransactionFinish(span: ReadableSpan): { scope: Scope; isolationScope: Scope } {
   // The finish scope should normally always be there (and it is already a clone),
   // but for the sake of type safety we fall back to a clone of the current scope
-  const scopes = getSpanFinishScopes(span);
+  const scopes = getSpanScopes(span);
   const scope = scopes?.scope || getCurrentScope().clone();
   const isolationScope = scopes?.isolationScope || getIsolationScope();
 
@@ -162,7 +162,7 @@ function parseSpan(span: ReadableSpan): { op?: string; origin?: SpanOrigin; sour
 }
 
 function createTransactionForOtelSpan(span: ReadableSpan): Transaction {
-  const scope = getSpanScope(span);
+  const { scope } = getSpanScopes(span) || {};
   // eslint-disable-next-line deprecation/deprecation
   const hub = getSpanHub(span) || getCurrentHub();
   const spanContext = span.spanContext();

packages/opentelemetry/src/spanProcessor.ts

@@ -10,7 +10,7 @@ import { DEBUG_BUILD } from './debug-build';
 import { SentrySpanExporter } from './spanExporter';
 import { maybeCaptureExceptionForTimedEvent } from './utils/captureExceptionForTimedEvent';
 import { getHubFromContext } from './utils/contextData';
-import { getSpanHub, setSpanFinishScopes, setSpanHub, setSpanParent, setSpanScope } from './utils/spanData';
+import { getSpanHub, setSpanHub, setSpanParent, setSpanScopes } from './utils/spanData';
 
 function onSpanStart(span: Span, parentContext: Context, _ScopeClass: typeof OpenTelemetryScope): void {
   // This is a reliable way to get the parent span - because this is exactly how the parent is identified in the OTEL SDK
@@ -36,14 +36,16 @@ function onSpanStart(span: Span, parentContext: Context, _ScopeClass: typeof Ope
     const scope = actualHub.getScope();
     // eslint-disable-next-line deprecation/deprecation
     const isolationScope = actualHub.getIsolationScope();
-    setSpanScope(span, scope);
     setSpanHub(span, actualHub);
 
     // Use this scope for finishing the span
+    // TODO: For now we need to clone this, as we need to store the `activeSpan` on it
+    // Once we can get rid of this (when we move breadcrumbs to the isolation scope),
+    // we can stop cloning this here
     const finishScope = (scope as OpenTelemetryScope).clone();
     // this is needed for breadcrumbs, for now, as they are stored on the span currently
     finishScope.activeSpan = span;
-    setSpanFinishScopes(span, { scope: finishScope, isolationScope });
+    setSpanScopes(span, { scope: finishScope, isolationScope });
   }
 }
 

packages/opentelemetry/src/utils/spanData.ts

@@ -3,11 +3,10 @@ import type { Hub, Scope, TransactionMetadata } from '@sentry/types';
 
 import type { AbstractSpan } from '../types';
 
-// We store the parent span, scope & metadata in separate weakmaps, so we can access them for a given span
+// We store the parent span, scopes & metadata in separate weakmaps, so we can access them for a given span
 // This way we can enhance the data that an OTEL Span natively gives us
 // and since we are using weakmaps, we do not need to clean up after ourselves
-const SpanScope = new WeakMap<AbstractSpan, Scope>();
-const SpanFinishScopes = new WeakMap<
+const SpanScopes = new WeakMap<
   AbstractSpan,
   {
     scope: Scope;
@@ -18,16 +17,6 @@ const SpanHub = new WeakMap<AbstractSpan, Hub>();
 const SpanParent = new WeakMap<AbstractSpan, Span>();
 const SpanMetadata = new WeakMap<AbstractSpan, Partial<TransactionMetadata>>();
 
-/** Set the Sentry scope on an OTEL span. */
-export function setSpanScope(span: AbstractSpan, scope: Scope): void {
-  SpanScope.set(span, scope);
-}
-
-/** Get the Sentry scope of an OTEL span. */
-export function getSpanScope(span: AbstractSpan): Scope | undefined {
-  return SpanScope.get(span);
-}
-
 /** Set the Sentry hub on an OTEL span. */
 export function setSpanHub(span: AbstractSpan, hub: Hub): void {
   SpanHub.set(span, hub);
@@ -58,23 +47,27 @@ export function getSpanMetadata(span: AbstractSpan): Partial<TransactionMetadata
   return SpanMetadata.get(span);
 }
 
-/** Set the Sentry scope to be used for finishing a given OTEL span. */
-export function setSpanFinishScopes(
+/**
+ * Set the Sentry scope to be used for finishing a given OTEL span.
+ * This is different from `setCapturedScopesOnSpan`, as that works on _sentry_ spans,
+ * while here we are basically "caching" this on the otel spans.
+ */
+export function setSpanScopes(
   span: AbstractSpan,
   scopes: {
     scope: Scope;
     isolationScope: Scope;
   },
 ): void {
-  SpanFinishScopes.set(span, scopes);
+  SpanScopes.set(span, scopes);
 }
 
-/** Get the Sentry scope to use for finishing an OTEL span. */
-export function getSpanFinishScopes(span: AbstractSpan):
+/** Get the Sentry scopes to use for finishing an OTEL span. */
+export function getSpanScopes(span: AbstractSpan):
   | {
       scope: Scope;
       isolationScope: Scope;
     }
   | undefined {
-  return SpanFinishScopes.get(span);
+  return SpanScopes.get(span);
 }

packages/opentelemetry/test/integration/scope.test.ts

@@ -12,7 +12,7 @@ import {
 import { OpenTelemetryHub } from '../../src/custom/hub';
 import { OpenTelemetryScope } from '../../src/custom/scope';
 import { startSpan } from '../../src/trace';
-import { getSpanScope } from '../../src/utils/spanData';
+import { getSpanScopes } from '../../src/utils/spanData';
 import type { TestClientInterface } from '../helpers/TestClient';
 import { cleanupOtel, mockSdkInit } from '../helpers/mockSdkInit';
 
@@ -57,7 +57,11 @@ describe('Integration | Scope', () => {
           scope2.setTag('tag3', 'val3');
 
           startSpan({ name: 'outer' }, span => {
-            expect(getSpanScope(span)).toBe(enableTracing ? scope2 : undefined);
+            // TODO: This is "incorrect" until we stop cloning the current scope for setSpanScopes
+            // Once we change this, the scopes _should_ be the same again
+            if (enableTracing) {
+              expect(getSpanScopes(span)?.scope).not.toBe(scope2);
+            }
 
             spanId = span.spanContext().spanId;
             traceId = span.spanContext().traceId;