Skip to content

feat(tracing): Handle incoming tracestate data, allow for third-party data #3275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 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 { extractTraceparentData, Span } from '@sentry/tracing';
import { extractSentrytraceData, extractTracestateData, Span } from '@sentry/tracing';
import { Event, ExtractedNodeRequestData, Transaction } from '@sentry/types';
import { forget, isPlainObject, isString, logger, normalize, stripUrlQueryAndFragment } from '@sentry/utils';
import * as cookie from 'cookie';
Expand Down Expand Up @@ -55,17 +55,21 @@ export function tracingHandler(): (
res: http.ServerResponse,
next: (error?: any) => void,
): void {
// 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'])) {
traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string);
// Extract data from trace headers
let traceparentData, tracestateData;
if (req.headers?.['sentry-trace']) {
traceparentData = extractSentrytraceData(req.headers['sentry-trace'] as string);
}
if (req.headers?.tracestate) {
tracestateData = extractTracestateData(req.headers.tracestate as string);
}

const transaction = startTransaction(
{
name: extractExpressTransactionName(req, { path: true, method: true }),
op: 'http.server',
...traceparentData,
...(tracestateData && { metadata: { tracestate: tracestateData } }),
},
{ request: extractRequestData(req) },
);
Expand Down
8 changes: 6 additions & 2 deletions packages/node/test/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,11 @@ describe('tracingHandler', () => {
expect(startTransaction).toHaveBeenCalled();
});

