From 74672a0330126e4c13f6ed5e655ad549ccad5c1e Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 1 Sep 2022 14:17:32 -0400 Subject: [PATCH] feat(remix): Continue transaction from request headers (#5600) Enables propogation of traces through `sentry-trace` and dynamic sampling propogation through `baggage` --- .../node-integration-tests/utils/index.ts | 20 ++++-- packages/remix/src/utils/instrumentServer.ts | 65 +++++++++++++++---- .../remix/src/utils/serverAdapters/express.ts | 12 +++- .../integration/test/server/loader.test.ts | 27 ++++++++ 4 files changed, 103 insertions(+), 21 deletions(-) diff --git a/packages/node-integration-tests/utils/index.ts b/packages/node-integration-tests/utils/index.ts index 5af54b065686..50da10fd7820 100644 --- a/packages/node-integration-tests/utils/index.ts +++ b/packages/node-integration-tests/utils/index.ts @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { EnvelopeItemType } from '@sentry/types'; import { logger, parseSemver } from '@sentry/utils'; -import axios from 'axios'; +import axios, { AxiosRequestConfig } from 'axios'; import { Express } from 'express'; import * as http from 'http'; import nock from 'nock'; @@ -102,12 +102,16 @@ export async function runScenario(url: string): Promise { await Sentry.flush(); } -async function makeRequest(method: 'get' | 'post' = 'get', url: string): Promise { +async function makeRequest( + method: 'get' | 'post' = 'get', + url: string, + axiosConfig?: AxiosRequestConfig, +): Promise { try { if (method === 'get') { - await axios.get(url); + await axios.get(url, axiosConfig); } else { - await axios.post(url); + await axios.post(url, axiosConfig); } } catch (e) { // We sometimes expect the request to fail, but not the test. @@ -117,6 +121,8 @@ async function makeRequest(method: 'get' | 'post' = 'get', url: string): Promise } export class TestEnv { + private _axiosConfig: AxiosRequestConfig | undefined = undefined; + public constructor(public readonly server: http.Server, public readonly url: string) { this.server = server; this.url = url; @@ -173,7 +179,7 @@ export class TestEnv { envelopeTypeArray, ); - void makeRequest(options.method, options.url || this.url); + void makeRequest(options.method, options.url || this.url, this._axiosConfig); return resProm; } @@ -246,4 +252,8 @@ export class TestEnv { .reply(200); }); } + + public setAxiosConfig(axiosConfig: AxiosRequestConfig): void { + this._axiosConfig = axiosConfig; + } } diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 04107fe07939..36a62a3749c0 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -1,8 +1,18 @@ /* eslint-disable max-lines */ import { captureException, getCurrentHub, Hub } from '@sentry/node'; import { getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; -import { Transaction, WrappedFunction } from '@sentry/types'; -import { addExceptionMechanism, fill, isNodeEnv, loadModule, logger, serializeBaggage } from '@sentry/utils'; +import { Transaction, TransactionSource, WrappedFunction } from '@sentry/types'; +import { + addExceptionMechanism, + extractTraceparentData, + fill, + isNodeEnv, + isSentryBaggageEmpty, + loadModule, + logger, + parseBaggageSetMutability, + serializeBaggage, +} from '@sentry/utils'; import * as domain from 'domain'; import { @@ -289,33 +299,52 @@ function matchServerRoutes( * @param pkg */ export function startRequestHandlerTransaction( - url: URL, - method: string, - routes: ServerRoute[], hub: Hub, - pkg?: ReactRouterDomPkg, + name: string, + source: TransactionSource, + request: { + headers: { + 'sentry-trace': string; + baggage: string; + }; + method: string; + }, ): Transaction { - const currentScope = hub.getScope(); - const matches = matchServerRoutes(routes, url.pathname, pkg); + // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision) + const traceparentData = extractTraceparentData(request.headers['sentry-trace']); + const baggage = parseBaggageSetMutability(request.headers.baggage, traceparentData); - const match = matches && getRequestMatch(url, matches); - const name = match === null ? url.pathname : match.route.id; - const source = match === null ? 'url' : 'route'; const transaction = hub.startTransaction({ name, op: 'http.server', tags: { - method: method, + method: request.method, }, + ...traceparentData, metadata: { source, + // Only attach baggage if it's defined + ...(!isSentryBaggageEmpty(baggage) && { baggage }), }, }); - currentScope?.setSpan(transaction); + hub.getScope()?.setSpan(transaction); return transaction; } +/** + * Get transaction name from routes and url + */ +export function getTransactionName( + routes: ServerRoute[], + url: URL, + pkg?: ReactRouterDomPkg, +): [string, TransactionSource] { + const matches = matchServerRoutes(routes, url.pathname, pkg); + const match = matches && getRequestMatch(url, matches); + return match === null ? [url.pathname, 'url'] : [match.route.id, 'route']; +} + function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBuild): RequestHandler { const routes = createRoutes(build.routes); const pkg = loadModule('react-router-dom'); @@ -330,7 +359,15 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui } const url = new URL(request.url); - const transaction = startRequestHandlerTransaction(url, request.method, routes, hub, pkg); + const [name, source] = getTransactionName(routes, url, pkg); + + const transaction = startRequestHandlerTransaction(hub, name, source, { + headers: { + 'sentry-trace': request.headers.get('sentry-trace') || '', + baggage: request.headers.get('baggage') || '', + }, + method: request.method, + }); const res = (await origRequestHandler.call(this, request, loadContext)) as Response; diff --git a/packages/remix/src/utils/serverAdapters/express.ts b/packages/remix/src/utils/serverAdapters/express.ts index 71798863430c..f3d3a7218281 100644 --- a/packages/remix/src/utils/serverAdapters/express.ts +++ b/packages/remix/src/utils/serverAdapters/express.ts @@ -2,10 +2,11 @@ import { getCurrentHub } from '@sentry/hub'; import { flush } from '@sentry/node'; import { hasTracingEnabled } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; -import { extractRequestData, loadModule, logger } from '@sentry/utils'; +import { extractRequestData, isString, loadModule, logger } from '@sentry/utils'; import { createRoutes, + getTransactionName, instrumentBuild, isRequestHandlerWrapped, startRequestHandlerTransaction, @@ -51,7 +52,14 @@ function wrapExpressRequestHandler( } const url = new URL(request.url); - const transaction = startRequestHandlerTransaction(url, request.method, routes, hub, pkg); + const [name, source] = getTransactionName(routes, url, pkg); + const transaction = startRequestHandlerTransaction(hub, name, source, { + headers: { + 'sentry-trace': (req.headers && isString(req.headers['sentry-trace']) && req.headers['sentry-trace']) || '', + baggage: (req.headers && isString(req.headers.baggage) && req.headers.baggage) || '', + }, + method: request.method, + }); // save a link to the transaction on the response, so that even if there's an error (landing us outside of // the domain), we can still finish it (albeit possibly missing some scope data) (res as AugmentedExpressResponse).__sentryTransaction = transaction; diff --git a/packages/remix/test/integration/test/server/loader.test.ts b/packages/remix/test/integration/test/server/loader.test.ts index 8d8b0b93ce00..78ecc2a67c01 100644 --- a/packages/remix/test/integration/test/server/loader.test.ts +++ b/packages/remix/test/integration/test/server/loader.test.ts @@ -157,4 +157,31 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada expect(tags[key]).toEqual(val); }); }); + + it('continues transaction from sentry-trace header and baggage', async () => { + const env = await RemixTestEnv.init(adapter); + const url = `${env.url}/loader-json-response/3`; + + // send sentry-trace and baggage headers to loader + env.setAxiosConfig({ + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-version=1.0,sentry-environment=production,sentry-trace_id=12312012123120121231201212312012', + }, + }); + const envelope = await env.getEnvelopeRequest({ url, envelopeType: 'transaction' }); + + expect(envelope[0].trace).toMatchObject({ + trace_id: '12312012123120121231201212312012', + }); + + assertSentryTransaction(envelope[2], { + contexts: { + trace: { + trace_id: '12312012123120121231201212312012', + parent_span_id: '1121201211212012', + }, + }, + }); + }); });