Skip to content

Commit

Permalink
refactor toTraceparent into extractTraceparentData
Browse files Browse the repository at this point in the history
  • Loading branch information
lobsterkatie committed Sep 7, 2020
1 parent 9824fd2 commit b707cfc
Show file tree
Hide file tree
Showing 10 changed files with 126 additions and 99 deletions.
18 changes: 4 additions & 14 deletions packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
Expand Down
17 changes: 6 additions & 11 deletions packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 */
Expand Down
2 changes: 1 addition & 1 deletion packages/tracing/src/index.bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};

Expand Down
2 changes: 1 addition & 1 deletion packages/tracing/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
34 changes: 0 additions & 34 deletions packages/tracing/src/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
35 changes: 34 additions & 1 deletion packages/tracing/src/utils.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
}
36 changes: 0 additions & 36 deletions packages/tracing/test/span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
64 changes: 64 additions & 0 deletions packages/tracing/test/utils.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
8 changes: 7 additions & 1 deletion packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
9 changes: 9 additions & 0 deletions packages/types/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`
*/
Expand Down

0 comments on commit b707cfc

Please sign in to comment.