refactor toTraceparent into extractTraceparentData

Author: lobsterkatie

packages/node/src/handlers.ts

@@ -1,7 +1,7 @@
 /* eslint-disable max-lines */
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import { captureException, getCurrentHub, startTransaction, withScope } from '@sentry/core';
-import { Span } from '@sentry/tracing';
+import { extractTraceparentData, Span } from '@sentry/tracing';
 import { Event } from '@sentry/types';
 import {
   extractNodeRequestData,
@@ -40,26 +40,16 @@ export function tracingHandler(): (
     const reqMethod = (req.method || '').toUpperCase();
     const reqUrl = req.url && stripUrlQueryAndFragment(req.url);
 
-    let traceId;
-    let parentSpanId;
-    let sampled;
-
     // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision)
+    let traceparentData;
     if (req.headers && isString(req.headers['sentry-trace'])) {
-      const span = Span.fromTraceparent(req.headers['sentry-trace'] as string);
-      if (span) {
-        traceId = span.traceId;
-        parentSpanId = span.parentSpanId;
-        sampled = span.sampled;
-      }
+      traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string);
     }
 
     const transaction = startTransaction({
       name: `${reqMethod} ${reqUrl}`,
       op: 'http.server',
-      parentSpanId,
-      sampled,
-      traceId,
+      ...traceparentData,
     });
 
     // We put the transaction on the scope so users can attach children to it

packages/tracing/src/browser/browsertracing.ts

@@ -4,8 +4,8 @@ import { logger } from '@sentry/utils';
 
 import { startIdleTransaction } from '../hubextensions';
 import { DEFAULT_IDLE_TIMEOUT, IdleTransaction } from '../idletransaction';
-import { Span } from '../span';
 import { SpanStatus } from '../spanstatus';
+import { extractTraceparentData } from '../utils';
 import { registerBackgroundTabDetection } from './backgroundtab';
 import { MetricsInstrumentation } from './metrics';
 import {
@@ -213,21 +213,16 @@ export class BrowserTracing implements Integration {
 
 /**
  * Gets transaction context from a sentry-trace meta.
+ *
+ * @returns Transaction context data from the header or undefined if there's no header or the header is malformed
  */
-function getHeaderContext(): Partial<TransactionContext> {
+function getHeaderContext(): Partial<TransactionContext> | undefined {
   const header = getMetaContent('sentry-trace');
   if (header) {
-    const span = Span.fromTraceparent(header);
-    if (span) {
-      return {
-        parentSpanId: span.parentSpanId,
-        sampled: span.sampled,
-        traceId: span.traceId,
-      };
-    }
+    return extractTraceparentData(header);
   }
 
-  return {};
+  return undefined;
 }
 
 /** Returns the value of a meta tag */

packages/tracing/src/index.bundle.ts

@@ -55,7 +55,7 @@ import { getGlobalObject } from '@sentry/utils';
 import { BrowserTracing } from './browser';
 import { addExtensionMethods } from './hubextensions';
 
-export { Span, TRACEPARENT_REGEXP } from './span';
+export { Span } from './span';
 
 let windowIntegrations = {};
 

packages/tracing/src/index.ts

@@ -5,7 +5,7 @@ import * as ApmIntegrations from './integrations';
 const Integrations = { ...ApmIntegrations, BrowserTracing };
 
 export { Integrations };
-export { Span, TRACEPARENT_REGEXP } from './span';
+export { Span } from './span';
 export { Transaction } from './transaction';
 
 export { SpanStatus } from './spanstatus';

packages/tracing/src/span.ts

@@ -4,14 +4,6 @@ import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils';
 
 import { SpanStatus } from './spanstatus';
 
-export const TRACEPARENT_REGEXP = new RegExp(
-  '^[ \\t]*' + // whitespace
-  '([0-9a-f]{32})?' + // trace_id
-  '-?([0-9a-f]{16})?' + // span_id
-  '-?([01])?' + // sampled
-    '[ \\t]*$', // whitespace
-);
-
 /**
  * Keeps track of finished spans for a given transaction
  * @internal
@@ -153,32 +145,6 @@ export class Span implements SpanInterface, SpanContext {
     }
   }
 
-  /**
-   * Continues a trace from a string (usually the header).
-   * @param traceparent Traceparent string
-   */
-  public static fromTraceparent(
-    traceparent: string,
-    spanContext?: Pick<SpanContext, Exclude<keyof SpanContext, 'spanId' | 'sampled' | 'traceId' | 'parentSpanId'>>,
-  ): Span | undefined {
-    const matches = traceparent.match(TRACEPARENT_REGEXP);
-    if (matches) {
-      let sampled: boolean | undefined;
-      if (matches[3] === '1') {
-        sampled = true;
-      } else if (matches[3] === '0') {
-        sampled = false;
-      }
-      return new Span({
-        ...spanContext,
-        parentSpanId: matches[2],
-        sampled,
-        traceId: matches[1],
-      });
-    }
-    return undefined;
-  }
-
   /**
    * @inheritDoc
    * @deprecated

packages/tracing/src/utils.ts

@@ -1,4 +1,12 @@
-import { Options } from '@sentry/types';
+import { Options, TraceparentData } from '@sentry/types';
+
+export const TRACEPARENT_REGEXP = new RegExp(
+  '^[ \\t]*' + // whitespace
+  '([0-9a-f]{32})?' + // trace_id
+  '-?([0-9a-f]{16})?' + // span_id
+  '-?([01])?' + // sampled
+    '[ \\t]*$', // whitespace
+);
 
 /**
  * Determines if tracing is currently enabled.
@@ -8,3 +16,28 @@ import { Options } from '@sentry/types';
 export function hasTracingEnabled(options: Options): boolean {
   return 'tracesSampleRate' in options || 'tracesSampler' in options;
 }
+
+/**
+ * Extract transaction context data from a `sentry-trace` header.
+ *
+ * @param traceparent Traceparent string
+ *
+ * @returns Object containing data from the header, or undefined if traceparent string is malformed
+ */
+export function extractTraceparentData(traceparent: string): TraceparentData | undefined {
+  const matches = traceparent.match(TRACEPARENT_REGEXP);
+  if (matches) {
+    let parentSampled: boolean | undefined;
+    if (matches[3] === '1') {
+      parentSampled = true;
+    } else if (matches[3] === '0') {
+      parentSampled = false;
+    }
+    return {
+      traceId: matches[1],
+      parentSampled,
+      parentSpanId: matches[2],
+    };
+  }
+  return undefined;
+}

packages/tracing/test/span.test.ts

@@ -100,42 +100,6 @@ describe('Span', () => {
     });
   });
 
-  describe('fromTraceparent', () => {
-    test('no sample', () => {
-      const from = Span.fromTraceparent('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb') as any;
-
-      expect(from.parentSpanId).toEqual('bbbbbbbbbbbbbbbb');
-      expect(from.traceId).toEqual('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa');
-      expect(from.spanId).not.toEqual('bbbbbbbbbbbbbbbb');
-      expect(from.sampled).toBeUndefined();
-    });
-    test('sample true', () => {
-      const from = Span.fromTraceparent('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-1') as any;
-      expect(from.sampled).toBeTruthy();
-    });
-
-    test('sample false', () => {
-      const from = Span.fromTraceparent('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-0') as any;
-      expect(from.sampled).toBeFalsy();
-    });
-
-    test('just sample rate', () => {
-      const from = Span.fromTraceparent('0') as any;
-      expect(from.traceId).toHaveLength(32);
-      expect(from.spanId).toHaveLength(16);
-      expect(from.sampled).toBeFalsy();
-
-      const from2 = Span.fromTraceparent('1') as any;
-      expect(from2.traceId).toHaveLength(32);
-      expect(from2.spanId).toHaveLength(16);
-      expect(from2.sampled).toBeTruthy();
-    });
-
-    test('invalid', () => {
-      expect(Span.fromTraceparent('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-x')).toBeUndefined();
-    });
-  });
-
   describe('toJSON', () => {
     test('simple', () => {
       const span = JSON.parse(

packages/tracing/test/utils.test.ts

@@ -0,0 +1,64 @@
+import { extractTraceparentData } from '../src/utils';
+
+describe('extractTraceparentData', () => {
+  test('no sample', () => {
+    const data = extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb') as any;
+
+    expect(data).toBeDefined();
+    expect(data.parentSpanId).toEqual('bbbbbbbbbbbbbbbb');
+    expect(data.traceId).toEqual('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa');
+    expect(data?.parentSampled).toBeUndefined();
+  });
+
+  test('sample true', () => {
+    const data = extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-1') as any;
+
+    expect(data).toBeDefined();
+    expect(data.parentSampled).toBeTruthy();
+  });
+
+  test('sample false', () => {
+    const data = extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-0') as any;
+
+    expect(data).toBeDefined();
+    expect(data.parentSampled).toBeFalsy();
+  });
+
+  test('just sample decision - false', () => {
+    const data = extractTraceparentData('0') as any;
+
+    expect(data).toBeDefined();
+    expect(data.traceId).toBeUndefined();
+    expect(data.spanId).toBeUndefined();
+    expect(data.parentSampled).toBeFalsy();
+  });
+
+  test('just sample decision - true', () => {
+    const data = extractTraceparentData('1') as any;
+
+    expect(data).toBeDefined();
+    expect(data.traceId).toBeUndefined();
+    expect(data.spanId).toBeUndefined();
+    expect(data.parentSampled).toBeTruthy();
+  });
+
+  test('invalid', () => {
+    // trace id wrong length
+    expect(extractTraceparentData('a-bbbbbbbbbbbbbbbb-1')).toBeUndefined();
+
+    // parent span id wrong length
+    expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-b-1')).toBeUndefined();
+
+    // parent sampling decision wrong length
+    expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-11')).toBeUndefined();
+
+    // trace id invalid hex value
+    expect(extractTraceparentData('someStuffHereWhichIsNotAtAllHexy-bbbbbbbbbbbbbbbb-1')).toBeUndefined();
+
+    // parent span id invalid hex value
+    expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-alsoNotSuperHexy-1')).toBeUndefined();
+
+    // bogus sampling decision
+    expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-x')).toBeUndefined();
+  });
+});

packages/types/src/index.ts

@@ -23,7 +23,13 @@ export { Span, SpanContext } from './span';
 export { StackFrame } from './stackframe';
 export { Stacktrace } from './stacktrace';
 export { Status } from './status';
-export { CustomSamplingContext, SamplingContext, Transaction, TransactionContext } from './transaction';
+export {
+  CustomSamplingContext,
+  SamplingContext,
+  TraceparentData,
+  Transaction,
+  TransactionContext,
+} from './transaction';
 export { Thread } from './thread';
 export { Transport, TransportOptions, TransportClass } from './transport';
 export { User } from './user';

packages/types/src/transaction.ts

@@ -5,7 +5,11 @@ import { Span, SpanContext } from './span';
  * Interface holding Transaction-specific properties
  */
 export interface TransactionContext extends SpanContext {
+  /**
+   * Human-readable identifier for the transaction
+   */
   name: string;
+
   /**
    * If true, sets the end timestamp of the transaction to the highest timestamp of child spans, trimming
    * the duration of the transaction. This is useful to discard extra time in the transaction that is not
@@ -20,6 +24,11 @@ export interface TransactionContext extends SpanContext {
   parentSampled?: boolean;
 }
 
+/**
+ * Data pulled from a `sentry-trace` header
+ */
+export type TraceparentData = Pick<TransactionContext, 'traceId' | 'parentSpanId' | 'parentSampled'>;
+
 /**
  * Transaction "Class", inherits Span only has `setName`
  */