From 13c9cc1d7db82d24d397922a5f636abac5685ebc Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 19 May 2022 15:21:26 +0200 Subject: [PATCH 01/21] feat(tracing): Add empty baggage header propagation --- packages/nextjs/src/utils/instrumentServer.ts | 13 +++- packages/nextjs/src/utils/withSentry.ts | 12 +++- packages/node/src/handlers.ts | 9 +-- packages/serverless/src/awslambda.ts | 9 ++- packages/serverless/src/gcpfunction/http.ts | 9 ++- packages/tracing/src/browser/request.ts | 62 ++++++++++++++----- packages/tracing/src/span.ts | 9 ++- packages/types/src/span.ts | 4 ++ packages/types/src/transaction.ts | 9 +-- 9 files changed, 106 insertions(+), 30 deletions(-) diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index cba8e0ab9423..bf4adc9ec8b3 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/packages/nextjs/src/utils/instrumentServer.ts @@ -8,7 +8,14 @@ import { startTransaction, } from '@sentry/node'; import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; -import { addExceptionMechanism, fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; +import { + addExceptionMechanism, + fill, + isString, + logger, + parseBaggageString, + stripUrlQueryAndFragment, +} from '@sentry/utils'; import * as domain from 'domain'; import * as http from 'http'; import { default as createNextServer } from 'next'; @@ -252,6 +259,9 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { IS_DEBUG_BUILD && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } + const baggage = + nextReq.headers && isString(nextReq.headers.baggage) && parseBaggageString(nextReq.headers.baggage); + // pull off query string, if any const reqPath = stripUrlQueryAndFragment(nextReq.url); @@ -265,6 +275,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { op: 'http.server', metadata: { requestPath: reqPath }, ...traceparentData, + ...(baggage && { metadata: { baggage: baggage } }), }, // Extra context passed to the `tracesSampler` (Note: We're combining `nextReq` and `req` this way in order // to not break people's `tracesSampler` functions, even though the format of `nextReq` has changed (see diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index 00e76ec40b69..742d9f4c9280 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -1,7 +1,14 @@ import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; import { extractTraceparentData, hasTracingEnabled } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; -import { addExceptionMechanism, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils'; +import { + addExceptionMechanism, + isString, + logger, + objectify, + parseBaggageString, + stripUrlQueryAndFragment, +} from '@sentry/utils'; import * as domain from 'domain'; import { NextApiHandler, NextApiRequest, NextApiResponse } from 'next'; @@ -48,6 +55,8 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler = IS_DEBUG_BUILD && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } + const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage); + const url = `${req.url}`; // pull off query string, if any let reqPath = stripUrlQueryAndFragment(url); @@ -66,6 +75,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler = name: `${reqMethod}${reqPath}`, op: 'http.server', ...traceparentData, + ...(baggage && { metadata: { baggage: baggage } }), }, // extra context passed to the `tracesSampler` { request: req }, diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 451ef14e9223..27dbeb3fc9cf 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -8,6 +8,7 @@ import { isString, logger, normalize, + parseBaggageString, stripUrlQueryAndFragment, } from '@sentry/utils'; import * as cookie from 'cookie'; @@ -61,16 +62,16 @@ export function tracingHandler(): ( 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']); - } + const traceparentData = + req.headers && isString(req.headers['sentry-trace']) && extractTraceparentData(req.headers['sentry-trace']); + const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage); const transaction = startTransaction( { name: extractExpressTransactionName(req, { path: true, method: true }), op: 'http.server', ...traceparentData, + ...(baggage && { metadata: { baggage: baggage } }), }, // extra context passed to the tracesSampler { request: extractRequestData(req) }, diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 5e00e3419153..a2825b1e1e2e 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -11,7 +11,7 @@ import { } from '@sentry/node'; import { extractTraceparentData } from '@sentry/tracing'; import { Integration } from '@sentry/types'; -import { extensionRelayDSN, isString, logger } from '@sentry/utils'; +import { extensionRelayDSN, isString, logger, parseBaggageString } 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'; @@ -288,10 +288,17 @@ export function wrapHandler( if (eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])) { traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace']); } + + const baggage = + eventWithHeaders.headers && + isString(eventWithHeaders.headers.baggage) && + parseBaggageString(eventWithHeaders.headers.baggage); + const transaction = startTransaction({ name: context.functionName, op: 'awslambda.handler', ...traceparentData, + ...(baggage && { metadata: { baggage: baggage } }), }); const hub = getCurrentHub(); diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index a1eec1e48736..26c32b4a21cb 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -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 { isString, logger, parseBaggageString, stripUrlQueryAndFragment } from '@sentry/utils'; import { IS_DEBUG_BUILD } from '../flags'; import { domainify, getActiveDomain, proxyFunction } from './../utils'; @@ -56,10 +56,17 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial + | Array<[string, string]> + // the below is not preicsely the Header type used in Request, but it'll pass duck-typing + | { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; + append: (key: string, value: string) => void; + }; + export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions = { traceFetch: true, traceXHR: true, @@ -179,25 +190,40 @@ export function fetchCallback( const request = (handlerData.args[0] = handlerData.args[0] as string | Request); // eslint-disable-next-line @typescript-eslint/no-explicit-any const options = (handlerData.args[1] = (handlerData.args[1] as { [key: string]: any }) || {}); - let headers = options.headers; - if (isInstanceOf(request, Request)) { - headers = (request as Request).headers; - } - if (headers) { + options.headers = addTracingHeaders(request, span, options); + } +} + +function addTracingHeaders( + request: string | Request, + span: Span, + options: { [key: string]: any }, +): PolymorphicRequestHeaders { + let headers = options.headers; + + if (isInstanceOf(request, Request)) { + headers = (request as Request).headers; + } + // TODO do we have to merge third-party-created added baggage headers with the ones we got form incoming req? + // const baggageContent = (request as Request).headers.get(BAGGAGE_HEADER_NAME) || ''; + const baggageContent = serializeBaggageFromSpan(span); + + if (headers) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (typeof headers.append === 'function') { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (typeof headers.append === 'function') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - headers.append('sentry-trace', span.toTraceparent()); - } else if (Array.isArray(headers)) { - headers = [...headers, ['sentry-trace', span.toTraceparent()]]; - } else { - headers = { ...headers, 'sentry-trace': span.toTraceparent() }; - } + headers.append('sentry-trace', span.toTraceparent()); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + headers.append(BAGGAGE_HEADER_NAME, baggageContent); + } else if (Array.isArray(headers)) { + headers = [...headers, ['sentry-trace', span.toTraceparent()], [BAGGAGE_HEADER_NAME, baggageContent]]; } else { - headers = { 'sentry-trace': span.toTraceparent() }; + headers = { ...headers, 'sentry-trace': span.toTraceparent(), baggage: baggageContent }; } - options.headers = headers; + } else { + headers = { 'sentry-trace': span.toTraceparent(), baggage: baggageContent }; } + return headers; } /** @@ -254,9 +280,15 @@ export function xhrCallback( if (handlerData.xhr.setRequestHeader) { try { handlerData.xhr.setRequestHeader('sentry-trace', span.toTraceparent()); + handlerData.xhr.setRequestHeader(BAGGAGE_HEADER_NAME, serializeBaggageFromSpan(span)); } catch (_) { // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED. } } } } + +function serializeBaggageFromSpan(span: Span): string { + const baggage = span.getBaggage(); + return (baggage && serializeBaggage(baggage)) || ''; +} diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts index baece680f52c..68cba79eb7cc 100644 --- a/packages/tracing/src/span.ts +++ b/packages/tracing/src/span.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types'; +import { Baggage, Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types'; import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils'; /** @@ -298,6 +298,13 @@ export class Span implements SpanInterface { }); } + /** + * @inheritdoc + */ + public getBaggage(): Baggage | undefined { + return this.transaction && this.transaction.metadata.baggage; + } + /** * @inheritDoc */ diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index d26887a93f2a..120d754cc329 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -1,3 +1,4 @@ +import { Baggage } from './baggage'; import { Primitive } from './misc'; import { Transaction } from './transaction'; @@ -174,4 +175,7 @@ export interface Span extends SpanContext { timestamp?: number; trace_id: string; }; + + /** return the baggage for dynamic sampling and trace propagation */ + getBaggage(): Baggage | undefined; } diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index 95f8b447e19a..b87fc2b151cf 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -1,6 +1,6 @@ +import { Baggage } from './baggage'; import { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc'; import { Span, SpanContext } from './span'; - /** * Interface holding Transaction-specific properties */ @@ -131,11 +131,8 @@ export type TransactionSamplingMethod = 'explicitly_set' | 'client_sampler' | 'c export interface TransactionMetadata { transactionSampling?: { rate?: number; method: TransactionSamplingMethod }; - /** The two halves (sentry and third-party) of a transaction's tracestate header, used for dynamic sampling */ - tracestate?: { - sentry?: string; - thirdparty?: string; - }; + /** The baggage object of a transaction's baggage header, used for dynamic sampling */ + baggage?: Baggage; /** For transactions tracing server-side request handling, the path of the request being tracked. */ requestPath?: string; From c0cdd2a450af1f3664b640d314c238409054cf0d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 19 May 2022 15:39:40 +0200 Subject: [PATCH 02/21] add missing baggage.ts module (moved baggage types to @sentry/types) --- packages/tracing/src/browser/request.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index fef47ef6a553..d5bbd984e64c 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -1,5 +1,4 @@ -import { BAGGAGE_HEADER_NAME, serializeBaggage } from '@sentry/utils'; -import { addInstrumentationHandler, isInstanceOf, isMatchingPattern } from '@sentry/utils'; +import { addInstrumentationHandler, BAGGAGE_HEADER_NAME, isInstanceOf, isMatchingPattern,serializeBaggage } from '@sentry/utils'; import { Span } from '../span'; import { getActiveTransaction, hasTracingEnabled } from '../utils'; From c374f1aa40c2d55186826e9ba0402b671300039f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 19 May 2022 15:48:25 +0200 Subject: [PATCH 03/21] fix linter errors --- packages/tracing/src/browser/request.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index d5bbd984e64c..c187fc62287a 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -1,4 +1,10 @@ -import { addInstrumentationHandler, BAGGAGE_HEADER_NAME, isInstanceOf, isMatchingPattern,serializeBaggage } from '@sentry/utils'; +import { + addInstrumentationHandler, + BAGGAGE_HEADER_NAME, + isInstanceOf, + isMatchingPattern, + serializeBaggage, +} from '@sentry/utils'; import { Span } from '../span'; import { getActiveTransaction, hasTracingEnabled } from '../utils'; From babafa1bd78764448da57b088c7b16aee963bb80 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 19 May 2022 23:14:13 +0200 Subject: [PATCH 04/21] merge incoming baggage header with possibly modified 3rd party headers --- packages/tracing/src/browser/request.ts | 66 ++++++++++++++++++++----- packages/utils/src/baggage.ts | 8 +++ 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index c187fc62287a..b117a8808428 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -1,8 +1,13 @@ +/* eslint-disable max-lines */ +import { Baggage } from '@sentry/types'; import { addInstrumentationHandler, BAGGAGE_HEADER_NAME, + createBaggage, + getThirdPartyBaggage, isInstanceOf, isMatchingPattern, + parseBaggageString, serializeBaggage, } from '@sentry/utils'; @@ -76,6 +81,7 @@ export interface XHRData { }; __sentry_xhr_span_id__?: string; setRequestHeader?: (key: string, val: string) => void; + getRequestHeader?: (key: string) => string; __sentry_own_request__?: boolean; }; startTimestamp: number; @@ -90,6 +96,7 @@ type PolymorphicRequestHeaders = // eslint-disable-next-line @typescript-eslint/no-explicit-any [key: string]: any; append: (key: string, value: string) => void; + get: (key: string) => string; }; export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions = { @@ -209,9 +216,7 @@ function addTracingHeaders( if (isInstanceOf(request, Request)) { headers = (request as Request).headers; } - // TODO do we have to merge third-party-created added baggage headers with the ones we got form incoming req? - // const baggageContent = (request as Request).headers.get(BAGGAGE_HEADER_NAME) || ''; - const baggageContent = serializeBaggageFromSpan(span); + const incomingBaggage = span.getBaggage(); if (headers) { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access @@ -219,14 +224,24 @@ function addTracingHeaders( // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access headers.append('sentry-trace', span.toTraceparent()); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - headers.append(BAGGAGE_HEADER_NAME, baggageContent); + headers.append(BAGGAGE_HEADER_NAME, mergeAndSerializeBaggage(incomingBaggage, headers.get(BAGGAGE_HEADER_NAME))); } else if (Array.isArray(headers)) { - headers = [...headers, ['sentry-trace', span.toTraceparent()], [BAGGAGE_HEADER_NAME, baggageContent]]; + const [, headerBaggageString] = headers.find(([key, _]) => key === BAGGAGE_HEADER_NAME); + headers = [ + ...headers, + ['sentry-trace', span.toTraceparent()], + [BAGGAGE_HEADER_NAME, mergeAndSerializeBaggage(incomingBaggage, headerBaggageString)], + ]; } else { - headers = { ...headers, 'sentry-trace': span.toTraceparent(), baggage: baggageContent }; + headers = { + ...headers, + 'sentry-trace': span.toTraceparent(), + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + baggage: mergeAndSerializeBaggage(incomingBaggage, headers.baggage), + }; } } else { - headers = { 'sentry-trace': span.toTraceparent(), baggage: baggageContent }; + headers = { 'sentry-trace': span.toTraceparent(), baggage: mergeAndSerializeBaggage(incomingBaggage) }; } return headers; } @@ -285,7 +300,14 @@ export function xhrCallback( if (handlerData.xhr.setRequestHeader) { try { handlerData.xhr.setRequestHeader('sentry-trace', span.toTraceparent()); - handlerData.xhr.setRequestHeader(BAGGAGE_HEADER_NAME, serializeBaggageFromSpan(span)); + + const headerBaggageString = + handlerData.xhr.getRequestHeader && handlerData.xhr.getRequestHeader(BAGGAGE_HEADER_NAME); + + handlerData.xhr.setRequestHeader( + BAGGAGE_HEADER_NAME, + mergeAndSerializeBaggage(span.getBaggage(), headerBaggageString), + ); } catch (_) { // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED. } @@ -293,7 +315,29 @@ export function xhrCallback( } } -function serializeBaggageFromSpan(span: Span): string { - const baggage = span.getBaggage(); - return (baggage && serializeBaggage(baggage)) || ''; +/** + * Merges the baggage header we saved from the incoming request (or meta tag) with + * a possibly created or modified baggage header by a third party that's been added + * to the outgoing request header. + * + * In case @param headerBaggage exists, we can safely add the the 3rd party part of @param headerBaggage + * with our @param incomingBaggage. This is possible because if we modified anything beforehand, + * it would only affect parts of the sentry baggage (@see Baggage interface). + * + * @param incomingBaggage the baggage header of the incoming request that might contain sentry entries + * @param headerBaggageString possibly existing baggage header string added from a third party to request headers + * + * @return a merged and serialized baggage string to be propagated with the outgoing request + */ +function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggageString?: string): string { + if (!incomingBaggage && !headerBaggageString) { + return ''; + } + + const headerBaggage = (headerBaggageString && parseBaggageString(headerBaggageString)) || undefined; + const thirdPartyHeaderBaggage = headerBaggage && getThirdPartyBaggage(headerBaggage); + + const finalBaggage = createBaggage((incomingBaggage && incomingBaggage[0]) || {}, thirdPartyHeaderBaggage || ''); + + return serializeBaggage(finalBaggage); } diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index ddf3b9b24afa..14a7ac7cb5cb 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -41,6 +41,14 @@ export function getSentryBaggageItems(baggage: Baggage): BaggageObj { return baggage[0]; } +/** + * Returns 3rd party baggage string of @param baggage + * @param baggage + */ +export function getThirdPartyBaggage(baggage: Baggage): string { + return baggage[1]; +} + /** Serialize a baggage object */ export function serializeBaggage(baggage: Baggage): string { return Object.keys(baggage[0]).reduce((prev, key: keyof BaggageObj) => { From 384b3bd0661e9ffda956340c077218ce403be206 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 09:37:57 +0200 Subject: [PATCH 05/21] add obtaining baggage from html meta tags (BrowserTracing) --- .../tracing/src/browser/browsertracing.ts | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 780edf9052d4..ff85fdcbea79 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -1,6 +1,6 @@ import { Hub } from '@sentry/hub'; import { EventProcessor, Integration, Transaction, TransactionContext } from '@sentry/types'; -import { getGlobalObject, logger } from '@sentry/utils'; +import { getGlobalObject, logger, parseBaggageString } from '@sentry/utils'; import { IS_DEBUG_BUILD } from '../flags'; import { startIdleTransaction } from '../hubextensions'; @@ -204,7 +204,7 @@ export class BrowserTracing implements Integration { // eslint-disable-next-line @typescript-eslint/unbound-method const { beforeNavigate, idleTimeout, finalTimeout } = this.options; - const parentContextFromHeader = context.op === 'pageload' ? getHeaderContext() : undefined; + const parentContextFromHeader = context.op === 'pageload' ? extractTraceDataFromMetaTags() : undefined; const expandedContext = { ...context, @@ -256,6 +256,28 @@ export function getHeaderContext(): Partial | undefined { return undefined; } +/** + * Gets transaction context data from `sentry-trace` and `baggage` tags. + * @returns Transaction context data or undefined neither tag exists or has valid data + */ +export function extractTraceDataFromMetaTags(): Partial | undefined { + const sentrytraceValue = getMetaContent('sentry-trace'); + const baggageValue = getMetaContent('baggage'); + + const sentrytraceData = sentrytraceValue ? extractTraceparentData(sentrytraceValue) : undefined; + const baggage = baggageValue ? parseBaggageString(baggageValue) : undefined; + + // TODO more extensive checks for baggage validity/emptyness? + if (sentrytraceData || baggage) { + return { + ...sentrytraceData, + ...(baggage && { metadata: { baggage } }), + }; + } + + return undefined; +} + /** Returns the value of a meta tag */ export function getMetaContent(metaName: string): string | null { const el = getGlobalObject().document.querySelector(`meta[name=${metaName}]`); From be70865ec6e070305d9fd8b1fec79d04ae98ae2d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 10:01:43 +0200 Subject: [PATCH 06/21] add outgoing request baggage propagation to node Http integration --- packages/node/src/integrations/http.ts | 11 +++++++-- packages/tracing/src/browser/request.ts | 33 +------------------------ packages/utils/src/baggage.ts | 27 ++++++++++++++++++++ 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 19a4e6ba1423..074c81ffe842 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -1,6 +1,6 @@ import { getCurrentHub } from '@sentry/core'; import { Integration, Span } from '@sentry/types'; -import { fill, logger, parseSemver } from '@sentry/utils'; +import { fill, logger, mergeAndSerializeBaggage, parseSemver } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; @@ -123,7 +123,14 @@ function _createWrappedRequestMethodFactory( logger.log( `[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to ${requestUrl}: `, ); - requestOptions.headers = { ...requestOptions.headers, 'sentry-trace': sentryTraceHeader }; + + const headerBaggageString = requestOptions.headers && (requestOptions.headers.baggage as string); + + requestOptions.headers = { + ...requestOptions.headers, + 'sentry-trace': sentryTraceHeader, + baggage: mergeAndSerializeBaggage(span.getBaggage(), headerBaggageString), + }; } } diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index b117a8808428..b45775edd05f 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -1,14 +1,10 @@ /* eslint-disable max-lines */ -import { Baggage } from '@sentry/types'; import { addInstrumentationHandler, BAGGAGE_HEADER_NAME, - createBaggage, - getThirdPartyBaggage, isInstanceOf, isMatchingPattern, - parseBaggageString, - serializeBaggage, + mergeAndSerializeBaggage, } from '@sentry/utils'; import { Span } from '../span'; @@ -314,30 +310,3 @@ export function xhrCallback( } } } - -/** - * Merges the baggage header we saved from the incoming request (or meta tag) with - * a possibly created or modified baggage header by a third party that's been added - * to the outgoing request header. - * - * In case @param headerBaggage exists, we can safely add the the 3rd party part of @param headerBaggage - * with our @param incomingBaggage. This is possible because if we modified anything beforehand, - * it would only affect parts of the sentry baggage (@see Baggage interface). - * - * @param incomingBaggage the baggage header of the incoming request that might contain sentry entries - * @param headerBaggageString possibly existing baggage header string added from a third party to request headers - * - * @return a merged and serialized baggage string to be propagated with the outgoing request - */ -function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggageString?: string): string { - if (!incomingBaggage && !headerBaggageString) { - return ''; - } - - const headerBaggage = (headerBaggageString && parseBaggageString(headerBaggageString)) || undefined; - const thirdPartyHeaderBaggage = headerBaggage && getThirdPartyBaggage(headerBaggage); - - const finalBaggage = createBaggage((incomingBaggage && incomingBaggage[0]) || {}, thirdPartyHeaderBaggage || ''); - - return serializeBaggage(finalBaggage); -} diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index 14a7ac7cb5cb..5824d47d2caa 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -86,3 +86,30 @@ export function parseBaggageString(inputBaggageString: string): Baggage { [{}, ''], ); } + +/** + * Merges the baggage header we saved from the incoming request (or meta tag) with + * a possibly created or modified baggage header by a third party that's been added + * to the outgoing request header. + * + * In case @param headerBaggage exists, we can safely add the the 3rd party part of @param headerBaggage + * with our @param incomingBaggage. This is possible because if we modified anything beforehand, + * it would only affect parts of the sentry baggage (@see Baggage interface). + * + * @param incomingBaggage the baggage header of the incoming request that might contain sentry entries + * @param headerBaggageString possibly existing baggage header string added from a third party to request headers + * + * @return a merged and serialized baggage string to be propagated with the outgoing request + */ +export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggageString?: string): string { + if (!incomingBaggage && !headerBaggageString) { + return ''; + } + + const headerBaggage = (headerBaggageString && parseBaggageString(headerBaggageString)) || undefined; + const thirdPartyHeaderBaggage = headerBaggage && getThirdPartyBaggage(headerBaggage); + + const finalBaggage = createBaggage((incomingBaggage && incomingBaggage[0]) || {}, thirdPartyHeaderBaggage || ''); + + return serializeBaggage(finalBaggage); +} From 69234630417653a8a68c22d0f4089d0a0447415c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 10:29:18 +0200 Subject: [PATCH 07/21] add node http integration unit tests --- packages/node/test/integrations/http.test.ts | 37 ++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index ff0af8b65acf..50d26e92452a 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -86,6 +86,43 @@ describe('tracing', () => { expect(sentryTraceHeader).not.toBeDefined(); }); + + it('attaches the baggage header to outgoing non-sentry requests', async () => { + nock('http://dogs.are.great').get('/').reply(200); + + createTransactionOnScope(); + + const request = http.get('http://dogs.are.great/'); + const baggageHeader = request.getHeader('baggage') as string; + + expect(baggageHeader).toBeDefined(); + // this might change once we actually add our baggage data to the header + expect(baggageHeader).toEqual(''); + }); + + it('propagates 3rd party baggage header data to outgoing non-sentry requests', async () => { + nock('http://dogs.are.great').get('/').reply(200); + + createTransactionOnScope(); + + const request = http.get({ host: 'http://dogs.are.great/', headers: { baggage: 'dog=great' } }); + const baggageHeader = request.getHeader('baggage') as string; + + expect(baggageHeader).toBeDefined(); + // this might change once we actually add our baggage data to the header + expect(baggageHeader).toEqual('dog=great'); + }); + + it("doesn't attach the sentry-trace header to outgoing sentry requests", () => { + nock('http://squirrelchasers.ingest.sentry.io').get('/api/12312012/store/').reply(200); + + createTransactionOnScope(); + + const request = http.get('http://squirrelchasers.ingest.sentry.io/api/12312012/store/'); + const baggage = request.getHeader('baggage'); + + expect(baggage).not.toBeDefined(); + }); }); describe('default protocols', () => { From 3643fabaccb53eddc5cbed9e0c4ba0b477d81597 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 10:39:48 +0200 Subject: [PATCH 08/21] add node handler unit tests --- packages/node/test/handlers.test.ts | 37 ++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 57f6528139f9..8cee24f678fd 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -2,7 +2,7 @@ import * as sentryCore from '@sentry/core'; import * as sentryHub from '@sentry/hub'; import { Hub } from '@sentry/hub'; import { Transaction } from '@sentry/tracing'; -import { Runtime } from '@sentry/types'; +import { Baggage, Runtime } from '@sentry/types'; import { SentryError } from '@sentry/utils'; import * as http from 'http'; import * as net from 'net'; @@ -368,6 +368,41 @@ describe('tracingHandler', () => { expect(transaction.traceId).toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toEqual('1121201211212012'); expect(transaction.sampled).toEqual(false); + expect(transaction.metadata?.baggage).toBeUndefined(); + }); + + it("pulls parent's data from tracing and baggage headers on the request", () => { + req.headers = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + baggage: 'sentry-version=1.0,sentry-environment=production', + }; + + sentryTracingMiddleware(req, res, next); + + const transaction = (res as any).__sentry_transaction; + + // since we have no tracesSampler defined, the default behavior (inherit if possible) applies + expect(transaction.traceId).toEqual('12312012123120121231201212312012'); + expect(transaction.parentSpanId).toEqual('1121201211212012'); + expect(transaction.sampled).toEqual(false); + expect(transaction.metadata?.baggage).toBeDefined(); + expect(transaction.metadata?.baggage).toEqual([{ version: '1.0', environment: 'production' }, ''] as Baggage); + }); + + it("pulls parent's baggage (sentry + third party entries) headers on the request", () => { + req.headers = { + baggage: 'sentry-version=1.0,sentry-environment=production,dogs=great,cats=boring', + }; + + sentryTracingMiddleware(req, res, next); + + const transaction = (res as any).__sentry_transaction; + + expect(transaction.metadata?.baggage).toBeDefined(); + expect(transaction.metadata?.baggage).toEqual([ + { version: '1.0', environment: 'production' }, + 'dogs=great,cats=boring', + ] as Baggage); }); it('extracts request data for sampling context', () => { From b8011e20522fbaf790ff521d47a23ba04febbf0a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 11:22:39 +0200 Subject: [PATCH 09/21] fix mergeAndSerializeBaggage bug --- packages/utils/src/baggage.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index 5824d47d2caa..b37acdc1eefb 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -109,7 +109,9 @@ export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggag const headerBaggage = (headerBaggageString && parseBaggageString(headerBaggageString)) || undefined; const thirdPartyHeaderBaggage = headerBaggage && getThirdPartyBaggage(headerBaggage); - const finalBaggage = createBaggage((incomingBaggage && incomingBaggage[0]) || {}, thirdPartyHeaderBaggage || ''); - + const finalBaggage = createBaggage( + (incomingBaggage && incomingBaggage[0]) || {}, + thirdPartyHeaderBaggage || (incomingBaggage && incomingBaggage[1]) || '', + ); return serializeBaggage(finalBaggage); } From ea67e5c2dbffe262ecfe3c98305f21feccef9125 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 11:30:25 +0200 Subject: [PATCH 10/21] add node integration tests --- .../baggage-header-assign/test.ts | 52 +++++++++++++++++++ .../sentry-trace/baggage-header-out/test.ts | 19 +++++++ .../suites/express/sentry-trace/server.ts | 2 +- 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts create mode 100644 packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts new file mode 100644 index 000000000000..81dddb7c1f97 --- /dev/null +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts @@ -0,0 +1,52 @@ +import * as path from 'path'; + +import { getAPIResponse, runServer } from '../../../../utils/index'; +import { TestAPIResponse } from '../server'; + +test('Should assign `baggage` header which contains 3rd party trace baggage data of an outgoing request.', async () => { + const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); + + const response = (await getAPIResponse(new URL(`${url}/express`), { + baggage: 'foo=bar,bar=baz', + })) as TestAPIResponse; + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + baggage: expect.stringContaining('foo=bar,bar=baz'), + }, + }); +}); + +test('Should assign `baggage` header which contains sentry trace baggage data of an outgoing request.', async () => { + const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); + + const response = (await getAPIResponse(new URL(`${url}/express`), { + baggage: 'sentry-version=1.0.0,sentry-environment=production', + })) as TestAPIResponse; + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + baggage: expect.stringContaining('sentry-version=1.0.0,sentry-environment=production'), + }, + }); +}); + +test('Should assign `baggage` header which contains sentry and 3rd party trace baggage data of an outgoing request.', async () => { + const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); + + const response = (await getAPIResponse(new URL(`${url}/express`), { + baggage: 'sentry-version=1.0.0,sentry-environment=production,dogs=great', + })) as TestAPIResponse; + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + baggage: expect.stringContaining('dogs=great,sentry-version=1.0.0,sentry-environment=production'), + }, + }); +}); diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts new file mode 100644 index 000000000000..70b6cc19edee --- /dev/null +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts @@ -0,0 +1,19 @@ +import * as path from 'path'; + +import { getAPIResponse, runServer } from '../../../../utils/index'; +import { TestAPIResponse } from '../server'; + +test('should attach a `baggage` header to an outgoing request.', async () => { + const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); + + const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse; + + expect(response).toBeDefined(); + expect(response).toMatchObject({ + test_data: { + host: 'somewhere.not.sentry', + // TODO this is currently still empty but eventually it should contain sentry data + baggage: expect.stringMatching(''), + }, + }); +}); diff --git a/packages/node-integration-tests/suites/express/sentry-trace/server.ts b/packages/node-integration-tests/suites/express/sentry-trace/server.ts index e93b50f18cd6..632d9b8338fb 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/server.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/server.ts @@ -6,7 +6,7 @@ import http from 'http'; const app = express(); -export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string } }; +export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } }; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', From 8cf755b2abe8d55230f702586558b213138bc49c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 11:44:50 +0200 Subject: [PATCH 11/21] add (currently skipped) meta tag integration test --- .../tracing/browsertracing/meta/template.html | 1 + .../suites/tracing/browsertracing/meta/test.ts | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/template.html b/packages/integration-tests/suites/tracing/browsertracing/meta/template.html index 0afff8864522..60c6c062c7f3 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/meta/template.html +++ b/packages/integration-tests/suites/tracing/browsertracing/meta/template.html @@ -2,5 +2,6 @@ + diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts index 263fd9257186..f4cae5743987 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts +++ b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts @@ -1,8 +1,8 @@ import { expect } from '@playwright/test'; -import { Event } from '@sentry/types'; +import { Event, EventEnvelopeHeaders } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; +import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; sentryTest( 'should create a pageload transaction based on `sentry-trace` ', @@ -21,6 +21,20 @@ sentryTest( }, ); +// TODO this we can't really test until we actually propagate sentry- entries in baggage +// skipping for now but this must be adjusted later on +sentryTest.skip( + 'should pick up `baggage` tag and propagate the content in transaction', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); + + expect(envHeader.baggage).toBeDefined(); + expect(envHeader.baggage).toEqual('{version:2.1.12}'); + }, +); + sentryTest( "should create a navigation that's not influenced by `sentry-trace` ", async ({ getLocalTestPath, page }) => { From 85663beddfa7554f00322e851c9a1d22292f5e3e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 14:42:23 +0200 Subject: [PATCH 12/21] add browsertracing unit tests remove no longer needed function --- .../tracing/src/browser/browsertracing.ts | 14 ---- .../test/browser/browsertracing.test.ts | 67 ++++++++++++++++--- 2 files changed, 56 insertions(+), 25 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index ff85fdcbea79..aea47f466091 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -242,20 +242,6 @@ 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 - */ -export function getHeaderContext(): Partial | undefined { - const header = getMetaContent('sentry-trace'); - if (header) { - return extractTraceparentData(header); - } - - return undefined; -} - /** * Gets transaction context data from `sentry-trace` and `baggage` tags. * @returns Transaction context data or undefined neither tag exists or has valid data diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 4bfc093fbe2b..2d6a4ef4c44e 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -1,12 +1,13 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, makeMain } from '@sentry/hub'; +import { BaggageObj } from '@sentry/types'; import { getGlobalObject, InstrumentHandlerCallback, InstrumentHandlerType } from '@sentry/utils'; import { JSDOM } from 'jsdom'; import { BrowserTracing, BrowserTracingOptions, - getHeaderContext, + extractTraceDataFromMetaTags, getMetaContent, } from '../../src/browser/browsertracing'; import { MetricsInstrumentation } from '../../src/browser/metrics'; @@ -215,7 +216,8 @@ describe('BrowserTracing', () => { it('sets transaction context from sentry-trace header', () => { const name = 'sentry-trace'; const content = '126de09502ae4e0fb26c6967190756a4-b6e54397b12a2a0f-1'; - document.head.innerHTML = ``; + document.head.innerHTML = + `` + ''; const startIdleTransaction = jest.spyOn(hubExtensions, 'startIdleTransaction'); createBrowserTracing(true, { routingInstrumentation: customInstrumentRouting }); @@ -226,6 +228,9 @@ describe('BrowserTracing', () => { traceId: '126de09502ae4e0fb26c6967190756a4', parentSpanId: 'b6e54397b12a2a0f', parentSampled: true, + metadata: { + baggage: [{ release: '2.1.14' }, 'foo=bar'], + }, }), expect.any(Number), expect.any(Number), @@ -322,7 +327,7 @@ describe('BrowserTracing', () => { }); }); - describe('sentry-trace element', () => { + describe('sentry-trace and baggage elements', () => { describe('getMetaContent', () => { it('finds the specified tag and extracts the value', () => { const name = 'sentry-trace'; @@ -352,12 +357,12 @@ describe('BrowserTracing', () => { }); }); - describe('getHeaderContext', () => { + describe('extractTraceDataFromMetaTags()', () => { it('correctly parses a valid sentry-trace meta header', () => { document.head.innerHTML = ''; - const headerContext = getHeaderContext(); + const headerContext = extractTraceDataFromMetaTags(); expect(headerContext).toBeDefined(); expect(headerContext!.traceId).toEqual('12312012123120121231201212312012'); @@ -365,54 +370,94 @@ describe('BrowserTracing', () => { expect(headerContext!.parentSampled).toEqual(false); }); - it('returns undefined if the header is malformed', () => { + it('correctly parses a valid baggage meta header', () => { + document.head.innerHTML = ''; + + const headerContext = extractTraceDataFromMetaTags(); + + expect(headerContext).toBeDefined(); + expect(headerContext?.metadata?.baggage).toBeDefined(); + const baggage = headerContext?.metadata?.baggage!; + expect(baggage[0]).toBeDefined(); + expect(baggage[0]).toEqual({ + release: '2.1.12', + } as BaggageObj); + expect(baggage[1]).toBeDefined(); + expect(baggage[1]).toEqual('foo=bar'); + }); + + it('returns undefined if the sentry-trace header is malformed', () => { document.head.innerHTML = ''; - const headerContext = getHeaderContext(); + const headerContext = extractTraceDataFromMetaTags(); expect(headerContext).toBeUndefined(); }); + it('does not crash if the baggage header is malformed', () => { + document.head.innerHTML = ''; + + const headerContext = extractTraceDataFromMetaTags(); + + // TODO currently this creates invalid baggage. This must be adressed in a follow-up PR + expect(headerContext).toBeDefined(); + expect(headerContext?.metadata?.baggage).toBeDefined(); + const baggage = headerContext?.metadata?.baggage!; + expect(baggage[0]).toBeDefined(); + //expect(baggage[0]).toEqual); + expect(baggage[1]).toBeDefined(); + }); + it("returns undefined if the header isn't there", () => { document.head.innerHTML = ''; - const headerContext = getHeaderContext(); + const headerContext = extractTraceDataFromMetaTags(); expect(headerContext).toBeUndefined(); }); }); describe('using the data', () => { - it('uses the data for pageload transactions', () => { + it('uses the tracing data for pageload transactions', () => { // make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one document.head.innerHTML = - ''; + '' + + ''; // pageload transactions are created as part of the BrowserTracing integration's initialization createBrowserTracing(true); const transaction = getActiveTransaction(hub) as IdleTransaction; + const baggage = transaction.getBaggage()!; expect(transaction).toBeDefined(); expect(transaction.op).toBe('pageload'); expect(transaction.traceId).toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toEqual('1121201211212012'); expect(transaction.sampled).toBe(false); + expect(baggage).toBeDefined(); + expect(baggage[0]).toBeDefined(); + expect(baggage[0]).toEqual({ release: '2.1.14' }); + expect(baggage[1]).toBeDefined(); + expect(baggage[1]).toEqual('foo=bar'); }); it('ignores the data for navigation transactions', () => { mockChangeHistory = () => undefined; document.head.innerHTML = - ''; + '' + + ''; createBrowserTracing(true); mockChangeHistory({ to: 'here', from: 'there' }); const transaction = getActiveTransaction(hub) as IdleTransaction; + const baggage = transaction.getBaggage()!; expect(transaction).toBeDefined(); expect(transaction.op).toBe('navigation'); expect(transaction.traceId).not.toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toBeUndefined(); + expect(baggage).toBeUndefined(); }); }); }); From a4b2d597a3e48b776e0a03e93cfd326c7088e10e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 14:54:36 +0200 Subject: [PATCH 13/21] Add migration instructions (draft) for CORS adjustments --- MIGRATION.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/MIGRATION.md b/MIGRATION.md index d2d1695719b4..87140c7eae40 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -9,7 +9,9 @@ Below we will outline all the breaking changes you should consider when upgradin - Distributed CommonJS files will be ES6. Use a transpiler if you need to support old node versions. - We bumped the TypeScript version we generate our types with to 3.8.3. Please check if your TypeScript projects using TypeScript version 3.7 or lower still compile. Otherwise, upgrade your TypeScript version. - `whitelistUrls` and `blacklistUrls` have been renamed to `allowUrls` and `denyUrls` in the `Sentry.init()` options. -- The `UserAgent` integration is now called `HttpContext`. +- The `UserAgent` integration is now called `HttpContext`.# +- If you are using Performance Monitoring and with tracing enabled, you might have to [make adjustments to +your server's CORS settings](#-propagation-of-baggage-header) ## Dropping Support for Node.js v6 @@ -319,6 +321,11 @@ session.update({ environment: 'prod' }); session.close('ok'); ``` +## Propagation of Baggage Header + +We introduced a new way of propagating tracing and transaction-related information between services. This +change adds the [`baggage` HTTP header](https://www.w3.org/TR/baggage/) to outgoing requests if the instrumentation of requests is enabled. Since this adds a header to your HTTP requests, you might need +to adjust your Server's CORS settings to allow this additional header. ## General API Changes For our efforts to reduce bundle size of the SDK we had to remove and refactor parts of the package which introduced a few changes to the API: From ae7ca0b24c9f311da5c1de09c483abca8ddb2bac Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 15:40:11 +0200 Subject: [PATCH 14/21] fix migration doc typo --- MIGRATION.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 87140c7eae40..ae3b2936e4fa 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -9,7 +9,7 @@ Below we will outline all the breaking changes you should consider when upgradin - Distributed CommonJS files will be ES6. Use a transpiler if you need to support old node versions. - We bumped the TypeScript version we generate our types with to 3.8.3. Please check if your TypeScript projects using TypeScript version 3.7 or lower still compile. Otherwise, upgrade your TypeScript version. - `whitelistUrls` and `blacklistUrls` have been renamed to `allowUrls` and `denyUrls` in the `Sentry.init()` options. -- The `UserAgent` integration is now called `HttpContext`.# +- The `UserAgent` integration is now called `HttpContext`. - If you are using Performance Monitoring and with tracing enabled, you might have to [make adjustments to your server's CORS settings](#-propagation-of-baggage-header) @@ -325,7 +325,8 @@ session.close('ok'); We introduced a new way of propagating tracing and transaction-related information between services. This change adds the [`baggage` HTTP header](https://www.w3.org/TR/baggage/) to outgoing requests if the instrumentation of requests is enabled. Since this adds a header to your HTTP requests, you might need -to adjust your Server's CORS settings to allow this additional header. +to adjust your Server's CORS settings to allow this additional header. Take a look at the [Sentry docs](https://docs.sentry.io/platforms/javascript/performance/connect-services/#navigation-and-other-xhr-requests) +for more in-depth instructions what to change. ## General API Changes For our efforts to reduce bundle size of the SDK we had to remove and refactor parts of the package which introduced a few changes to the API: From aedb242dd0bcf9a1edf2607900c2460b06f568ff Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 16:57:52 +0200 Subject: [PATCH 15/21] fix linter errors (tracing) --- .../tracing/test/browser/browsertracing.test.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 2d6a4ef4c44e..0d5265522406 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -377,13 +377,13 @@ describe('BrowserTracing', () => { expect(headerContext).toBeDefined(); expect(headerContext?.metadata?.baggage).toBeDefined(); - const baggage = headerContext?.metadata?.baggage!; - expect(baggage[0]).toBeDefined(); - expect(baggage[0]).toEqual({ + const baggage = headerContext?.metadata?.baggage; + expect(baggage && baggage[0]).toBeDefined(); + expect(baggage && baggage[0]).toEqual({ release: '2.1.12', } as BaggageObj); - expect(baggage[1]).toBeDefined(); - expect(baggage[1]).toEqual('foo=bar'); + expect(baggage && baggage[1]).toBeDefined(); + expect(baggage && baggage[1]).toEqual('foo=bar'); }); it('returns undefined if the sentry-trace header is malformed', () => { @@ -402,10 +402,9 @@ describe('BrowserTracing', () => { // TODO currently this creates invalid baggage. This must be adressed in a follow-up PR expect(headerContext).toBeDefined(); expect(headerContext?.metadata?.baggage).toBeDefined(); - const baggage = headerContext?.metadata?.baggage!; - expect(baggage[0]).toBeDefined(); - //expect(baggage[0]).toEqual); - expect(baggage[1]).toBeDefined(); + const baggage = headerContext?.metadata?.baggage; + expect(baggage && baggage[0]).toBeDefined(); + expect(baggage && baggage[1]).toBeDefined(); }); it("returns undefined if the header isn't there", () => { From 81f7fa65d17558dbe1339ad8c0f5bf75d27a6f71 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 May 2022 17:36:06 +0200 Subject: [PATCH 16/21] add unit tests for serverless aws and gcp --- packages/serverless/test/awslambda.test.ts | 34 ++++++++++++++++++++ packages/serverless/test/gcpfunction.test.ts | 33 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts index 1e49d3d8326f..078eff338ea9 100644 --- a/packages/serverless/test/awslambda.test.ts +++ b/packages/serverless/test/awslambda.test.ts @@ -229,6 +229,40 @@ 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', + baggage: 'sentry-release=2.12.1,maisey=silly,charlie=goofy', + }; + + const handler: Handler = (_event, _context, callback) => { + expect(Sentry.startTransaction).toBeCalledWith( + expect.objectContaining({ + parentSpanId: '1121201211212012', + parentSampled: false, + op: 'awslambda.handler', + name: 'functionName', + traceId: '12312012123120121231201212312012', + metadata: { + baggage: [ + { + release: '2.12.1', + }, + 'maisey=silly,charlie=goofy', + ], + }, + }), + ); + + callback(undefined, { its: 'fine' }); + }; + + const wrappedHandler = wrapHandler(handler); + await wrappedHandler(fakeEvent, fakeContext, fakeCallback); + }); + test('capture error', async () => { expect.assertions(10); diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 5072de8b17b7..89b6aa084b9b 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -120,6 +120,39 @@ 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', + baggage: 'sentry-release=2.12.1,maisey=silly,charlie=goofy', + }; + await handleHttp(wrappedHandler, traceHeaders); + + expect(Sentry.startTransaction).toBeCalledWith( + expect.objectContaining({ + name: 'POST /path', + op: 'gcp.function.http', + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + metadata: { + baggage: [ + { + release: '2.12.1', + }, + 'maisey=silly,charlie=goofy', + ], + }, + }), + ); + }); + test('capture error', async () => { expect.assertions(5); From da3287843e9fd2aeaa2fb339d0c71454a3fdddbc Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 23 May 2022 09:19:41 +0200 Subject: [PATCH 17/21] add unit tests for mergeAndSerializeBaggage --- packages/utils/test/baggage.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts index e1c91a87f5ee..07ccdaaedd48 100644 --- a/packages/utils/test/baggage.test.ts +++ b/packages/utils/test/baggage.test.ts @@ -2,6 +2,7 @@ import { createBaggage, getBaggageValue, isBaggageEmpty, + mergeAndSerializeBaggage, parseBaggageString, serializeBaggage, setBaggageValue, @@ -105,4 +106,27 @@ describe('Baggage', () => { expect(isBaggageEmpty(baggage)).toEqual(outcome); }); }); + + describe('mergeAndSerializeBaggage', () => { + it.each([ + [ + 'returns original baggage when there is no additional baggage', + createBaggage({ release: '1.1.1', userid: '1234' }, 'foo=bar'), + undefined, + 'foo=bar,sentry-release=1.1.1,sentry-userid=1234', + ], + [ + 'returns merged baggage when there is a 3rd party header added', + createBaggage({ release: '1.1.1', userid: '1234' }, 'foo=bar'), + 'bar=baz,key=value', + 'bar=baz,key=value,sentry-release=1.1.1,sentry-userid=1234', + ], + ['returns merged baggage original baggage is empty', createBaggage({}), 'bar=baz,key=value', 'bar=baz,key=value'], + ['returns empty string when original and 3rd party baggage are empty', createBaggage({}), '', ''], + ['returns merged baggage original baggage is undefined', undefined, 'bar=baz,key=value', 'bar=baz,key=value'], + ['returns empty string when both params are undefined', undefined, undefined, ''], + ])('%s', (_: string, baggage, headerBaggageString, outcome) => { + expect(mergeAndSerializeBaggage(baggage, headerBaggageString)).toEqual(outcome); + }); + }); }); From c95af988be1f3d600ad1b2f622879f539ce9b52a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 23 May 2022 10:03:52 +0200 Subject: [PATCH 18/21] Apply suggestions from code review Co-authored-by: Luca Forstner --- MIGRATION.md | 1 + packages/utils/src/baggage.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/MIGRATION.md b/MIGRATION.md index ae3b2936e4fa..d6aad039ecaf 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -327,6 +327,7 @@ We introduced a new way of propagating tracing and transaction-related informati change adds the [`baggage` HTTP header](https://www.w3.org/TR/baggage/) to outgoing requests if the instrumentation of requests is enabled. Since this adds a header to your HTTP requests, you might need to adjust your Server's CORS settings to allow this additional header. Take a look at the [Sentry docs](https://docs.sentry.io/platforms/javascript/performance/connect-services/#navigation-and-other-xhr-requests) for more in-depth instructions what to change. + ## General API Changes For our efforts to reduce bundle size of the SDK we had to remove and refactor parts of the package which introduced a few changes to the API: diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index b37acdc1eefb..bc2f6568ff1f 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -92,7 +92,7 @@ export function parseBaggageString(inputBaggageString: string): Baggage { * a possibly created or modified baggage header by a third party that's been added * to the outgoing request header. * - * In case @param headerBaggage exists, we can safely add the the 3rd party part of @param headerBaggage + * In case @param headerBaggageString exists, we can safely add the the 3rd party part of @param headerBaggage * with our @param incomingBaggage. This is possible because if we modified anything beforehand, * it would only affect parts of the sentry baggage (@see Baggage interface). * From f4c8acfd409090902dd6cf30d0df3a84b156accb Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 23 May 2022 12:30:43 +0200 Subject: [PATCH 19/21] don't return empty trace data from meta tags but undefined --- packages/tracing/src/browser/browsertracing.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index aea47f466091..ae3912fc1fff 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -256,7 +256,7 @@ export function extractTraceDataFromMetaTags(): Partial | un // TODO more extensive checks for baggage validity/emptyness? if (sentrytraceData || baggage) { return { - ...sentrytraceData, + ...(sentrytraceData && { sentrytraceData }), ...(baggage && { metadata: { baggage } }), }; } From 8d502edb79e8ab48a46970675b4fa9f26c87c85f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 23 May 2022 13:56:51 +0200 Subject: [PATCH 20/21] fix meta tag extraction return value and unit tests after rebase --- .../suites/tracing/browsertracing/meta/test.ts | 4 ++-- packages/tracing/src/browser/browsertracing.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts index f4cae5743987..d2d44ae74050 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts +++ b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts @@ -30,8 +30,8 @@ sentryTest.skip( const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); - expect(envHeader.baggage).toBeDefined(); - expect(envHeader.baggage).toEqual('{version:2.1.12}'); + expect(envHeader.trace).toBeDefined(); + expect(envHeader.trace).toEqual('{version:2.1.12}'); }, ); diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index ae3912fc1fff..0d8e24de8c92 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -256,7 +256,7 @@ export function extractTraceDataFromMetaTags(): Partial | un // TODO more extensive checks for baggage validity/emptyness? if (sentrytraceData || baggage) { return { - ...(sentrytraceData && { sentrytraceData }), + ...(sentrytraceData && sentrytraceData), ...(baggage && { metadata: { baggage } }), }; } From bca837e610a3b07a5f0ede9b8cbf837771cccaed Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 23 May 2022 14:00:47 +0200 Subject: [PATCH 21/21] fix missing type export --- packages/core/test/lib/envelope.test.ts | 3 +-- packages/types/src/index.ts | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index 72060cb81364..13dda0e276d4 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -1,5 +1,4 @@ -import { DsnComponents, Event } from '@sentry/types'; -import { EventTraceContext } from '@sentry/types/build/types/envelope'; +import { DsnComponents, Event, EventTraceContext } from '@sentry/types'; import { createEventEnvelope } from '../../src/envelope'; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 6f9e3342b779..3032d7d6cdec 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -19,6 +19,7 @@ export type { EventEnvelope, EventEnvelopeHeaders, EventItem, + EventTraceContext, SessionEnvelope, SessionItem, UserFeedbackItem,