feat(core): Add span.spanContext()

mydea · mydea · commit 495442fdd1f8 · 2024-01-04T12:31:12.000+01:00
diff --git a/MIGRATION.md b/MIGRATION.md
@@ -17,6 +17,9 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
 * `span.setName(newName)`: Use `span.updateName(newName)` instead.
 * `span.toTraceparent()`: use `spanToTraceHeader(span)` util instead.
 * `span.getTraceContext()`: Use `spanToTraceContext(span)` utility function instead.
+* `span.spanId`: Use `span.spanContext().spanId` instead.
+* `span.traceId`: Use `span.spanContext().traceId` instead.
+* `span.sampled`: Use `spanIsSampled(span)` instead.
 
 ## Deprecate `pushScope` & `popScope` in favor of `withScope`
 
diff --git a/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts
@@ -39,5 +39,6 @@ sentryTest('should report finished spans as children of the root transaction', a
 
   const span_5 = transaction.spans?.[2];
   expect(span_5?.op).toBe('span_5');
+  // eslint-disable-next-line deprecation/deprecation
   expect(span_5?.parentSpanId).toEqual(span_3?.spanId);
 });
diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts
@@ -28,6 +28,7 @@ app.use(cors());
 app.get('/test/express', (_req, res) => {
   const transaction = Sentry.getCurrentHub().getScope().getTransaction();
   if (transaction) {
+    // eslint-disable-next-line deprecation/deprecation
     transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
   }
   const headers = http.get('http://somewhere.not.sentry/').getHeaders();
diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts
@@ -31,6 +31,7 @@ app.use(cors());
 app.get('/test/express', (_req, res) => {
   const transaction = Sentry.getCurrentHub().getScope().getTransaction();
   if (transaction) {
+    // eslint-disable-next-line deprecation/deprecation
     transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
     transaction.setMetadata({ source: 'route' });
   }
diff --git a/packages/astro/test/server/meta.test.ts b/packages/astro/test/server/meta.test.ts
@@ -4,16 +4,23 @@ import { vi } from 'vitest';
 import { getTracingMetaTags, isValidBaggageString } from '../../src/server/meta';
 
 const mockedSpan = {
-  sampled: true,
-  traceId: '12345678901234567890123456789012',
-  spanId: '1234567890123456',
+  spanContext: () => {
+    return {
+      traceId: '12345678901234567890123456789012',
+      spanId: '1234567890123456',
+      traceFlags: TraceFlagSampled,
+    };
+  },
   transaction: {
     getDynamicSamplingContext: () => ({
       environment: 'production',
     }),
   },
 } as any;
 
+// eslint-disable-next-line no-bitwise
+const TraceFlagSampled = 0x1 << 0;
+
 const mockedClient = {} as any;
 
 const mockedScope = {
@@ -70,9 +77,13 @@ describe('getTracingMetaTags', () => {
     const tags = getTracingMetaTags(
       // @ts-expect-error - only passing a partial span object
       {
-        sampled: true,
-        traceId: '12345678901234567890123456789012',
-        spanId: '1234567890123456',
+        spanContext: () => {
+          return {
+            traceId: '12345678901234567890123456789012',
+            spanId: '1234567890123456',
+            traceFlags: TraceFlagSampled,
+          };
+        },
         transaction: undefined,
       },
       mockedScope,
@@ -84,7 +95,7 @@ describe('getTracingMetaTags', () => {
     });
   });
 
-  it('returns only the `sentry-trace` tag if no DSC is available', () => {
+  it('returns only the `sentry-trace` tag if no DSC is available without a client', () => {
     vi.spyOn(SentryCore, 'getDynamicSamplingContextFromClient').mockReturnValueOnce({
       trace_id: '',
       public_key: undefined,
@@ -93,9 +104,13 @@ describe('getTracingMetaTags', () => {
     const tags = getTracingMetaTags(
       // @ts-expect-error - only passing a partial span object
       {
-        sampled: true,
-        traceId: '12345678901234567890123456789012',
-        spanId: '1234567890123456',
+        spanContext: () => {
+          return {
+            traceId: '12345678901234567890123456789012',
+            spanId: '1234567890123456',
+            traceFlags: TraceFlagSampled,
+          };
+        },
         transaction: undefined,
       },
       mockedScope,
diff --git a/packages/browser/src/profiling/utils.ts b/packages/browser/src/profiling/utils.ts
@@ -1,6 +1,6 @@
 /* eslint-disable max-lines */
 
-import { DEFAULT_ENVIRONMENT, getClient } from '@sentry/core';
+import { DEFAULT_ENVIRONMENT, getClient, spanIsSampled } from '@sentry/core';
 import type { DebugImage, Envelope, Event, EventEnvelope, StackFrame, StackParser, Transaction } from '@sentry/types';
 import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling';
 import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils';
@@ -515,7 +515,7 @@ export function shouldProfileTransaction(transaction: Transaction): boolean {
     return false;
   }
 
-  if (!transaction.sampled) {
+  if (!spanIsSampled(transaction)) {
     if (DEBUG_BUILD) {
       logger.log('[Profiling] Discarding profile because transaction was not sampled.');
     }
diff --git a/packages/browser/test/unit/profiling/hubextensions.test.ts b/packages/browser/test/unit/profiling/hubextensions.test.ts
@@ -10,6 +10,9 @@ import { JSDOM } from 'jsdom';
 
 import { onProfilingStartRouteTransaction } from '../../../src';
 
+// eslint-disable-next-line no-bitwise
+const TraceFlagSampled = 0x1 << 0;
+
 // @ts-expect-error store a reference so we can reset it later
 const globalDocument = global.document;
 // @ts-expect-error store a reference so we can reset it later
@@ -67,9 +70,16 @@ describe('BrowserProfilingIntegration', () => {
     // @ts-expect-error force api to be undefined
     global.window.Profiler = undefined;
     // set sampled to true so that profiling does not early return
-    const mockTransaction = { sampled: true } as Transaction;
+    const mockTransaction = {
+      spanContext: () => ({
+        traceId: '12345678901234567890123456789012',
+        spanId: '1234567890123456',
+        traceFlags: TraceFlagSampled,
+      }),
+    } as Transaction;
     expect(() => onProfilingStartRouteTransaction(mockTransaction)).not.toThrow();
   });
+
   it('does not throw if constructor throws', () => {
     const spy = jest.fn();
 
@@ -81,7 +91,13 @@ describe('BrowserProfilingIntegration', () => {
     }
 
     // set sampled to true so that profiling does not early return
-    const mockTransaction = { sampled: true } as Transaction;
+    const mockTransaction = {
+      spanContext: () => ({
+        traceId: '12345678901234567890123456789012',
+        spanId: '1234567890123456',
+        traceFlags: TraceFlagSampled,
+      }),
+    } as Transaction;
 
     // @ts-expect-error override with our own constructor
     global.window.Profiler = Profiler;
diff --git a/packages/bun/test/integrations/bunserver.test.ts b/packages/bun/test/integrations/bunserver.test.ts
@@ -1,5 +1,5 @@
 import { beforeAll, beforeEach, describe, expect, test } from 'bun:test';
-import { Hub, makeMain } from '@sentry/core';
+import { Hub, makeMain, spanIsSampled } from '@sentry/core';
 
 import { BunClient } from '../../src/client';
 import { instrumentBunServe } from '../../src/integrations/bunserver';
@@ -78,9 +78,9 @@ describe('Bun Serve Integration', () => {
     const SENTRY_BAGGAGE_HEADER = 'sentry-version=1.0,sentry-environment=production';
 
     client.on('finishTransaction', transaction => {
-      expect(transaction.traceId).toBe(TRACE_ID);
+      expect(transaction.spanContext().traceId).toBe(TRACE_ID);
       expect(transaction.parentSpanId).toBe(PARENT_SPAN_ID);
-      expect(transaction.sampled).toBe(true);
+      expect(spanIsSampled(transaction)).toBe(true);
 
       expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' });
     });
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -70,7 +70,7 @@ export { createCheckInEnvelope } from './checkin';
 export { hasTracingEnabled } from './utils/hasTracingEnabled';
 export { isSentryRequestUrl } from './utils/isSentryRequestUrl';
 export { handleCallbackErrors } from './utils/handleCallbackErrors';
-export { spanToTraceHeader } from './utils/spanUtils';
+export { spanToTraceHeader, spanIsSampled } from './utils/spanUtils';
 export { DEFAULT_ENVIRONMENT } from './constants';
 export { ModuleMetadata } from './integrations/metadata';
 export { RequestData } from './integrations/requestdata';
diff --git a/packages/core/src/tracing/idletransaction.ts b/packages/core/src/tracing/idletransaction.ts
@@ -45,18 +45,18 @@ export class IdleTransactionSpanRecorder extends SpanRecorder {
   public add(span: Span): void {
     // We should make sure we do not push and pop activities for
     // the transaction that this span recorder belongs to.
-    if (span.spanId !== this.transactionSpanId) {
+    if (span.spanContext().spanId !== this.transactionSpanId) {
       // We patch span.end() to pop an activity after setting an endTimestamp.
       // eslint-disable-next-line @typescript-eslint/unbound-method
       const originalEnd = span.end;
       span.end = (...rest: unknown[]) => {
-        this._popActivity(span.spanId);
+        this._popActivity(span.spanContext().spanId);
         return originalEnd.apply(span, rest);
       };
 
       // We should only push new activities if the span does not have an end timestamp.
       if (span.endTimestamp === undefined) {
-        this._pushActivity(span.spanId);
+        this._pushActivity(span.spanContext().spanId);
       }
     }
 
@@ -123,7 +123,7 @@ export class IdleTransaction extends Transaction {
     if (_onScope) {
       // We set the transaction here on the scope so error events pick up the trace
       // context and attach it to the error.
-      DEBUG_BUILD && logger.log(`Setting idle transaction on scope. Span ID: ${this.spanId}`);
+      DEBUG_BUILD && logger.log(`Setting idle transaction on scope. Span ID: ${this.spanContext().spanId}`);
       _idleHub.getScope().setSpan(this);
     }
 
@@ -158,7 +158,7 @@ export class IdleTransaction extends Transaction {
 
       this.spanRecorder.spans = this.spanRecorder.spans.filter((span: Span) => {
         // If we are dealing with the transaction itself, we just return it
-        if (span.spanId === this.spanId) {
+        if (span.spanContext().spanId === this.spanContext().spanId) {
           return true;
         }
 
@@ -233,7 +233,7 @@ export class IdleTransaction extends Transaction {
         this._popActivity(id);
       };
 
-      this.spanRecorder = new IdleTransactionSpanRecorder(pushActivity, popActivity, this.spanId, maxlen);
+      this.spanRecorder = new IdleTransactionSpanRecorder(pushActivity, popActivity, this.spanContext().spanId, maxlen);
 
       // Start heartbeat so that transactions do not run forever.
       DEBUG_BUILD && logger.log('Starting heartbeat');
diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts
@@ -6,6 +6,7 @@ import type {
   SpanAttributeValue,
   SpanAttributes,
   SpanContext,
+  SpanContextData,
   SpanOrigin,
   TraceContext,
   Transaction,
@@ -16,6 +17,10 @@ import { DEBUG_BUILD } from '../debug-build';
 import { spanToTraceContext, spanToTraceHeader } from '../utils/spanUtils';
 import { ensureTimestampInSeconds } from './utils';
 
+export const TraceFlagNone = 0x0;
+// eslint-disable-next-line no-bitwise
+export const TraceFlagSampled = 0x1 << 0;
+
 /**
  * Keeps track of finished spans for a given transaction
  * @internal
@@ -184,6 +189,16 @@ export class Span implements SpanInterface {
     this.updateName(name);
   }
 
+  /** @inheritdoc */
+  public spanContext(): SpanContextData {
+    const { spanId, traceId, sampled } = this;
+    return {
+      spanId,
+      traceId,
+      traceFlags: sampled ? TraceFlagSampled : TraceFlagNone,
+    };
+  }
+
   /**
    * @inheritDoc
    */
@@ -207,7 +222,7 @@ export class Span implements SpanInterface {
     if (DEBUG_BUILD && childSpan.transaction) {
       const opStr = (spanContext && spanContext.op) || '< unknown op >';
       const nameStr = childSpan.transaction.name || '< unknown name >';
-      const idStr = childSpan.transaction.spanId;
+      const idStr = childSpan.transaction.spanContext().spanId;
 
       const logMessage = `[Tracing] Starting '${opStr}' span on transaction '${nameStr}' (${idStr}).`;
       childSpan.transaction.metadata.spanMetadata[childSpan.spanId] = { logMessage };
@@ -305,7 +320,7 @@ export class Span implements SpanInterface {
       DEBUG_BUILD &&
       // Don't call this for transactions
       this.transaction &&
-      this.transaction.spanId !== this.spanId
+      this.transaction.spanContext().spanId !== this.spanId
     ) {
       const { logMessage } = this.transaction.metadata.spanMetadata[this.spanId];
       if (logMessage) {
diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts
@@ -1,5 +1,6 @@
 import type { Span, TraceContext } from '@sentry/types';
 import { dropUndefinedKeys, generateSentryTraceHeader } from '@sentry/utils';
+import { TraceFlagSampled } from '../tracing/span';
 
 /**
  * Convert a span to a trace context, which can be sent as the `trace` context in an event.
@@ -24,5 +25,14 @@ export function spanToTraceContext(span: Span): TraceContext {
  * Convert a Span to a Sentry trace header.
  */
 export function spanToTraceHeader(span: Span): string {
-  return generateSentryTraceHeader(span.traceId, span.spanId, span.sampled);
+  const { traceId, spanId } = span.spanContext();
+  const sampled = spanIsSampled(span);
+  return generateSentryTraceHeader(traceId, spanId, sampled);
+}
+
+/** Returns true if a span is sampled. */
+export function spanIsSampled(span: Span): boolean {
+  const { traceFlags } = span.spanContext();
+  // eslint-disable-next-line no-bitwise
+  return Boolean(traceFlags & TraceFlagSampled);
 }
diff --git a/packages/core/test/lib/tracing/span.test.ts b/packages/core/test/lib/tracing/span.test.ts
@@ -1,4 +1,5 @@
 import { Span } from '../../../src';
+import { TraceFlagNone, TraceFlagSampled } from '../../../src/tracing/span';
 
 describe('span', () => {
   it('works with name', () => {
@@ -174,6 +175,35 @@ describe('span', () => {
     });
   });
 
+  describe('spanContext', () => {
+    it('works with default span', () => {
+      const span = new Span();
+      expect(span.spanContext()).toEqual({
+        spanId: span.spanId,
+        traceId: span.traceId,
+        traceFlags: TraceFlagNone,
+      });
+    });
+
+    it('works sampled span', () => {
+      const span = new Span({ sampled: true });
+      expect(span.spanContext()).toEqual({
+        spanId: span.spanId,
+        traceId: span.traceId,
+        traceFlags: TraceFlagSampled,
+      });
+    });
+
+    it('works unsampled span', () => {
+      const span = new Span({ sampled: false });
+      expect(span.spanContext()).toEqual({
+        spanId: span.spanId,
+        traceId: span.traceId,
+        traceFlags: TraceFlagNone,
+      });
+    });
+  });
+
   // Ensure that attributes & data are merged together
   describe('_getData', () => {
     it('works without data & attributes', () => {
diff --git a/packages/core/test/lib/utils/spanUtils.test.ts b/packages/core/test/lib/utils/spanUtils.test.ts
@@ -1,5 +1,6 @@
 import { TRACEPARENT_REGEXP } from '@sentry/utils';
 import { Span, spanToTraceHeader } from '../../../src';
+import { spanIsSampled } from '../../../src/utils/spanUtils';
 
 describe('spanToTraceHeader', () => {
   test('simple', () => {
@@ -11,3 +12,15 @@ describe('spanToTraceHeader', () => {
     expect(spanToTraceHeader(span)).toMatch(TRACEPARENT_REGEXP);
   });
 });
+
+describe('spanIsSampled', () => {
+  test('sampled', () => {
+    const span = new Span({ sampled: true });
+    expect(spanIsSampled(span)).toBe(true);
+  });
+
+  test('not sampled', () => {
+    const span = new Span({ sampled: false });
+    expect(spanIsSampled(span)).toBe(false);
+  });
+});
diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts
@@ -448,7 +448,7 @@ describe('tracingHandler', () => {
       expect(finishTransaction).toHaveBeenCalled();
       expect(span.endTimestamp).toBeLessThanOrEqual(transaction.endTimestamp!);
       expect(sentEvent.spans?.length).toEqual(1);
-      expect(sentEvent.spans?.[0].spanId).toEqual(span.spanId);
+      expect(sentEvent.spans?.[0].spanContext().spanId).toEqual(span.spanId);
       done();
     });
   });
diff --git a/packages/opentelemetry-node/src/utils/spanMap.ts b/packages/opentelemetry-node/src/utils/spanMap.ts
@@ -31,7 +31,7 @@ export function getSentrySpan(spanId: string): SentrySpan | undefined {
 export function setSentrySpan(spanId: string, sentrySpan: SentrySpan): void {
   let ref: SpanRefType = SPAN_REF_ROOT;
 
-  const rootSpanId = sentrySpan.transaction?.spanId;
+  const rootSpanId = sentrySpan.transaction?.spanContext().spanId;
 
   if (rootSpanId && rootSpanId !== spanId) {
     const root = SPAN_MAP.get(rootSpanId);
diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts
diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts
diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts
diff --git a/packages/tracing-internal/src/common/fetch.ts b/packages/tracing-internal/src/common/fetch.ts
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts
diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts

Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ app.use(cors());`
`28`	`28`	`app.get('/test/express', (_req, res) => {`
`29`	`29`	`const transaction = Sentry.getCurrentHub().getScope().getTransaction();`
`30`	`30`	`if (transaction) {`
	`31`	`+ // eslint-disable-next-line deprecation/deprecation`
`31`	`32`	`transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';`
`32`	`33`	`}`
`33`	`34`	`const headers = http.get('http://somewhere.not.sentry/').getHeaders();`
Original file line number	Diff line number	Diff line change
`@@ -31,6 +31,7 @@ app.use(cors());`
`31`	`31`	`app.get('/test/express', (_req, res) => {`
`32`	`32`	`const transaction = Sentry.getCurrentHub().getScope().getTransaction();`
`33`	`33`	`if (transaction) {`
	`34`	`+ // eslint-disable-next-line deprecation/deprecation`
`34`	`35`	`transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';`
`35`	`36`	`transaction.setMetadata({ source: 'route' });`
`36`	`37`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`/* eslint-disable max-lines */`
`2`	`2`
`3`		`-import { DEFAULT_ENVIRONMENT, getClient } from '@sentry/core';`
	`3`	`+import { DEFAULT_ENVIRONMENT, getClient, spanIsSampled } from '@sentry/core';`
`4`	`4`	`import type { DebugImage, Envelope, Event, EventEnvelope, StackFrame, StackParser, Transaction } from '@sentry/types';`
`5`	`5`	`import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling';`
`6`	`6`	`import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils';`
`@@ -515,7 +515,7 @@ export function shouldProfileTransaction(transaction: Transaction): boolean {`
`515`	`515`	`return false;`
`516`	`516`	`}`
`517`	`517`
`518`		`- if (!transaction.sampled) {`
	`518`	`+ if (!spanIsSampled(transaction)) {`
`519`	`519`	`if (DEBUG_BUILD) {`
`520`	`520`	`logger.log('[Profiling] Discarding profile because transaction was not sampled.');`
`521`	`521`	`}`