From 5b9a450bb125400f3a24227f7a3b02c56ab972cb Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Tue, 4 Nov 2025 13:00:38 +0200 Subject: [PATCH 01/13] feat(LLMO-1023): add trace ID support to log wrapper - Add getTraceId() function to extract AWS X-Ray trace IDs - Enhance logWrapper to include both jobId and traceId in logs - Add comprehensive test coverage for trace ID functionality - Update TypeScript definitions - Update README with trace ID documentation --- packages/spacecat-shared-utils/README.md | 51 +++++++++++++ packages/spacecat-shared-utils/src/index.d.ts | 22 ++++++ packages/spacecat-shared-utils/src/index.js | 2 +- .../spacecat-shared-utils/src/log-wrapper.js | 39 +++++++--- packages/spacecat-shared-utils/src/xray.js | 21 +++++ .../test/log-wrapper.test.js | 76 ++++++++++++++++++- .../spacecat-shared-utils/test/xray.test.js | 73 +++++++++++++++++- 7 files changed, 269 insertions(+), 15 deletions(-) diff --git a/packages/spacecat-shared-utils/README.md b/packages/spacecat-shared-utils/README.md index e2b0e913f..6a3e213fa 100644 --- a/packages/spacecat-shared-utils/README.md +++ b/packages/spacecat-shared-utils/README.md @@ -45,6 +45,42 @@ The library includes the following utility functions: - `hasText(str)`: Checks if the given string is not empty. - `dateAfterDays(number)`: Calculates the date after a specified number of days from the current date. +## Log Wrapper + +The `logWrapper` enhances your Lambda function logs by automatically prepending `jobId` (from message) and `traceId` (from AWS X-Ray) to all log statements. This improves log traceability across distributed services. + +### Features +- Automatically extracts AWS X-Ray trace ID +- Includes jobId from message when available +- Creates `context.contextualLog` with enhanced logging methods +- Works seamlessly with existing log levels (info, error, debug, warn, trace, etc.) + +### Usage + +```javascript +import { logWrapper, sqsEventAdapter } from '@adobe/spacecat-shared-utils'; + +async function run(message, context) { + const { contextualLog } = context; + + // Use contextualLog instead of log for enhanced logging + contextualLog.info('Processing started'); + // Output: [jobId=xxx] [traceId=1-xxx-xxx] Processing started +} + +export const main = wrap(run) + .with(sqsEventAdapter) + .with(logWrapper) // Add this line early in the wrapper chain + .with(dataAccess) + .with(sqs) + .with(secrets) + .with(helixStatus); +``` + +**Important:** Use `context.contextualLog` instead of `context.log` in your functions to get trace ID and job ID in your logs. + +For detailed integration instructions, see [LOG_WRAPPER_INTEGRATION_GUIDE.md](../../LOG_WRAPPER_INTEGRATION_GUIDE.md). + ## SQS Event Adapter The library also includes an SQS event adapter to convert an SQS record into a function parameter. This is useful when working with AWS Lambda functions that are triggered by an SQS event. Usage: @@ -62,6 +98,21 @@ export const main = wrap(run) .with(helixStatus); ```` +## AWS X-Ray Integration + +### getTraceId() + +Extracts the current AWS X-Ray trace ID from the segment. Returns `null` if not in AWS Lambda or no segment is available. + +```javascript +import { getTraceId } from '@adobe/spacecat-shared-utils'; + +const traceId = getTraceId(); +// Returns: '1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e' or null +``` + +This function is automatically used by `logWrapper` to include trace IDs in logs. + ## Testing This library includes a comprehensive test suite to ensure the reliability of the utility functions. To run the tests, use the following command: diff --git a/packages/spacecat-shared-utils/src/index.d.ts b/packages/spacecat-shared-utils/src/index.d.ts index afb3ea3d7..4f545fba7 100644 --- a/packages/spacecat-shared-utils/src/index.d.ts +++ b/packages/spacecat-shared-utils/src/index.d.ts @@ -64,6 +64,28 @@ export function sqsWrapper(fn: (message: object, context: object) => Promise Promise): (request: object, context: object) => Promise; +/** + * A higher-order function that wraps a given function and enhances logging by appending + * a `jobId` and `traceId` to log messages when available. + * @param fn - The original function to be wrapped + * @returns A wrapped function that enhances logging + */ +export function logWrapper(fn: (message: object, context: object) => Promise): + (message: object, context: object) => Promise; + +/** + * Instruments an AWS SDK v3 client with X-Ray tracing when running in AWS Lambda. + * @param client - The AWS SDK v3 client to instrument + * @returns The instrumented client (or original client if not in Lambda) + */ +export function instrumentAWSClient(client: T): T; + +/** + * Extracts the trace ID from the current AWS X-Ray segment. + * @returns The trace ID if available, or null if not in AWS Lambda or no segment found + */ +export function getTraceId(): string | null; + /** * Prepends 'https://' schema to the URL if it's not already present. * @param url - The URL to modify. diff --git a/packages/spacecat-shared-utils/src/index.js b/packages/spacecat-shared-utils/src/index.js index ed6bac01f..9641d8481 100644 --- a/packages/spacecat-shared-utils/src/index.js +++ b/packages/spacecat-shared-utils/src/index.js @@ -52,7 +52,7 @@ export { sqsWrapper } from './sqs.js'; export { sqsEventAdapter } from './sqs.js'; export { logWrapper } from './log-wrapper.js'; -export { instrumentAWSClient } from './xray.js'; +export { instrumentAWSClient, getTraceId } from './xray.js'; export { composeBaseURL, diff --git a/packages/spacecat-shared-utils/src/log-wrapper.js b/packages/spacecat-shared-utils/src/log-wrapper.js index 021f8b386..708132cbd 100644 --- a/packages/spacecat-shared-utils/src/log-wrapper.js +++ b/packages/spacecat-shared-utils/src/log-wrapper.js @@ -10,45 +10,62 @@ * governing permissions and limitations under the License. */ +import { getTraceId } from './xray.js'; + /** * A higher-order function that wraps a given function and enhances logging by appending - * a `jobId` to log messages when available. This improves traceability of logs associated - * with specific jobs or processes. + * a `jobId` and `traceId` to log messages when available. This improves traceability of logs + * associated with specific jobs or processes. * * The wrapper checks if a `log` object exists in the `context` and whether the `message` - * contains a `jobId`. If found, log methods (e.g., `info`, `error`, etc.) will prepend the - * `jobId` to all log statements where `context.contextualLog` is used. If no `jobId` is found, - * logging will remain unchanged. + * contains a `jobId`. It also extracts the AWS X-Ray trace ID if available. If found, log + * methods (e.g., `info`, `error`, etc.) will prepend the `jobId` and/or `traceId` to all log + * statements where `context.contextualLog` is used. If neither is found, logging will remain + * unchanged. * * @param {function} fn - The original function to be wrapped, called with the provided * message and context after logging enhancement. * @returns {function(object, object): Promise} - A wrapped function that enhances * logging and returns the result of the original function. * - * `context.contextualLog` will include logging methods with `jobId` prefixed, or fall back - * to the existing `log` object if no `jobId` is provided. + * `context.contextualLog` will include logging methods with `jobId` and/or `traceId` prefixed, + * or fall back to the existing `log` object if neither is provided. */ export function logWrapper(fn) { return async (message, context) => { const { log } = context; if (log && !context.contextualLog) { + const markers = []; + + // Extract jobId from message if available if (typeof message === 'object' && message !== null && 'jobId' in message) { const { jobId } = message; - const jobIdMarker = `[jobId=${jobId}]`; + markers.push(`[jobId=${jobId}]`); + } + + // Extract traceId from AWS X-Ray + const traceId = getTraceId(); + if (traceId) { + markers.push(`[traceId=${traceId}]`); + } + + // If we have markers, enhance the log object + if (markers.length > 0) { + const markerString = markers.join(' '); // Define log levels const logLevels = ['info', 'error', 'debug', 'warn', 'trace', 'verbose', 'silly', 'fatal']; - // Enhance the log object to include jobId in all log statements + // Enhance the log object to include markers in all log statements context.contextualLog = logLevels.reduce((accumulator, level) => { if (typeof log[level] === 'function') { - accumulator[level] = (...args) => log[level](jobIdMarker, ...args); + accumulator[level] = (...args) => log[level](markerString, ...args); } return accumulator; }, {}); } else { - log.debug('No jobId found in the provided message. Log entries will be recorded without a jobId.'); + log.debug('No jobId or traceId found. Log entries will be recorded without additional context.'); context.contextualLog = log; } } diff --git a/packages/spacecat-shared-utils/src/xray.js b/packages/spacecat-shared-utils/src/xray.js index 32e055bd7..a205072c2 100644 --- a/packages/spacecat-shared-utils/src/xray.js +++ b/packages/spacecat-shared-utils/src/xray.js @@ -16,3 +16,24 @@ import { isAWSLambda } from './runtimes.js'; export function instrumentAWSClient(client) { return isAWSLambda() ? AWSXray.captureAWSv3Client(client) : client; } + +/** + * Extracts the trace ID from the current AWS X-Ray segment. + * This function is designed to work in AWS Lambda environments where X-Ray tracing is enabled. + * + * @returns {string|null} The trace ID if available, or null if not in AWS Lambda or no segment found + */ +export function getTraceId() { + if (!isAWSLambda()) { + return null; + } + + const segment = AWSXray.getSegment(); + if (!segment) { + return null; + } + + // Get the root trace ID + const effectiveSegment = segment.segment || segment; + return effectiveSegment.trace_id; +} diff --git a/packages/spacecat-shared-utils/test/log-wrapper.test.js b/packages/spacecat-shared-utils/test/log-wrapper.test.js index c16784943..069fcfe92 100644 --- a/packages/spacecat-shared-utils/test/log-wrapper.test.js +++ b/packages/spacecat-shared-utils/test/log-wrapper.test.js @@ -14,7 +14,10 @@ import sinon from 'sinon'; import { expect } from 'chai'; -import { logWrapper } from '../src/index.js'; +import esmock from 'esmock'; + +let logWrapper; +let getTraceIdStub; const message = { processingType: 'import', @@ -41,8 +44,19 @@ const mockFnFromSqs = sinon.spy(); let mockContext; describe('logWrapper tests', () => { + before(async () => { + getTraceIdStub = sinon.stub().returns(null); + + logWrapper = await esmock('../src/log-wrapper.js', { + '../src/xray.js': { + getTraceId: getTraceIdStub, + }, + }).then((module) => module.logWrapper); + }); + beforeEach(() => { sinon.resetHistory(); + getTraceIdStub.returns(null); // Default to no trace ID mockContext = { // Simulate an SQS event invocation: { @@ -66,7 +80,7 @@ describe('logWrapper tests', () => { }); afterEach(() => { - sinon.restore(); + sinon.resetHistory(); }); it('should call the original function with the provided message and context, and update the context with contextualLog object', async () => { @@ -134,4 +148,62 @@ describe('logWrapper tests', () => { expect(logArgs).to.not.contain('[jobId='); }); }); + + logLevels.forEach((level) => { + it(`should call ${level} log method with traceId when available`, async () => { + getTraceIdStub.returns('1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e'); + const wrappedFn = logWrapper(mockFnFromSqs); + + await wrappedFn(message, mockContext); + + // Log something to test the wrapper + mockContext.contextualLog[level](`${level} log`); + + // Verify that the traceId is included in the log statement + const logArgs = mockContext.log[level].getCall(0).args[0]; + expect(logArgs).to.contain('[traceId=1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e]'); + }); + }); + + logLevels.forEach((level) => { + it(`should call ${level} log method with both jobId and traceId when both are available`, async () => { + getTraceIdStub.returns('1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e'); + const wrappedFn = logWrapper(mockFnFromSqs); + + await wrappedFn(message, mockContext); + + // Log something to test the wrapper + mockContext.contextualLog[level](`${level} log`); + + // Verify that both jobId and traceId are included in the log statement + const logArgs = mockContext.log[level].getCall(0).args[0]; + expect(logArgs).to.contain(`[jobId=${message.jobId}]`); + expect(logArgs).to.contain('[traceId=1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e]'); + }); + }); + + it('should not include traceId when getTraceId returns null', async () => { + getTraceIdStub.returns(null); + const wrappedFn = logWrapper(mockFnFromSqs); + + await wrappedFn(message, mockContext); + + // Log something to test the wrapper + mockContext.contextualLog.info('info log'); + + // Verify that the traceId is not included in the log statement + const logArgs = mockContext.log.info.getCall(0).args[0]; + expect(logArgs).to.not.contain('[traceId='); + expect(logArgs).to.contain(`[jobId=${message.jobId}]`); + }); + + it('should assign context.log to context.contextualLog when neither jobId nor traceId are available', async () => { + getTraceIdStub.returns(null); + const wrappedFn = logWrapper(mockFnFromSqs); + + await wrappedFn({}, mockContext); + + expect(mockContext).to.have.property('contextualLog'); + expect(mockContext.contextualLog).to.equal(mockContext.log); + }); }); diff --git a/packages/spacecat-shared-utils/test/xray.test.js b/packages/spacecat-shared-utils/test/xray.test.js index 3b4a06058..c8e911263 100644 --- a/packages/spacecat-shared-utils/test/xray.test.js +++ b/packages/spacecat-shared-utils/test/xray.test.js @@ -15,7 +15,7 @@ import { expect } from 'chai'; import sinon from 'sinon'; import AWSXray from 'aws-xray-sdk'; -import { instrumentAWSClient } from '../src/index.js'; +import { instrumentAWSClient, getTraceId } from '../src/index.js'; describe('instrumentClient', () => { let captureStub; @@ -52,3 +52,74 @@ describe('instrumentClient', () => { expect(result).to.equal(client); }); }); + +describe('getTraceId', () => { + let getSegmentStub; + + beforeEach(() => { + getSegmentStub = sinon.stub(AWSXray, 'getSegment'); + }); + + afterEach(() => { + sinon.restore(); + delete process.env.AWS_EXECUTION_ENV; + }); + + it('should return null when not in AWS Lambda environment', () => { + delete process.env.AWS_EXECUTION_ENV; + + const result = getTraceId(); + + expect(result).to.be.null; + expect(getSegmentStub.called).to.be.false; + }); + + it('should return null when segment is not available', () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + getSegmentStub.returns(null); + + const result = getTraceId(); + + expect(result).to.be.null; + expect(getSegmentStub.calledOnce).to.be.true; + }); + + it('should return trace ID from segment', () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const mockSegment = { + trace_id: '1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e', + }; + getSegmentStub.returns(mockSegment); + + const result = getTraceId(); + + expect(result).to.equal('1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e'); + }); + + it('should return trace ID from root segment when segment has nested structure', () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const mockRootSegment = { + trace_id: '1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e', + }; + const mockSegment = { + segment: mockRootSegment, + }; + getSegmentStub.returns(mockSegment); + + const result = getTraceId(); + + expect(result).to.equal('1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e'); + }); + + it('should return null when segment exists but has no trace_id', () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const mockSegment = { + id: 'some-segment-id', + }; + getSegmentStub.returns(mockSegment); + + const result = getTraceId(); + + expect(result).to.be.undefined; + }); +}); From 41dcdf430d3443efbbb892e0f412652d4be0ba2f Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 6 Nov 2025 14:05:10 +0200 Subject: [PATCH 02/13] feat: add trace ID propagation for SQS messages and HTTP headers --- .../src/enrich-path-info-wrapper.js | 11 +++++++- packages/spacecat-shared-utils/src/index.d.ts | 8 ++++++ packages/spacecat-shared-utils/src/index.js | 2 +- packages/spacecat-shared-utils/src/sqs.js | 26 ++++++++++++++++--- packages/spacecat-shared-utils/src/xray.js | 24 +++++++++++++++++ 5 files changed, 65 insertions(+), 6 deletions(-) diff --git a/packages/spacecat-shared-http-utils/src/enrich-path-info-wrapper.js b/packages/spacecat-shared-http-utils/src/enrich-path-info-wrapper.js index 6e2be9278..ac473368e 100644 --- a/packages/spacecat-shared-http-utils/src/enrich-path-info-wrapper.js +++ b/packages/spacecat-shared-http-utils/src/enrich-path-info-wrapper.js @@ -13,14 +13,23 @@ export function enrichPathInfo(fn) { // export for testing return async (request, context) => { const [, route] = context?.pathInfo?.suffix?.split(/\/+/) || []; + const headers = request.headers.plain(); + context.pathInfo = { ...context.pathInfo, ...{ method: request.method.toUpperCase(), - headers: request.headers.plain(), + headers, route, }, }; + + // Extract and store traceId from x-trace-id header if present + const traceIdHeader = headers['x-trace-id']; + if (traceIdHeader) { + context.traceId = traceIdHeader; + } + return fn(request, context); }; } diff --git a/packages/spacecat-shared-utils/src/index.d.ts b/packages/spacecat-shared-utils/src/index.d.ts index 4f545fba7..d15da2f87 100644 --- a/packages/spacecat-shared-utils/src/index.d.ts +++ b/packages/spacecat-shared-utils/src/index.d.ts @@ -86,6 +86,14 @@ export function instrumentAWSClient(client: T): T; */ export function getTraceId(): string | null; +/** + * Adds the x-trace-id header to a headers object if a trace ID is available. + * @param headers - The headers object to augment + * @param context - The context object that may contain traceId + * @returns The headers object with x-trace-id added if available + */ +export function addTraceIdHeader(headers?: Record, context?: object): Record; + /** * Prepends 'https://' schema to the URL if it's not already present. * @param url - The URL to modify. diff --git a/packages/spacecat-shared-utils/src/index.js b/packages/spacecat-shared-utils/src/index.js index 9641d8481..6cc2d1dec 100644 --- a/packages/spacecat-shared-utils/src/index.js +++ b/packages/spacecat-shared-utils/src/index.js @@ -52,7 +52,7 @@ export { sqsWrapper } from './sqs.js'; export { sqsEventAdapter } from './sqs.js'; export { logWrapper } from './log-wrapper.js'; -export { instrumentAWSClient, getTraceId } from './xray.js'; +export { instrumentAWSClient, getTraceId, addTraceIdHeader } from './xray.js'; export { composeBaseURL, diff --git a/packages/spacecat-shared-utils/src/sqs.js b/packages/spacecat-shared-utils/src/sqs.js index 4c27d986e..0e74af63c 100644 --- a/packages/spacecat-shared-utils/src/sqs.js +++ b/packages/spacecat-shared-utils/src/sqs.js @@ -12,7 +12,7 @@ import { Response } from '@adobe/fetch'; import { SendMessageCommand, SQSClient } from '@aws-sdk/client-sqs'; -import { instrumentAWSClient } from './xray.js'; +import { instrumentAWSClient, getTraceId } from './xray.js'; import { hasText, isNonEmptyArray } from './functions.js'; import { isAWSLambda } from './runtimes.js'; @@ -46,8 +46,11 @@ class SQS { /** * Send a message to an SQS queue. For FIFO queues, messageGroupId is required. + * Automatically includes traceId in the message payload if available from: + * 1. The message itself (if explicitly set by caller, e.g. from context.traceId) + * 2. AWS X-Ray segment (current Lambda execution trace) * @param {string} queueUrl - The URL of the SQS queue. - * @param {object} message - The message body to send. + * @param {object} message - The message body to send. Can include traceId for propagation. * @param {string} messageGroupId - (Optional) The message group ID for FIFO queues. * @return {Promise} */ @@ -57,6 +60,15 @@ class SQS { timestamp: new Date().toISOString(), }; + // Add traceId to message payload if not already present + // Priority: 1) Explicit traceId in message (from context), 2) X-Ray traceId + if (!body.traceId) { + const traceId = getTraceId(); + if (traceId) { + body.traceId = traceId; + } + } + const params = { MessageBody: JSON.stringify(body), QueueUrl: queueUrl, @@ -71,7 +83,7 @@ class SQS { try { const data = await this.sqsClient.send(msgCommand); - this.log.debug(`Success, message sent. MessageID: ${data.MessageId}`); + this.log.debug(`Success, message sent. MessageID: ${data.MessageId}${body.traceId ? `, TraceID: ${body.traceId}` : ''}`); } catch (e) { const { type, code, message: msg } = e; this.log.error(`Message sent failed. Type: ${type}, Code: ${code}, Message: ${msg}`); @@ -95,6 +107,7 @@ export function sqsWrapper(fn) { /** * Wrapper to turn an SQS record into a function param * Inspired by https://github.com/adobe/helix-admin/blob/main/src/index.js#L108-L133 + * Extracts traceId from the message payload if present and stores it in context for propagation. * * @param {UniversalAction} fn * @returns {function(object, UniversalContext): Promise} @@ -124,7 +137,12 @@ export function sqsEventAdapter(fn) { try { message = JSON.parse(record.body); - log.debug(`Received message with id: ${record.messageId}`); + log.debug(`Received message with id: ${record.messageId}${message.traceId ? `, traceId: ${message.traceId}` : ''}`); + + // Store traceId in context if present in the message for downstream propagation + if (message.traceId) { + context.traceId = message.traceId; + } } catch (e) { log.warn('Function was not invoked properly, message body is not a valid JSON', e); return badRequest('Event does not contain a valid message body'); diff --git a/packages/spacecat-shared-utils/src/xray.js b/packages/spacecat-shared-utils/src/xray.js index a205072c2..d7abd277d 100644 --- a/packages/spacecat-shared-utils/src/xray.js +++ b/packages/spacecat-shared-utils/src/xray.js @@ -37,3 +37,27 @@ export function getTraceId() { const effectiveSegment = segment.segment || segment; return effectiveSegment.trace_id; } + +/** + * Adds the x-trace-id header to a headers object if a trace ID is available. + * Checks for traceId from: + * 1. Explicit context.traceId (from incoming HTTP request or SQS message) + * 2. AWS X-Ray segment (current Lambda execution) + * + * @param {object} headers - The headers object to augment + * @param {object} context - The context object that may contain traceId + * @returns {object} The headers object with x-trace-id added if available + */ +export function addTraceIdHeader(headers = {}, context = {}) { + // Priority: 1) context.traceId (propagated from incoming request), 2) X-Ray traceId + const traceId = context.traceId || getTraceId(); + + if (traceId) { + return { + ...headers, + 'x-trace-id': traceId, + }; + } + + return headers; +} From f870b34502c2f2ae485d7c060d57e6e65c79dca6 Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 6 Nov 2025 14:19:37 +0200 Subject: [PATCH 03/13] fix: resolve linting errors - trailing spaces and line length --- packages/spacecat-shared-utils/src/sqs.js | 2 +- packages/spacecat-shared-utils/src/xray.js | 6 +++--- packages/spacecat-shared-utils/test/log-wrapper.test.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/spacecat-shared-utils/src/sqs.js b/packages/spacecat-shared-utils/src/sqs.js index 0e74af63c..456bc7b42 100644 --- a/packages/spacecat-shared-utils/src/sqs.js +++ b/packages/spacecat-shared-utils/src/sqs.js @@ -138,7 +138,7 @@ export function sqsEventAdapter(fn) { try { message = JSON.parse(record.body); log.debug(`Received message with id: ${record.messageId}${message.traceId ? `, traceId: ${message.traceId}` : ''}`); - + // Store traceId in context if present in the message for downstream propagation if (message.traceId) { context.traceId = message.traceId; diff --git a/packages/spacecat-shared-utils/src/xray.js b/packages/spacecat-shared-utils/src/xray.js index d7abd277d..8988493bf 100644 --- a/packages/spacecat-shared-utils/src/xray.js +++ b/packages/spacecat-shared-utils/src/xray.js @@ -21,7 +21,7 @@ export function instrumentAWSClient(client) { * Extracts the trace ID from the current AWS X-Ray segment. * This function is designed to work in AWS Lambda environments where X-Ray tracing is enabled. * - * @returns {string|null} The trace ID if available, or null if not in AWS Lambda or no segment found + * @returns {string|null} The trace ID if available, or null if not in Lambda or no segment */ export function getTraceId() { if (!isAWSLambda()) { @@ -51,13 +51,13 @@ export function getTraceId() { export function addTraceIdHeader(headers = {}, context = {}) { // Priority: 1) context.traceId (propagated from incoming request), 2) X-Ray traceId const traceId = context.traceId || getTraceId(); - + if (traceId) { return { ...headers, 'x-trace-id': traceId, }; } - + return headers; } diff --git a/packages/spacecat-shared-utils/test/log-wrapper.test.js b/packages/spacecat-shared-utils/test/log-wrapper.test.js index 069fcfe92..de6becead 100644 --- a/packages/spacecat-shared-utils/test/log-wrapper.test.js +++ b/packages/spacecat-shared-utils/test/log-wrapper.test.js @@ -46,7 +46,7 @@ let mockContext; describe('logWrapper tests', () => { before(async () => { getTraceIdStub = sinon.stub().returns(null); - + logWrapper = await esmock('../src/log-wrapper.js', { '../src/xray.js': { getTraceId: getTraceIdStub, From d54af4ea7a7baece68709ebd57a15fdf27b82d6e Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 6 Nov 2025 14:24:57 +0200 Subject: [PATCH 04/13] fix: remove trailing spaces in enrich-path-info-wrapper.js --- .../src/enrich-path-info-wrapper.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/spacecat-shared-http-utils/src/enrich-path-info-wrapper.js b/packages/spacecat-shared-http-utils/src/enrich-path-info-wrapper.js index ac473368e..499932434 100644 --- a/packages/spacecat-shared-http-utils/src/enrich-path-info-wrapper.js +++ b/packages/spacecat-shared-http-utils/src/enrich-path-info-wrapper.js @@ -14,7 +14,7 @@ export function enrichPathInfo(fn) { // export for testing return async (request, context) => { const [, route] = context?.pathInfo?.suffix?.split(/\/+/) || []; const headers = request.headers.plain(); - + context.pathInfo = { ...context.pathInfo, ...{ @@ -23,13 +23,13 @@ export function enrichPathInfo(fn) { // export for testing route, }, }; - + // Extract and store traceId from x-trace-id header if present const traceIdHeader = headers['x-trace-id']; if (traceIdHeader) { context.traceId = traceIdHeader; } - + return fn(request, context); }; } From 11c8f7b631682d945b6d9e1ed0be5ee58ca0d0b1 Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 6 Nov 2025 14:35:41 +0200 Subject: [PATCH 05/13] test: add comprehensive tests for trace ID propagation features --- .../test/enrich-path-info-wrapper.test.js | 151 ++++++++++++++++++ .../spacecat-shared-utils/test/index.test.js | 2 + .../spacecat-shared-utils/test/sqs.test.js | 66 +++++++- .../spacecat-shared-utils/test/xray.test.js | 115 ++++++++++++- 4 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 packages/spacecat-shared-http-utils/test/enrich-path-info-wrapper.test.js diff --git a/packages/spacecat-shared-http-utils/test/enrich-path-info-wrapper.test.js b/packages/spacecat-shared-http-utils/test/enrich-path-info-wrapper.test.js new file mode 100644 index 000000000..84d724593 --- /dev/null +++ b/packages/spacecat-shared-http-utils/test/enrich-path-info-wrapper.test.js @@ -0,0 +1,151 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +/* eslint-env mocha */ +import { expect } from 'chai'; +import sinon from 'sinon'; + +import { enrichPathInfo } from '../src/enrich-path-info-wrapper.js'; + +describe('enrichPathInfo', () => { + let mockRequest; + let mockContext; + let mockFn; + + beforeEach(() => { + mockFn = sinon.stub().resolves({ status: 200 }); + + mockRequest = { + method: 'POST', + headers: { + plain: () => ({ + 'content-type': 'application/json', + 'user-agent': 'test-agent', + }), + }, + }; + + mockContext = { + pathInfo: { + suffix: '/api/test', + }, + }; + }); + + afterEach(() => { + sinon.restore(); + }); + + it('should enrich context with pathInfo including method, headers, and route', async () => { + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + expect(mockContext.pathInfo).to.deep.include({ + method: 'POST', + route: 'api', + }); + expect(mockContext.pathInfo.headers).to.deep.equal({ + 'content-type': 'application/json', + 'user-agent': 'test-agent', + }); + }); + + it('should extract traceId from x-trace-id header and store in context', async () => { + mockRequest.headers.plain = () => ({ + 'content-type': 'application/json', + 'x-trace-id': '1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e', + }); + + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + expect(mockContext.traceId).to.equal('1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e'); + }); + + it('should not set traceId in context when x-trace-id header is missing', async () => { + mockRequest.headers.plain = () => ({ + 'content-type': 'application/json', + }); + + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + expect(mockContext.traceId).to.be.undefined; + }); + + it('should handle case-sensitive x-trace-id header', async () => { + mockRequest.headers.plain = () => ({ + 'content-type': 'application/json', + 'X-Trace-Id': '1-different-case', + }); + + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + // Header keys should be lowercase + expect(mockContext.traceId).to.be.undefined; + }); + + it('should call the wrapped function with request and context', async () => { + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + expect(mockFn.calledOnce).to.be.true; + expect(mockFn.firstCall.args[0]).to.equal(mockRequest); + expect(mockFn.firstCall.args[1]).to.equal(mockContext); + }); + + it('should return the result from the wrapped function', async () => { + mockFn.resolves({ status: 201, body: 'created' }); + + const wrapper = enrichPathInfo(mockFn); + const result = await wrapper(mockRequest, mockContext); + + expect(result).to.deep.equal({ status: 201, body: 'created' }); + }); + + it('should handle empty pathInfo suffix', async () => { + mockContext.pathInfo.suffix = ''; + + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + expect(mockContext.pathInfo.route).to.be.undefined; + }); + + it('should handle missing pathInfo suffix', async () => { + mockContext.pathInfo = {}; + + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + expect(mockContext.pathInfo.route).to.be.undefined; + }); + + it('should convert method to uppercase', async () => { + mockRequest.method = 'get'; + + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + expect(mockContext.pathInfo.method).to.equal('GET'); + }); + + it('should handle complex route extraction', async () => { + mockContext.pathInfo.suffix = '/api/v1/users/123'; + + const wrapper = enrichPathInfo(mockFn); + await wrapper(mockRequest, mockContext); + + expect(mockContext.pathInfo.route).to.equal('api'); + }); +}); + diff --git a/packages/spacecat-shared-utils/test/index.test.js b/packages/spacecat-shared-utils/test/index.test.js index ea303beac..d3c6fa325 100644 --- a/packages/spacecat-shared-utils/test/index.test.js +++ b/packages/spacecat-shared-utils/test/index.test.js @@ -67,6 +67,8 @@ describe('Index Exports', () => { 'SPACECAT_USER_AGENT', 'isAWSLambda', 'instrumentAWSClient', + 'getTraceId', + 'addTraceIdHeader', 'getDateRanges', 'getLastNumberOfWeeks', 'resolveCanonicalUrl', diff --git a/packages/spacecat-shared-utils/test/sqs.test.js b/packages/spacecat-shared-utils/test/sqs.test.js index 0685445d6..7785f7ebd 100644 --- a/packages/spacecat-shared-utils/test/sqs.test.js +++ b/packages/spacecat-shared-utils/test/sqs.test.js @@ -102,7 +102,7 @@ describe('SQS', () => { await ctx.sqs.sendMessage(queueUrl, message); }).with(sqsWrapper)({}, context); - expect(logSpy).to.have.been.calledWith(`Success, message sent. MessageID: ${messageId}`); + expect(logSpy).to.have.been.calledWith(`Success, message sent. MessageID: ${messageId}`); }); }); @@ -280,5 +280,69 @@ describe('SQS', () => { ]); expect(firstSendArg.input.MessageGroupId).to.be.undefined; }); + + it('should include traceId in message when explicitly provided', async () => { + const action = wrap(async (req, ctx) => { + await ctx.sqs.sendMessage('queue-url', { key: 'value', traceId: '1-explicit-traceid' }); + }).with(sqsWrapper); + + await action({}, context); + + const firstSendArg = sendStub.getCall(0).args[0]; + const messageBody = JSON.parse(firstSendArg.input.MessageBody); + expect(messageBody.traceId).to.equal('1-explicit-traceid'); + }); + + it('should extract traceId from SQS message and store in context', async () => { + const ctx = { + log: console, + invocation: { + event: { + Records: [ + { + body: JSON.stringify({ id: '1234567890', traceId: '1-sqs-traceid' }), + messageId: 'abcd', + }, + ], + }, + }, + }; + + const testHandler = sandbox.spy(async (message, context) => { + expect(context.traceId).to.equal('1-sqs-traceid'); + return new Response('ok'); + }); + + const handler = sqsEventAdapter(testHandler); + await handler({}, ctx); + + expect(testHandler.calledOnce).to.be.true; + }); + + it('should not set context.traceId when message has no traceId', async () => { + const ctx = { + log: console, + invocation: { + event: { + Records: [ + { + body: JSON.stringify({ id: '1234567890' }), + messageId: 'abcd', + }, + ], + }, + }, + }; + + const testHandler = sandbox.spy(async (message, context) => { + expect(context.traceId).to.be.undefined; + return new Response('ok'); + }); + + const handler = sqsEventAdapter(testHandler); + await handler({}, ctx); + + expect(testHandler.calledOnce).to.be.true; + }); }); }); diff --git a/packages/spacecat-shared-utils/test/xray.test.js b/packages/spacecat-shared-utils/test/xray.test.js index c8e911263..75f0009ed 100644 --- a/packages/spacecat-shared-utils/test/xray.test.js +++ b/packages/spacecat-shared-utils/test/xray.test.js @@ -15,7 +15,7 @@ import { expect } from 'chai'; import sinon from 'sinon'; import AWSXray from 'aws-xray-sdk'; -import { instrumentAWSClient, getTraceId } from '../src/index.js'; +import { instrumentAWSClient, getTraceId, addTraceIdHeader } from '../src/index.js'; describe('instrumentClient', () => { let captureStub; @@ -123,3 +123,116 @@ describe('getTraceId', () => { expect(result).to.be.undefined; }); }); + +describe('addTraceIdHeader', () => { + let getSegmentStub; + + beforeEach(() => { + getSegmentStub = sinon.stub(AWSXray, 'getSegment'); + }); + + afterEach(() => { + sinon.restore(); + delete process.env.AWS_EXECUTION_ENV; + }); + + it('should add x-trace-id header when context.traceId is present', () => { + const headers = { 'content-type': 'application/json' }; + const context = { traceId: '1-context-traceid' }; + + const result = addTraceIdHeader(headers, context); + + expect(result).to.deep.equal({ + 'content-type': 'application/json', + 'x-trace-id': '1-context-traceid', + }); + }); + + it('should add x-trace-id header from X-Ray when context.traceId is not present', () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const mockSegment = { + trace_id: '1-xray-traceid', + }; + getSegmentStub.returns(mockSegment); + + const headers = { 'content-type': 'application/json' }; + const context = {}; + + const result = addTraceIdHeader(headers, context); + + expect(result).to.deep.equal({ + 'content-type': 'application/json', + 'x-trace-id': '1-xray-traceid', + }); + }); + + it('should prioritize context.traceId over X-Ray trace ID', () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const mockSegment = { + trace_id: '1-xray-traceid', + }; + getSegmentStub.returns(mockSegment); + + const headers = { 'content-type': 'application/json' }; + const context = { traceId: '1-context-traceid' }; + + const result = addTraceIdHeader(headers, context); + + expect(result).to.deep.equal({ + 'content-type': 'application/json', + 'x-trace-id': '1-context-traceid', + }); + expect(getSegmentStub.called).to.be.false; + }); + + it('should return original headers when no trace ID is available', () => { + delete process.env.AWS_EXECUTION_ENV; + + const headers = { 'content-type': 'application/json' }; + const context = {}; + + const result = addTraceIdHeader(headers, context); + + expect(result).to.deep.equal({ + 'content-type': 'application/json', + }); + }); + + it('should work with empty headers object', () => { + const headers = {}; + const context = { traceId: '1-test-traceid' }; + + const result = addTraceIdHeader(headers, context); + + expect(result).to.deep.equal({ + 'x-trace-id': '1-test-traceid', + }); + }); + + it('should work with no headers parameter', () => { + const context = { traceId: '1-test-traceid' }; + + const result = addTraceIdHeader(undefined, context); + + expect(result).to.deep.equal({ + 'x-trace-id': '1-test-traceid', + }); + }); + + it('should work with no context parameter', () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const mockSegment = { + trace_id: '1-xray-traceid', + }; + getSegmentStub.returns(mockSegment); + + const headers = { 'content-type': 'application/json' }; + + const result = addTraceIdHeader(headers, undefined); + + expect(result).to.deep.equal({ + 'content-type': 'application/json', + 'x-trace-id': '1-xray-traceid', + }); + }); +}); From 7f37b6fc15aa8bc4da1e3fd68aaefc02024fae7f Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 6 Nov 2025 14:36:54 +0200 Subject: [PATCH 06/13] test: fix SQS traceId tests to set AWS_EXECUTION_ENV --- packages/spacecat-shared-utils/test/sqs.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/spacecat-shared-utils/test/sqs.test.js b/packages/spacecat-shared-utils/test/sqs.test.js index 7785f7ebd..956428f5c 100644 --- a/packages/spacecat-shared-utils/test/sqs.test.js +++ b/packages/spacecat-shared-utils/test/sqs.test.js @@ -294,6 +294,8 @@ describe('SQS', () => { }); it('should extract traceId from SQS message and store in context', async () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const ctx = { log: console, invocation: { @@ -317,9 +319,12 @@ describe('SQS', () => { await handler({}, ctx); expect(testHandler.calledOnce).to.be.true; + delete process.env.AWS_EXECUTION_ENV; }); it('should not set context.traceId when message has no traceId', async () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const ctx = { log: console, invocation: { @@ -343,6 +348,7 @@ describe('SQS', () => { await handler({}, ctx); expect(testHandler.calledOnce).to.be.true; + delete process.env.AWS_EXECUTION_ENV; }); }); }); From 99a6077f5883b6ec776eefde2047bb75959632be Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 6 Nov 2025 14:41:22 +0200 Subject: [PATCH 07/13] fix: resolve variable shadowing in SQS tests --- packages/spacecat-shared-utils/test/sqs.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/spacecat-shared-utils/test/sqs.test.js b/packages/spacecat-shared-utils/test/sqs.test.js index 956428f5c..c04ef979b 100644 --- a/packages/spacecat-shared-utils/test/sqs.test.js +++ b/packages/spacecat-shared-utils/test/sqs.test.js @@ -310,8 +310,8 @@ describe('SQS', () => { }, }; - const testHandler = sandbox.spy(async (message, context) => { - expect(context.traceId).to.equal('1-sqs-traceid'); + const testHandler = sandbox.spy(async (message, handlerContext) => { + expect(handlerContext.traceId).to.equal('1-sqs-traceid'); return new Response('ok'); }); @@ -339,8 +339,8 @@ describe('SQS', () => { }, }; - const testHandler = sandbox.spy(async (message, context) => { - expect(context.traceId).to.be.undefined; + const testHandler = sandbox.spy(async (message, handlerContext) => { + expect(handlerContext.traceId).to.be.undefined; return new Response('ok'); }); From 23172f252149a645eb4e93a7651a31ac2023f30a Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 6 Nov 2025 14:45:53 +0200 Subject: [PATCH 08/13] fix: remove extra blank lines at end of test file --- .../test/enrich-path-info-wrapper.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/spacecat-shared-http-utils/test/enrich-path-info-wrapper.test.js b/packages/spacecat-shared-http-utils/test/enrich-path-info-wrapper.test.js index 84d724593..acbfd319f 100644 --- a/packages/spacecat-shared-http-utils/test/enrich-path-info-wrapper.test.js +++ b/packages/spacecat-shared-http-utils/test/enrich-path-info-wrapper.test.js @@ -148,4 +148,3 @@ describe('enrichPathInfo', () => { expect(mockContext.pathInfo.route).to.equal('api'); }); }); - From a99af94708d8028ecbe3c3c8deeeca4ae59b9656 Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 6 Nov 2025 14:55:04 +0200 Subject: [PATCH 09/13] test: add coverage for X-Ray auto-add traceId and silence X-Ray warnings --- .../spacecat-shared-utils/test/setup-env.js | 2 ++ .../spacecat-shared-utils/test/sqs.test.js | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/packages/spacecat-shared-utils/test/setup-env.js b/packages/spacecat-shared-utils/test/setup-env.js index 35acea728..4abcc1dc3 100644 --- a/packages/spacecat-shared-utils/test/setup-env.js +++ b/packages/spacecat-shared-utils/test/setup-env.js @@ -14,3 +14,5 @@ console.log('Forcing HTTP/1.1 for Adobe Fetch'); process.env.HELIX_FETCH_FORCE_HTTP1 = 'true'; process.env.AWS_ACCESS_KEY_ID = 'fake-key-id'; process.env.AWS_SECRET_ACCESS_KEY = 'fake-secret'; +// Silence AWS X-Ray context warnings in tests +process.env.AWS_XRAY_CONTEXT_MISSING = 'IGNORE_ERROR'; diff --git a/packages/spacecat-shared-utils/test/sqs.test.js b/packages/spacecat-shared-utils/test/sqs.test.js index c04ef979b..3a740e5d4 100644 --- a/packages/spacecat-shared-utils/test/sqs.test.js +++ b/packages/spacecat-shared-utils/test/sqs.test.js @@ -20,6 +20,7 @@ import sinonChai from 'sinon-chai'; import chaiAsPromised from 'chai-as-promised'; import nock from 'nock'; import crypto from 'crypto'; +import AWSXray from 'aws-xray-sdk'; import { sqsEventAdapter, sqsWrapper } from '../src/sqs.js'; use(sinonChai); @@ -293,6 +294,26 @@ describe('SQS', () => { expect(messageBody.traceId).to.equal('1-explicit-traceid'); }); + it('should automatically add traceId from X-Ray when not explicitly provided', async () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const getSegmentStub = sandbox.stub(AWSXray, 'getSegment').returns({ + trace_id: '1-xray-auto-traceid', + }); + + const action = wrap(async (req, ctx) => { + await ctx.sqs.sendMessage('queue-url', { key: 'value' }); + }).with(sqsWrapper); + + await action({}, context); + + const firstSendArg = sendStub.getCall(0).args[0]; + const messageBody = JSON.parse(firstSendArg.input.MessageBody); + expect(messageBody.traceId).to.equal('1-xray-auto-traceid'); + + getSegmentStub.restore(); + delete process.env.AWS_EXECUTION_ENV; + }); + it('should extract traceId from SQS message and store in context', async () => { process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; From cad0607ad90430b284e9526b77f93863029dfc5e Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 13 Nov 2025 18:36:03 +0200 Subject: [PATCH 10/13] feat: add Jobs Dispatcher opt-out for trace propagation - Handle explicit traceId: null to opt-out of trace propagation - Jobs Dispatcher can send individual job messages without shared traceId - Each downstream Lambda will generate its own X-Ray trace - Add test case for opt-out behavior --- packages/spacecat-shared-utils/src/sqs.js | 21 +++++++++++--- .../spacecat-shared-utils/test/sqs.test.js | 29 +++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/spacecat-shared-utils/src/sqs.js b/packages/spacecat-shared-utils/src/sqs.js index 456bc7b42..5d555157f 100644 --- a/packages/spacecat-shared-utils/src/sqs.js +++ b/packages/spacecat-shared-utils/src/sqs.js @@ -49,8 +49,13 @@ class SQS { * Automatically includes traceId in the message payload if available from: * 1. The message itself (if explicitly set by caller, e.g. from context.traceId) * 2. AWS X-Ray segment (current Lambda execution trace) + * + * Special handling for Jobs Dispatcher and similar scenarios: + * - Set traceId to null to opt-out of trace propagation (each worker gets its own trace) + * * @param {string} queueUrl - The URL of the SQS queue. - * @param {object} message - The message body to send. Can include traceId for propagation. + * @param {object} message - The message body to send. + * Can include traceId for propagation or set to null to opt-out. * @param {string} messageGroupId - (Optional) The message group ID for FIFO queues. * @return {Promise} */ @@ -60,14 +65,22 @@ class SQS { timestamp: new Date().toISOString(), }; - // Add traceId to message payload if not already present - // Priority: 1) Explicit traceId in message (from context), 2) X-Ray traceId - if (!body.traceId) { + // Handle traceId based on explicit setting or auto-generation + // Three cases: + // 1. Property not in message → auto-add X-Ray traceId + // 2. Property set to null → explicit opt-out (e.g., Jobs Dispatcher) + // 3. Property has a value → use that value + if (!('traceId' in message)) { + // Case 1: No traceId property - auto-add X-Ray trace const traceId = getTraceId(); if (traceId) { body.traceId = traceId; } + } else if (message.traceId === null) { + // Case 2: Explicitly null - opt-out of trace propagation + delete body.traceId; } + // Case 3: Has a value - already in body from spread, keep it const params = { MessageBody: JSON.stringify(body), diff --git a/packages/spacecat-shared-utils/test/sqs.test.js b/packages/spacecat-shared-utils/test/sqs.test.js index 3a740e5d4..6802ba963 100644 --- a/packages/spacecat-shared-utils/test/sqs.test.js +++ b/packages/spacecat-shared-utils/test/sqs.test.js @@ -371,5 +371,34 @@ describe('SQS', () => { expect(testHandler.calledOnce).to.be.true; delete process.env.AWS_EXECUTION_ENV; }); + + it('should not include traceId when explicitly set to null (Jobs Dispatcher opt-out)', async () => { + process.env.AWS_EXECUTION_ENV = 'AWS_Lambda_nodejs18.x'; + const getSegmentStub = sandbox.stub(AWSXray, 'getSegment').returns({ + trace_id: '1-xray-dispatcher-traceid', + }); + + const action = wrap(async (req, ctx) => { + // Jobs Dispatcher explicitly opts out of trace propagation + await ctx.sqs.sendMessage('queue-url', { + type: 'audit', + siteId: 'site-001', + traceId: null, // Explicit opt-out + }); + }).with(sqsWrapper); + + await action({}, context); + + const firstSendArg = sendStub.getCall(0).args[0]; + const messageBody = JSON.parse(firstSendArg.input.MessageBody); + + // traceId should NOT be in the message + expect(messageBody).to.not.have.property('traceId'); + expect(messageBody.type).to.equal('audit'); + expect(messageBody.siteId).to.equal('site-001'); + + getSegmentStub.restore(); + delete process.env.AWS_EXECUTION_ENV; + }); }); }); From 2685cf38a9669e40c8e5f34b40e70f84afb9f301 Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Thu, 13 Nov 2025 18:43:30 +0200 Subject: [PATCH 11/13] fix: remove trailing spaces in JSDoc comments --- packages/spacecat-shared-utils/src/sqs.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/spacecat-shared-utils/src/sqs.js b/packages/spacecat-shared-utils/src/sqs.js index 5d555157f..34507ffef 100644 --- a/packages/spacecat-shared-utils/src/sqs.js +++ b/packages/spacecat-shared-utils/src/sqs.js @@ -49,12 +49,12 @@ class SQS { * Automatically includes traceId in the message payload if available from: * 1. The message itself (if explicitly set by caller, e.g. from context.traceId) * 2. AWS X-Ray segment (current Lambda execution trace) - * + * * Special handling for Jobs Dispatcher and similar scenarios: * - Set traceId to null to opt-out of trace propagation (each worker gets its own trace) - * + * * @param {string} queueUrl - The URL of the SQS queue. - * @param {object} message - The message body to send. + * @param {object} message - The message body to send. * Can include traceId for propagation or set to null to opt-out. * @param {string} messageGroupId - (Optional) The message group ID for FIFO queues. * @return {Promise} From ec85750f811f7d118733f467fe7dd6a9be62e376 Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Wed, 19 Nov 2025 09:10:46 +0200 Subject: [PATCH 12/13] feat(LLMO-1023): enhance context.log directly for automatic trace ID logging - Modify logWrapper to enhance context.log in place instead of creating contextualLog - No code changes needed in consuming services - existing context.log calls automatically include trace IDs - Set context.contextualLog as an alias for backward compatibility check - Update tests to verify context.log is enhanced directly - Update README to clarify no code changes are needed This addresses reviewer feedback to make trace ID inclusion transparent without requiring code changes throughout the codebase. --- packages/spacecat-shared-utils/README.md | 12 +-- .../spacecat-shared-utils/src/log-wrapper.js | 19 ++-- .../test/log-wrapper.test.js | 91 ++++++++++++------- 3 files changed, 74 insertions(+), 48 deletions(-) diff --git a/packages/spacecat-shared-utils/README.md b/packages/spacecat-shared-utils/README.md index 6a3e213fa..5ef27baeb 100644 --- a/packages/spacecat-shared-utils/README.md +++ b/packages/spacecat-shared-utils/README.md @@ -52,7 +52,7 @@ The `logWrapper` enhances your Lambda function logs by automatically prepending ### Features - Automatically extracts AWS X-Ray trace ID - Includes jobId from message when available -- Creates `context.contextualLog` with enhanced logging methods +- Enhances `context.log` directly - **no code changes needed** - Works seamlessly with existing log levels (info, error, debug, warn, trace, etc.) ### Usage @@ -61,10 +61,10 @@ The `logWrapper` enhances your Lambda function logs by automatically prepending import { logWrapper, sqsEventAdapter } from '@adobe/spacecat-shared-utils'; async function run(message, context) { - const { contextualLog } = context; + const { log } = context; - // Use contextualLog instead of log for enhanced logging - contextualLog.info('Processing started'); + // Use context.log as usual - trace IDs are added automatically + log.info('Processing started'); // Output: [jobId=xxx] [traceId=1-xxx-xxx] Processing started } @@ -77,9 +77,7 @@ export const main = wrap(run) .with(helixStatus); ``` -**Important:** Use `context.contextualLog` instead of `context.log` in your functions to get trace ID and job ID in your logs. - -For detailed integration instructions, see [LOG_WRAPPER_INTEGRATION_GUIDE.md](../../LOG_WRAPPER_INTEGRATION_GUIDE.md). +**Note:** The `logWrapper` enhances `context.log` directly. All existing code using `context.log` will automatically include trace IDs and job IDs in logs without any code changes. ## SQS Event Adapter diff --git a/packages/spacecat-shared-utils/src/log-wrapper.js b/packages/spacecat-shared-utils/src/log-wrapper.js index 708132cbd..1f558ef9a 100644 --- a/packages/spacecat-shared-utils/src/log-wrapper.js +++ b/packages/spacecat-shared-utils/src/log-wrapper.js @@ -20,16 +20,15 @@ import { getTraceId } from './xray.js'; * The wrapper checks if a `log` object exists in the `context` and whether the `message` * contains a `jobId`. It also extracts the AWS X-Ray trace ID if available. If found, log * methods (e.g., `info`, `error`, etc.) will prepend the `jobId` and/or `traceId` to all log - * statements where `context.contextualLog` is used. If neither is found, logging will remain - * unchanged. + * statements. All existing code using `context.log` will automatically include these markers. * * @param {function} fn - The original function to be wrapped, called with the provided * message and context after logging enhancement. * @returns {function(object, object): Promise} - A wrapped function that enhances * logging and returns the result of the original function. * - * `context.contextualLog` will include logging methods with `jobId` and/or `traceId` prefixed, - * or fall back to the existing `log` object if neither is provided. + * `context.log` will be enhanced in place to include `jobId` and/or `traceId` prefixed to all + * log messages. No code changes needed - existing `context.log` calls work automatically. */ export function logWrapper(fn) { return async (message, context) => { @@ -50,24 +49,24 @@ export function logWrapper(fn) { markers.push(`[traceId=${traceId}]`); } - // If we have markers, enhance the log object + // If we have markers, enhance the log object directly if (markers.length > 0) { const markerString = markers.join(' '); // Define log levels const logLevels = ['info', 'error', 'debug', 'warn', 'trace', 'verbose', 'silly', 'fatal']; - // Enhance the log object to include markers in all log statements - context.contextualLog = logLevels.reduce((accumulator, level) => { + // Enhance context.log directly to include markers in all log statements + context.log = logLevels.reduce((accumulator, level) => { if (typeof log[level] === 'function') { accumulator[level] = (...args) => log[level](markerString, ...args); } return accumulator; }, {}); - } else { - log.debug('No jobId or traceId found. Log entries will be recorded without additional context.'); - context.contextualLog = log; } + + // Mark that we've processed this context + context.contextualLog = context.log; } return fn(message, context); diff --git a/packages/spacecat-shared-utils/test/log-wrapper.test.js b/packages/spacecat-shared-utils/test/log-wrapper.test.js index de6becead..48f1d2e78 100644 --- a/packages/spacecat-shared-utils/test/log-wrapper.test.js +++ b/packages/spacecat-shared-utils/test/log-wrapper.test.js @@ -83,52 +83,43 @@ describe('logWrapper tests', () => { sinon.resetHistory(); }); - it('should call the original function with the provided message and context, and update the context with contextualLog object', async () => { + it('should call the original function with the provided message and context', async () => { const wrappedFn = logWrapper(mockFnFromSqs); await wrappedFn(message, mockContext); // Verify the original function is called with the correct parameters expect(mockFnFromSqs.calledWith(message, mockContext)).to.be.true; - - // Verify the context is updated correctly - expect(mockContext).to.have.property('contextualLog'); - expect(mockContext.contextualLog).to.be.an('object'); }); - it('should handle empty messages and assign context.log to context.contextualLog', async () => { + it('should handle empty messages without errors', async () => { const wrappedFn = logWrapper(mockFnFromSqs); // Test with empty message await wrappedFn({}, mockContext); expect(mockFnFromSqs.calledWith({}, mockContext)).to.be.true; - expect(mockContext).to.have.property('contextualLog'); - expect(mockContext.contextualLog).to.be.an('object'); - expect(mockContext.contextualLog).to.equal(mockContext.log); }); - it('should handle null messages and assign context.log to context.contextualLog', async () => { + it('should handle null messages without errors', async () => { const wrappedFn = logWrapper(mockFnFromSqs); // Test with null message await wrappedFn(null, mockContext); expect(mockFnFromSqs.calledWith(null, mockContext)).to.be.true; - expect(mockContext).to.have.property('contextualLog'); - expect(mockContext.contextualLog).to.be.an('object'); - expect(mockContext.contextualLog).to.equal(mockContext.log); }); logLevels.forEach((level) => { it(`should call ${level} log method with correct parameters when jobId is present`, async () => { + const originalLog = mockContext.log; const wrappedFn = logWrapper(mockFnFromSqs); await wrappedFn(message, mockContext); - // Log something to test the wrapper - mockContext.contextualLog[level](`${level} log`); + // Log something to test the wrapper using context.log + mockContext.log[level](`${level} log`); // Verify that the jobId is included in the log statement - const logArgs = mockContext.log[level].getCall(0).args[0]; + const logArgs = originalLog[level].getCall(0).args[0]; expect(logArgs).to.contain(`[jobId=${message.jobId}]`); }); }); @@ -140,27 +131,28 @@ describe('logWrapper tests', () => { // Call without a jobId await wrappedFn({}, mockContext); - // Log something to test the wrapper - mockContext.contextualLog[level](`${level} log`); + // Log something to test the wrapper using context.log + mockContext.log[level](`${level} log`); // Verify that the jobIdMarker is not included in the log statement const logArgs = mockContext.log[level].getCall(0).args[0]; - expect(logArgs).to.not.contain('[jobId='); + expect(logArgs).to.equal(`${level} log`); }); }); logLevels.forEach((level) => { it(`should call ${level} log method with traceId when available`, async () => { getTraceIdStub.returns('1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e'); + const originalLog = mockContext.log; const wrappedFn = logWrapper(mockFnFromSqs); await wrappedFn(message, mockContext); - // Log something to test the wrapper - mockContext.contextualLog[level](`${level} log`); + // Log something to test the wrapper using context.log + mockContext.log[level](`${level} log`); // Verify that the traceId is included in the log statement - const logArgs = mockContext.log[level].getCall(0).args[0]; + const logArgs = originalLog[level].getCall(0).args[0]; expect(logArgs).to.contain('[traceId=1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e]'); }); }); @@ -168,15 +160,16 @@ describe('logWrapper tests', () => { logLevels.forEach((level) => { it(`should call ${level} log method with both jobId and traceId when both are available`, async () => { getTraceIdStub.returns('1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e'); + const originalLog = mockContext.log; const wrappedFn = logWrapper(mockFnFromSqs); await wrappedFn(message, mockContext); - // Log something to test the wrapper - mockContext.contextualLog[level](`${level} log`); + // Log something to test the wrapper using context.log + mockContext.log[level](`${level} log`); // Verify that both jobId and traceId are included in the log statement - const logArgs = mockContext.log[level].getCall(0).args[0]; + const logArgs = originalLog[level].getCall(0).args[0]; expect(logArgs).to.contain(`[jobId=${message.jobId}]`); expect(logArgs).to.contain('[traceId=1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e]'); }); @@ -184,26 +177,62 @@ describe('logWrapper tests', () => { it('should not include traceId when getTraceId returns null', async () => { getTraceIdStub.returns(null); + const originalLog = mockContext.log; const wrappedFn = logWrapper(mockFnFromSqs); await wrappedFn(message, mockContext); - // Log something to test the wrapper - mockContext.contextualLog.info('info log'); + // Log something to test the wrapper using context.log + mockContext.log.info('info log'); // Verify that the traceId is not included in the log statement - const logArgs = mockContext.log.info.getCall(0).args[0]; + const logArgs = originalLog.info.getCall(0).args[0]; expect(logArgs).to.not.contain('[traceId='); expect(logArgs).to.contain(`[jobId=${message.jobId}]`); }); - it('should assign context.log to context.contextualLog when neither jobId nor traceId are available', async () => { + it('should not modify context.log when neither jobId nor traceId are available', async () => { getTraceIdStub.returns(null); + const originalLog = mockContext.log; const wrappedFn = logWrapper(mockFnFromSqs); await wrappedFn({}, mockContext); - expect(mockContext).to.have.property('contextualLog'); - expect(mockContext.contextualLog).to.equal(mockContext.log); + // context.log should remain unchanged + expect(mockContext.log).to.equal(originalLog); }); + + // Tests to verify context.log is enhanced directly (main feature) + logLevels.forEach((level) => { + it(`should enhance context.log.${level} directly with jobId and traceId`, async () => { + getTraceIdStub.returns('1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e'); + const originalLog = mockContext.log; + const wrappedFn = logWrapper(mockFnFromSqs); + + await wrappedFn(message, mockContext); + + // Verify that context.log is a new enhanced object, not the original + expect(mockContext.log).to.not.equal(originalLog); + + // Log something using context.log (not contextualLog) + mockContext.log[level](`${level} log`); + + // Verify that the original log method was called with markers + const logArgs = originalLog[level].getCall(0).args[0]; + expect(logArgs).to.contain(`[jobId=${message.jobId}]`); + expect(logArgs).to.contain('[traceId=1-5e8e8e8e-5e8e8e8e5e8e8e8e5e8e8e8e]'); + }); + }); + + it('should keep context.log unchanged when no jobId or traceId is available', async () => { + getTraceIdStub.returns(null); + const originalLog = mockContext.log; + const wrappedFn = logWrapper(mockFnFromSqs); + + await wrappedFn({}, mockContext); + + // context.log should remain unchanged + expect(mockContext.log).to.equal(originalLog); + }); + }); From 5c9d66e44b5ee17ae4638edd56d8d7002c29b1e2 Mon Sep 17 00:00:00 2001 From: Alexandru Tudoran Date: Wed, 19 Nov 2025 09:22:10 +0200 Subject: [PATCH 13/13] fix: remove padded blank line in log-wrapper test --- packages/spacecat-shared-utils/test/log-wrapper.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/spacecat-shared-utils/test/log-wrapper.test.js b/packages/spacecat-shared-utils/test/log-wrapper.test.js index 48f1d2e78..83a37d25a 100644 --- a/packages/spacecat-shared-utils/test/log-wrapper.test.js +++ b/packages/spacecat-shared-utils/test/log-wrapper.test.js @@ -234,5 +234,4 @@ describe('logWrapper tests', () => { // context.log should remain unchanged expect(mockContext.log).to.equal(originalLog); }); - });