From 7e61bf0c7491f74123868f77acb93524310576fa Mon Sep 17 00:00:00 2001 From: Patrick Heneise Date: Wed, 22 Sep 2021 12:38:10 +0300 Subject: [PATCH 1/2] fix: add missing request to sentry transaction --- index.js | 26 ++++++++-- lib/extractRequestData.js | 48 +++++++++++++++++++ test/{auth.js => auth.test.js} | 0 test/{basic.js => basic.test.js} | 0 ...configuration.js => configuration.test.js} | 0 ...error-handler.js => error-handler.test.js} | 1 + test/extractRequestData.test.js | 39 +++++++++++++++ 7 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 lib/extractRequestData.js rename test/{auth.js => auth.test.js} (100%) rename test/{basic.js => basic.test.js} (100%) rename test/{configuration.js => configuration.test.js} (100%) rename test/{error-handler.js => error-handler.test.js} (95%) create mode 100644 test/extractRequestData.test.js diff --git a/index.js b/index.js index 2123771..e9c820b 100644 --- a/index.js +++ b/index.js @@ -3,6 +3,7 @@ const fp = require('fastify-plugin') const Sentry = require('@sentry/node') const Tracing = require('@sentry/tracing') // eslint-disable-line no-unused-vars +const extractRequestData = require('./lib/extractRequestData.js') function sentryConnector(fastify, opts, next) { if (!opts || !opts.dsn) { @@ -22,10 +23,27 @@ function sentryConnector(fastify, opts, next) { fastify.decorateRequest('sentryTransaction', null) fastify.addHook('onRequest', async (request) => { - request.sentryTransaction = Sentry.startTransaction({ - op: 'request', - name: `${request.method} ${request.url}` - }) + // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision) + let traceparentData + if ( + request.headers && + typeof request.headers['sentry-trace'] === 'string' + ) { + traceparentData = Tracing.extractTraceparentData( + request.headers['sentry-trace'] + ) + } + + const extractedRequestData = extractRequestData(request) + + request.sentryTransaction = Sentry.startTransaction( + { + op: 'request', + name: `${request.method} ${request.url}`, + ...traceparentData + }, + { request: extractedRequestData } + ) return }) diff --git a/lib/extractRequestData.js b/lib/extractRequestData.js new file mode 100644 index 0000000..745a1cb --- /dev/null +++ b/lib/extractRequestData.js @@ -0,0 +1,48 @@ +'use strict' + +const DEFAULT_REQUEST_KEYS = ['headers', 'method', 'query_string', 'url'] + +/** + * Function copied from + * https://github.com/getsentry/sentry-javascript/blob/master/packages/node/src/handlers.ts + * and mofidied for Fastify + * + * Data (req.body) isn't available in onRequest hook, + * as it is parsed later in the fastify lifecycle + * https://www.fastify.io/docs/latest/Hooks/#onrequest + */ +function extractRequestData(req, keys) { + if (!keys || keys.length <= 0 || typeof keys !== 'object') { + keys = DEFAULT_REQUEST_KEYS + } + const requestData = {} + const headers = req.headers || {} + const method = req.method + const host = req.hostname + const protocol = req.protocol + const originalUrl = req.url + const absoluteUrl = protocol + '://' + host + originalUrl + keys.forEach(function (key) { + switch (key) { + case 'headers': + requestData.headers = headers + break + case 'method': + requestData.method = method + break + case 'url': + requestData.url = absoluteUrl + break + case 'query_string': + requestData.query_string = Object.assign({}, req.query) + break + default: + if ({}.hasOwnProperty.call(req, key)) { + requestData[key] = req[key] + } + } + }) + return requestData +} + +module.exports = exports = extractRequestData diff --git a/test/auth.js b/test/auth.test.js similarity index 100% rename from test/auth.js rename to test/auth.test.js diff --git a/test/basic.js b/test/basic.test.js similarity index 100% rename from test/basic.js rename to test/basic.test.js diff --git a/test/configuration.js b/test/configuration.test.js similarity index 100% rename from test/configuration.js rename to test/configuration.test.js diff --git a/test/error-handler.js b/test/error-handler.test.js similarity index 95% rename from test/error-handler.js rename to test/error-handler.test.js index 86f92c6..70fb19e 100644 --- a/test/error-handler.js +++ b/test/error-handler.test.js @@ -30,6 +30,7 @@ tap.test('sentryConnector with custom error handler', async (test) => { const response = await fastify.inject({ method: 'POST', + headers: { 'sentry-trace': 'testing trace' }, url: '/', payload: { error: 503, message: 'Internal Server Error' } }) diff --git a/test/extractRequestData.test.js b/test/extractRequestData.test.js new file mode 100644 index 0000000..523bb00 --- /dev/null +++ b/test/extractRequestData.test.js @@ -0,0 +1,39 @@ +'use strict' + +/* eslint-disable node/no-unpublished-require */ +const tap = require('tap') +const fn = require('../lib/extractRequestData') + +tap.test('extractedRequestData should process fastify request', (test) => { + test.plan(1) + const request = { + headers: { + host: 'localhost:3000', + accept: '/*/' + }, + hostname: 'localhost:3000', + protocol: 'http', + method: 'GET', + url: '/testing' + } + + const expected = { + headers: { host: 'localhost:3000', accept: '/*/' }, + method: 'GET', + query_string: {}, + url: 'http://localhost:3000/testing' + } + + const actual = fn(request) + test.strictSame(actual, expected) +}) + +tap.test('extractedRequestData should process other keys', (test) => { + test.plan(1) + const request = { + testing: 'testing' + } + + const actual = fn(request, ['testing']) + test.equal(actual.testing, 'testing') +}) From 8df55789c07feda00e8591aaa61d399d7baa5506 Mon Sep 17 00:00:00 2001 From: Patrick Heneise Date: Wed, 22 Sep 2021 12:41:37 +0300 Subject: [PATCH 2/2] test: branch coverage :100: --- test/extractRequestData.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/extractRequestData.test.js b/test/extractRequestData.test.js index 523bb00..f98d605 100644 --- a/test/extractRequestData.test.js +++ b/test/extractRequestData.test.js @@ -29,11 +29,12 @@ tap.test('extractedRequestData should process fastify request', (test) => { }) tap.test('extractedRequestData should process other keys', (test) => { - test.plan(1) + test.plan(2) const request = { testing: 'testing' } - const actual = fn(request, ['testing']) + const actual = fn(request, ['testing', 'testing2']) test.equal(actual.testing, 'testing') + test.equal(actual.testing2, undefined) })