it("pulls parent's data from tracing header on the request", () => {
req.headers = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' };
it('pulls data from tracing headers on the request', () => {
req.headers = {
'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
tracestate: 'sentry=doGsaREgReaT',
};

sentryTracingMiddleware(req, res, next);

Expand All @@ -232,6 +235,7 @@ describe('tracingHandler', () => {
expect(transaction.traceId).toEqual('12312012123120121231201212312012');
expect(transaction.parentSpanId).toEqual('1121201211212012');
expect(transaction.sampled).toEqual(false);
expect(transaction.metadata?.tracestate).toEqual({ sentry: 'sentry=doGsaREgReaT' });
});

it('extracts request data for sampling context', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as sentryCore from '@sentry/core';
import { Hub } from '@sentry/hub';
import * as hubModule from '@sentry/hub';
import { addExtensionMethods, Span, TRACEPARENT_REGEXP, Transaction } from '@sentry/tracing';
import { addExtensionMethods, SENTRY_TRACE_REGEX, Span, Transaction } from '@sentry/tracing';
import * as http from 'http';
import * as nock from 'nock';

Expand Down Expand Up @@ -77,7 +77,7 @@ describe('tracing', () => {

expect(sentryTraceHeader).toBeDefined();
expect(tracestateHeader).toBeDefined();
expect(TRACEPARENT_REGEXP.test(sentryTraceHeader as string)).toBe(true);
expect(SENTRY_TRACE_REGEX.test(sentryTraceHeader as string)).toBe(true);
});

it("doesn't attach tracing headers to outgoing sentry requests", () => {
Expand Down
20 changes: 13 additions & 7 deletions packages/serverless/src/awslambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import {
withScope,
} from '@sentry/node';
import * as Sentry from '@sentry/node';
import { extractTraceparentData } from '@sentry/tracing';
import { extractSentrytraceData, extractTracestateData } from '@sentry/tracing';
import { Integration } from '@sentry/types';
import { isString, logger } from '@sentry/utils';
import { logger } from '@sentry/utils';
// NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil
// eslint-disable-next-line import/no-unresolved
import { Context, Handler } from 'aws-lambda';
Expand Down Expand Up @@ -192,7 +192,7 @@ export function wrapHandler<TEvent, TResult>(
};
let timeoutWarningTimer: NodeJS.Timeout;

// AWSLambda is like Express. It makes a distinction about handlers based on it's last argument
// AWSLambda is like Express. It makes a distinction about handlers based on its last argument
// async (event) => async handler
// async (event, context) => async handler
// (event, context, callback) => sync handler
Expand Down Expand Up @@ -243,16 +243,22 @@ export function wrapHandler<TEvent, TResult>(
}, timeoutWarningDelay);
}

// Applying `sentry-trace` to context
let traceparentData;
// Extract tracing data from headers
let traceparentData, tracestateData;
const eventWithHeaders = event as { headers?: { [key: string]: string } };
if (eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])) {
traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace'] as string);

if (eventWithHeaders.headers?.['sentry-trace']) {
traceparentData = extractSentrytraceData(eventWithHeaders.headers['sentry-trace'] as string);
}
if (eventWithHeaders.headers?.tracestate) {
tracestateData = extractTracestateData(eventWithHeaders.headers.tracestate as string);
}

const transaction = startTransaction({
name: context.functionName,
op: 'awslambda.handler',
...traceparentData,
...(tracestateData && { metadata: { tracestate: tracestateData } }),
});

const hub = getCurrentHub();
Expand Down
18 changes: 12 additions & 6 deletions packages/serverless/src/gcpfunction/http.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
import { extractTraceparentData } from '@sentry/tracing';
import { isString, logger, stripUrlQueryAndFragment } from '@sentry/utils';
import { extractSentrytraceData, extractTracestateData } from '@sentry/tracing';
import { logger, stripUrlQueryAndFragment } from '@sentry/utils';

import { domainify, getActiveDomain, proxyFunction } from './../utils';
import { HttpFunction, WrapperOptions } from './general';
Expand Down Expand Up @@ -49,16 +49,22 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
const reqMethod = (req.method || '').toUpperCase();
const reqUrl = stripUrlQueryAndFragment(req.originalUrl || req.url || '');

// Applying `sentry-trace` to context
let traceparentData;
// Extract tracing data from headers
let traceparentData, tracestateData;
const reqWithHeaders = req as { headers?: { [key: string]: string } };
if (reqWithHeaders.headers && isString(reqWithHeaders.headers['sentry-trace'])) {
traceparentData = extractTraceparentData(reqWithHeaders.headers['sentry-trace'] as string);

if (reqWithHeaders.headers?.['sentry-trace']) {
traceparentData = extractSentrytraceData(reqWithHeaders.headers['sentry-trace'] as string);
}
if (reqWithHeaders.headers?.tracestate) {
tracestateData = extractTracestateData(reqWithHeaders.headers.tracestate as string);
}

const transaction = startTransaction({
name: `${reqMethod} ${reqUrl}`,
op: 'gcp.function.http',
...traceparentData,
...(tracestateData && { metadata: { tracestate: tracestateData } }),
});

// getCurrentHub() is expected to use current active domain as a carrier
Expand Down
87 changes: 87 additions & 0 deletions packages/serverless/test/awslambda.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,35 @@ describe('AWSLambda', () => {
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
});

test('incoming trace headers are correctly parsed and used', async () => {
expect.assertions(1);

fakeEvent.headers = {
'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
tracestate: 'sentry=doGsaREgReaT,maisey=silly,charlie=goofy',
};

const handler: Handler = (_event, _context, callback) => {
expect(Sentry.startTransaction).toBeCalledWith(
expect.objectContaining({
traceId: '12312012123120121231201212312012',
parentSpanId: '1121201211212012',
parentSampled: false,
metadata: {
tracestate: {
sentry: 'sentry=doGsaREgReaT',
thirdparty: 'maisey=silly,charlie=goofy',
},
},
}),
);

callback(undefined, { its: 'fine' });
};
const wrappedHandler = wrapHandler(handler);
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
});

test('capture error', async () => {
expect.assertions(10);

Expand Down Expand Up @@ -278,6 +307,35 @@ describe('AWSLambda', () => {
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
});

test('incoming trace headers are correctly parsed and used', async () => {
expect.assertions(1);

fakeEvent.headers = {
'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
tracestate: 'sentry=doGsaREgReaT,maisey=silly,charlie=goofy',
};

const handler: Handler = async (_event, _context, callback) => {
expect(Sentry.startTransaction).toBeCalledWith(
expect.objectContaining({
traceId: '12312012123120121231201212312012',
parentSpanId: '1121201211212012',
parentSampled: false,
metadata: {
tracestate: {
sentry: 'sentry=doGsaREgReaT',
thirdparty: 'maisey=silly,charlie=goofy',
},
},
}),
);

callback(undefined, { its: 'fine' });
};
const wrappedHandler = wrapHandler(handler);
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
});

test('capture error', async () => {
expect.assertions(10);

Expand Down Expand Up @@ -328,6 +386,35 @@ describe('AWSLambda', () => {
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
});

test('incoming trace headers are correctly parsed and used', async () => {
expect.assertions(1);

fakeEvent.headers = {
'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
tracestate: 'sentry=doGsaREgReaT,maisey=silly,charlie=goofy',
};

const handler: Handler = async (_event, _context, callback) => {
expect(Sentry.startTransaction).toBeCalledWith(
expect.objectContaining({
traceId: '12312012123120121231201212312012',
parentSpanId: '1121201211212012',
parentSampled: false,
metadata: {
tracestate: {
sentry: 'sentry=doGsaREgReaT',
thirdparty: 'maisey=silly,charlie=goofy',
},
},
}),
);

callback(undefined, { its: 'fine' });
};
const wrappedHandler = wrapHandler(handler);
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
});

test('capture error', async () => {
expect.assertions(10);

Expand Down
29 changes: 29 additions & 0 deletions packages/serverless/test/gcpfunction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,35 @@ describe('GCPFunction', () => {
expect(Sentry.flush).toBeCalledWith(2000);
});

test('incoming trace headers are correctly parsed and used', async () => {
expect.assertions(1);

const handler: HttpFunction = (_req, res) => {
res.statusCode = 200;
res.end();
};
const wrappedHandler = wrapHttpFunction(handler);
const traceHeaders = {
'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
tracestate: 'sentry=doGsaREgReaT,maisey=silly,charlie=goofy',
};
await handleHttp(wrappedHandler, traceHeaders);

expect(Sentry.startTransaction).toBeCalledWith(
expect.objectContaining({
traceId: '12312012123120121231201212312012',
parentSpanId: '1121201211212012',
parentSampled: false,
metadata: {
tracestate: {
sentry: 'sentry=doGsaREgReaT',
thirdparty: 'maisey=silly,charlie=goofy',
},
},
}),
);
});

test('capture error', async () => {
expect.assertions(5);

Expand Down
24 changes: 16 additions & 8 deletions packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { getGlobalObject, logger } from '@sentry/utils';
import { startIdleTransaction } from '../hubextensions';
import { DEFAULT_IDLE_TIMEOUT, IdleTransaction } from '../idletransaction';
import { SpanStatus } from '../spanstatus';
import { extractTraceparentData, secToMs } from '../utils';
import { extractSentrytraceData, extractTracestateData, secToMs } from '../utils';
import { registerBackgroundTabDetection } from './backgroundtab';
import { MetricsInstrumentation } from './metrics';
import {
Expand Down Expand Up @@ -191,7 +191,7 @@ export class BrowserTracing implements Integration {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { beforeNavigate, idleTimeout, maxTransactionDuration } = this.options;

const parentContextFromHeader = context.op === 'pageload' ? getHeaderContext() : undefined;
const parentContextFromHeader = context.op === 'pageload' ? extractTraceDataFromMetaTags() : undefined;

const expandedContext = {
...context,
Expand Down Expand Up @@ -230,14 +230,22 @@ export class BrowserTracing implements Integration {
}

/**
* Gets transaction context from a sentry-trace meta.
* Gets transaction context data from `sentry-trace` and `tracestate` <meta> tags.
*
* @returns Transaction context data from the header or undefined if there's no header or the header is malformed
* @returns Transaction context data or undefined neither tag exists or has valid data
*/
export function getHeaderContext(): Partial<TransactionContext> | undefined {
const header = getMetaContent('sentry-trace');
if (header) {
return extractTraceparentData(header);
export function extractTraceDataFromMetaTags(): Partial<TransactionContext> | undefined {
const sentrytraceValue = getMetaContent('sentry-trace');
const tracestateValue = getMetaContent('tracestate');

const sentrytraceData = sentrytraceValue ? extractSentrytraceData(sentrytraceValue) : undefined;
const tracestateData = tracestateValue ? extractTracestateData(tracestateValue) : undefined;

if (sentrytraceData || tracestateData?.sentry || tracestateData?.thirdparty) {
Copy link
Contributor

@kamilogorek kamilogorek Feb 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tracestateData?.thirdparty is useless here, as it will never be reached due to the first sentrytraceData check already shorting it with its truthiness.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But sentrytraceData isn't guaranteed to be truthy - if the sentry-trace <meta> tag isn't there, or if the value is somehow malformed, you could have tracestate data without having sentry-trace data. Similarly, if we have a tracesatate value in which there's no sentry portion (or the sentry portion is malformed), then we could have third-party tracestate data without having sentry tracestate data. So I think we need all three checks, since the existence of each value is independent of the existence of the others.

return {
...sentrytraceData,
...(tracestateData && { metadata: { tracestate: tracestateData } }),
};
}

return undefined;
Expand Down
5 changes: 3 additions & 2 deletions packages/tracing/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ addExtensionMethods();
export { addExtensionMethods };

export {
extractTraceparentData,
extractSentrytraceData,
extractTracestateData,
getActiveTransaction,
hasTracingEnabled,
SENTRY_TRACE_REGEX,
stripUrlQueryAndFragment,
TRACEPARENT_REGEXP,
} from './utils';
Loading