From b453e8ea32d37864885a883555247be5cd546684 Mon Sep 17 00:00:00 2001 From: Chris Turner <23338096+christopherjturner@users.noreply.github.com> Date: Tue, 10 Dec 2024 13:37:35 +0000 Subject: [PATCH 1/3] adds tests --- jest.config.js | 3 ++- package-lock.json | 9 ++++++++ package.json | 1 + .../common/helpers/logging/logger-options.js | 17 +++++++-------- .../common/helpers/logging/trace-id-mixin.js | 11 ++++++++++ .../helpers/logging/trace-id-mixin.test.js | 19 +++++++++++++++++ src/api/common/helpers/request-tracing.js | 9 ++++++++ src/api/index.js | 21 +++++++++++++------ src/config/index.js | 8 +++++++ 9 files changed, 82 insertions(+), 16 deletions(-) create mode 100644 src/api/common/helpers/logging/trace-id-mixin.js create mode 100644 src/api/common/helpers/logging/trace-id-mixin.test.js create mode 100644 src/api/common/helpers/request-tracing.js diff --git a/jest.config.js b/jest.config.js index 34bd9ff..1602b04 100644 --- a/jest.config.js +++ b/jest.config.js @@ -19,7 +19,8 @@ export default { '/.server', 'index.js' ], - coverageDirectory: '/coverage' + coverageDirectory: '/coverage', + transformIgnorePatterns: ['/node_modules(?!/(@defra/hapi-tracing))'] } /** diff --git a/package-lock.json b/package-lock.json index a832171..3a54659 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,6 +11,7 @@ "license": "OGL-UK-3.0", "dependencies": { "@babel/runtime": "^7.26.0", + "@defra/hapi-tracing": "^0.1.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", "@hapi/hapi": "^21.3.12", @@ -1876,6 +1877,14 @@ "integrity": "sha512-UcUu/DLh/aM4W3C8zZfwxxm6/6FIZUlm3mcAXuNOCa6Aj4iizNvNXQyb8DjZQH2jKSQbMRyNlngP6TPimuGjpQ==", "license": "MIT" }, + "node_modules/@defra/hapi-tracing": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/@defra/hapi-tracing/-/hapi-tracing-0.1.0.tgz", + "integrity": "sha512-I7iB18ew0wYZTvD4u+0cHZ7YBo8piF3i9308E6fejKDyllJfDkVRFWJOxUnD0yUGDcFsoYAR8oGFwsoD2HNfyw==", + "engines": { + "node": ">=22" + } + }, "node_modules/@elastic/ecs-helpers": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/@elastic/ecs-helpers/-/ecs-helpers-2.1.1.tgz", diff --git a/package.json b/package.json index a9024f0..4e31bce 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "license": "OGL-UK-3.0", "dependencies": { "@babel/runtime": "^7.26.0", + "@defra/hapi-tracing": "^0.1.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", "@hapi/hapi": "^21.3.12", diff --git a/src/api/common/helpers/logging/logger-options.js b/src/api/common/helpers/logging/logger-options.js index 6aa566f..3cc8631 100644 --- a/src/api/common/helpers/logging/logger-options.js +++ b/src/api/common/helpers/logging/logger-options.js @@ -1,5 +1,6 @@ import { ecsFormat } from '@elastic/ecs-pino-format' import { config } from '~/src/config/index.js' +import { traceIdMixin } from '~/src/api/common/helpers/logging/trace-id-mixin.js' const logConfig = config.get('log') const serviceName = config.get('serviceName') @@ -10,14 +11,10 @@ const serviceVersion = config.get('serviceVersion') */ const formatters = { ecs: { - ...ecsFormat(), - base: { - service: { - name: serviceName, - type: 'nodeJs', - version: serviceVersion - } - } + ...ecsFormat({ + serviceVersion, + serviceName + }) }, 'pino-pretty': { transport: { target: 'pino-pretty' } } } @@ -33,7 +30,9 @@ export const loggerOptions = { remove: true }, level: logConfig.level, - ...formatters[logConfig.format] + ...formatters[logConfig.format], + nesting: true, + mixin: traceIdMixin } /** diff --git a/src/api/common/helpers/logging/trace-id-mixin.js b/src/api/common/helpers/logging/trace-id-mixin.js new file mode 100644 index 0000000..8dbd232 --- /dev/null +++ b/src/api/common/helpers/logging/trace-id-mixin.js @@ -0,0 +1,11 @@ +import { getTraceId } from '@defra/hapi-tracing' + +export function traceIdMixin() { + const mixinValues = {} + const traceId = getTraceId() + + if (traceId) { + mixinValues.trace = { id: traceId } + } + return mixinValues +} diff --git a/src/api/common/helpers/logging/trace-id-mixin.test.js b/src/api/common/helpers/logging/trace-id-mixin.test.js new file mode 100644 index 0000000..91357fc --- /dev/null +++ b/src/api/common/helpers/logging/trace-id-mixin.test.js @@ -0,0 +1,19 @@ +import { traceIdMixin } from '~/src/api/common/helpers/logging/trace-id-mixin.js' +import { getTraceId } from '@defra/hapi-tracing' + +jest.mock('@defra/hapi-tracing', () => ({ + getTraceId: jest.fn(), + name: 'mock-hapi-tracing' +})) + +describe('traceIdMixin', () => { + test('adds traceId when set', () => { + getTraceId.mockReturnValue('1234') + expect(traceIdMixin()).toEqual({ trace: { id: '1234' } }) + }) + + test('doesnt add traceId if its not set', () => { + getTraceId.mockReturnValue(null) + expect(traceIdMixin()).toEqual({}) + }) +}) diff --git a/src/api/common/helpers/request-tracing.js b/src/api/common/helpers/request-tracing.js new file mode 100644 index 0000000..587463a --- /dev/null +++ b/src/api/common/helpers/request-tracing.js @@ -0,0 +1,9 @@ +import { tracing } from '@defra/hapi-tracing' +import { config } from '~/src/config/index.js' + +export const requestTracing = { + plugin: tracing.plugin, + options: { + tracingHeader: config.get('tracing.header') + } +} diff --git a/src/api/index.js b/src/api/index.js index b78ec8a..5e7ed73 100644 --- a/src/api/index.js +++ b/src/api/index.js @@ -8,6 +8,7 @@ import { mongoDb } from '~/src/api/common/helpers/mongodb.js' import { failAction } from '~/src/api/common/helpers/fail-action.js' import { secureContext } from '~/src/api/common/helpers/secure-context/index.js' import { pulse } from '~/src/api/common/helpers/pulse.js' +import { requestTracing } from '~/src/api/common/helpers/request-tracing.js' async function createServer() { const server = hapi.server({ @@ -39,12 +40,20 @@ async function createServer() { }) // Hapi Plugins: - // requestLogger - automatically logs incoming requests - // secureContext - loads CA certificates from environment config - // pulse - provides shutdown handlers - // mongoDb - sets up mongo connection pool and attaches to `server` and `request` objects - // router - routes used in the app - await server.register([requestLogger, secureContext, pulse, mongoDb, router]) + // requestLogger - automatically logs incoming requests + // requestTracing - trace header logging and propagation + // secureContext - loads CA certificates from environment config + // pulse - provides shutdown handlers + // mongoDb - sets up mongo connection pool and attaches to `server` and `request` objects + // router - routes used in the app + await server.register([ + requestLogger, + requestTracing, + secureContext, + pulse, + mongoDb, + router + ]) return server } diff --git a/src/config/index.js b/src/config/index.js index 4c1d350..563f25f 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -116,6 +116,14 @@ const config = convict({ format: Boolean, default: isProduction, env: 'ENABLE_METRICS' + }, + tracing: { + header: { + doc: 'Which header to track', + format: String, + default: 'x-cdp-request-id', + env: 'TRACING_HEADER' + } } }) From 6be825d1f9542792e2629357100373b2fddaaa0c Mon Sep 17 00:00:00 2001 From: Chris Turner <23338096+christopherjturner@users.noreply.github.com> Date: Tue, 10 Dec 2024 14:12:59 +0000 Subject: [PATCH 2/3] inlines mixin --- jest.config.js | 2 +- .../common/helpers/logging/logger-options.js | 11 +++++++++-- .../common/helpers/logging/trace-id-mixin.js | 11 ----------- .../helpers/logging/trace-id-mixin.test.js | 19 ------------------- 4 files changed, 10 insertions(+), 33 deletions(-) delete mode 100644 src/api/common/helpers/logging/trace-id-mixin.js delete mode 100644 src/api/common/helpers/logging/trace-id-mixin.test.js diff --git a/jest.config.js b/jest.config.js index 1602b04..d07c768 100644 --- a/jest.config.js +++ b/jest.config.js @@ -20,7 +20,7 @@ export default { 'index.js' ], coverageDirectory: '/coverage', - transformIgnorePatterns: ['/node_modules(?!/(@defra/hapi-tracing))'] + transformIgnorePatterns: ['/node_modules(?!/(@defra/hapi-tracing))'] // ESM only modules } /** diff --git a/src/api/common/helpers/logging/logger-options.js b/src/api/common/helpers/logging/logger-options.js index 3cc8631..6932d69 100644 --- a/src/api/common/helpers/logging/logger-options.js +++ b/src/api/common/helpers/logging/logger-options.js @@ -1,6 +1,6 @@ import { ecsFormat } from '@elastic/ecs-pino-format' import { config } from '~/src/config/index.js' -import { traceIdMixin } from '~/src/api/common/helpers/logging/trace-id-mixin.js' +import { getTraceId } from '@defra/hapi-tracing' const logConfig = config.get('log') const serviceName = config.get('serviceName') @@ -32,7 +32,14 @@ export const loggerOptions = { level: logConfig.level, ...formatters[logConfig.format], nesting: true, - mixin: traceIdMixin + mixin() { + const mixinValues = {} + const traceId = getTraceId() + if (traceId) { + mixinValues.trace = { id: traceId } + } + return mixinValues + } } /** diff --git a/src/api/common/helpers/logging/trace-id-mixin.js b/src/api/common/helpers/logging/trace-id-mixin.js deleted file mode 100644 index 8dbd232..0000000 --- a/src/api/common/helpers/logging/trace-id-mixin.js +++ /dev/null @@ -1,11 +0,0 @@ -import { getTraceId } from '@defra/hapi-tracing' - -export function traceIdMixin() { - const mixinValues = {} - const traceId = getTraceId() - - if (traceId) { - mixinValues.trace = { id: traceId } - } - return mixinValues -} diff --git a/src/api/common/helpers/logging/trace-id-mixin.test.js b/src/api/common/helpers/logging/trace-id-mixin.test.js deleted file mode 100644 index 91357fc..0000000 --- a/src/api/common/helpers/logging/trace-id-mixin.test.js +++ /dev/null @@ -1,19 +0,0 @@ -import { traceIdMixin } from '~/src/api/common/helpers/logging/trace-id-mixin.js' -import { getTraceId } from '@defra/hapi-tracing' - -jest.mock('@defra/hapi-tracing', () => ({ - getTraceId: jest.fn(), - name: 'mock-hapi-tracing' -})) - -describe('traceIdMixin', () => { - test('adds traceId when set', () => { - getTraceId.mockReturnValue('1234') - expect(traceIdMixin()).toEqual({ trace: { id: '1234' } }) - }) - - test('doesnt add traceId if its not set', () => { - getTraceId.mockReturnValue(null) - expect(traceIdMixin()).toEqual({}) - }) -}) From 5a63fdba939d3786e76da4cafab3b8098e0e2ff8 Mon Sep 17 00:00:00 2001 From: Chris Turner <23338096+christopherjturner@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:41:39 +0000 Subject: [PATCH 3/3] Bumps tracing version --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3a54659..115eabc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "license": "OGL-UK-3.0", "dependencies": { "@babel/runtime": "^7.26.0", - "@defra/hapi-tracing": "^0.1.0", + "@defra/hapi-tracing": "^0.1.1", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", "@hapi/hapi": "^21.3.12", @@ -1878,9 +1878,9 @@ "license": "MIT" }, "node_modules/@defra/hapi-tracing": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/@defra/hapi-tracing/-/hapi-tracing-0.1.0.tgz", - "integrity": "sha512-I7iB18ew0wYZTvD4u+0cHZ7YBo8piF3i9308E6fejKDyllJfDkVRFWJOxUnD0yUGDcFsoYAR8oGFwsoD2HNfyw==", + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/@defra/hapi-tracing/-/hapi-tracing-0.1.1.tgz", + "integrity": "sha512-zm4DkgFq9wewTP4YmZ7sic6i1hP7pva/X9TJSX6Hntv5NGxkXObNPoQoWLOLHJiyf5qtOW+1QmO4v4czQIOyUQ==", "engines": { "node": ">=22" } diff --git a/package.json b/package.json index a8ea68a..88312c1 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "license": "OGL-UK-3.0", "dependencies": { "@babel/runtime": "^7.26.0", - "@defra/hapi-tracing": "^0.1.0", + "@defra/hapi-tracing": "^0.1.1", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", "@hapi/hapi": "^21.3.12",