From 518dd73b182023257f40d54f9042c2cd7e6bd116 Mon Sep 17 00:00:00 2001 From: liuyunn Date: Thu, 26 Dec 2024 10:17:32 -0800 Subject: [PATCH 1/4] fix trace context pattern and delete trace id. --- src/async_local_storage.ts | 2 -- src/execution_context.ts | 3 +-- src/logger.ts | 4 ---- test/async_local_storage.ts | 2 -- test/execution_context.ts | 3 --- test/logger.ts | 3 --- 6 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/async_local_storage.ts b/src/async_local_storage.ts index 25cd64db2..ddad69e6e 100644 --- a/src/async_local_storage.ts +++ b/src/async_local_storage.ts @@ -6,7 +6,6 @@ import type {AsyncLocalStorage} from 'node:async_hooks'; export interface ExecutionContext { executionId?: string; - traceId?: string; spanId?: string; } @@ -32,7 +31,6 @@ export async function asyncLocalStorageMiddleware( asyncLocalStorage.run( { executionId: req.executionId, - traceId: req.traceId, spanId: req.spanId, }, () => { diff --git a/src/execution_context.ts b/src/execution_context.ts index dcb199a3d..22665213e 100644 --- a/src/execution_context.ts +++ b/src/execution_context.ts @@ -5,7 +5,7 @@ const FUNCTION_EXECUTION_ID_HEADER_KEY = 'function-execution-id'; const TRACE_CONTEXT_HEADER_KEY = 'X-Cloud-Trace-Context'; const TRACE_CONTEXT_PATTERN = - /^(?\w+)\/(?\d+);o=(?.+)$/; + /^(?\w+)\/(?\d+)(?:;o=(?.+))?$/; function generateExecutionId() { const timestampPart = Date.now().toString(36).slice(-6); @@ -29,7 +29,6 @@ export const executionContextMiddleware = ( const match = cloudTraceContext.match(TRACE_CONTEXT_PATTERN); if (match?.groups) { const {traceId, spanId} = match.groups; - req.traceId = traceId; req.spanId = spanId; } } diff --git a/src/logger.ts b/src/logger.ts index b09416be9..d054e6bd7 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -18,7 +18,6 @@ import {getCurrentContext, ExecutionContext} from './async_local_storage'; import {Buffer} from 'buffer'; export const EXECUTION_CONTEXT_LABELS_KEY = 'logging.googleapis.com/labels'; -export const EXECUTION_CONTEXT_TRACE_KEY = 'logging.googleapis.com/trace'; export const EXECUTION_CONTEXT_SPAN_ID_KEY = 'logging.googleapis.com/spanId'; const SEVERITY = 'severity'; @@ -132,7 +131,6 @@ export function getModifiedData( let dataWithContext: { message: string | Uint8Array; 'logging.googleapis.com/labels': {execution_id: string | undefined}; - 'logging.googleapis.com/trace': string | undefined; 'logging.googleapis.com/spanId': string | undefined; severity?: string | undefined; }; @@ -155,7 +153,6 @@ function getTextWithContext( return { message: data, [EXECUTION_CONTEXT_LABELS_KEY]: {execution_id: context.executionId}, - [EXECUTION_CONTEXT_TRACE_KEY]: context.traceId, [EXECUTION_CONTEXT_SPAN_ID_KEY]: context.spanId, }; } @@ -169,7 +166,6 @@ function getJSONWithContext(json: any, context: ExecutionContext) { } return { ...json, - [EXECUTION_CONTEXT_TRACE_KEY]: context.traceId, [EXECUTION_CONTEXT_SPAN_ID_KEY]: context.spanId, }; } diff --git a/test/async_local_storage.ts b/test/async_local_storage.ts index 5c863dd7b..2cc15a95d 100644 --- a/test/async_local_storage.ts +++ b/test/async_local_storage.ts @@ -15,7 +15,6 @@ describe('asyncLocalStorageMiddleware', () => { const req = { body: 'test body', executionId: 'testExecutionId', - traceId: 'testtrace', spanId: 'testSpanId', }; let executionContext; @@ -25,7 +24,6 @@ describe('asyncLocalStorageMiddleware', () => { assert(executionContext); assert.strictEqual(executionContext.executionId, req.executionId); assert.strictEqual(executionContext.spanId, req.spanId); - assert.strictEqual(executionContext.traceId, req.traceId); }; await asyncLocalStorageMiddleware( diff --git a/test/execution_context.ts b/test/execution_context.ts index f92f01ab7..4df83f1fb 100644 --- a/test/execution_context.ts +++ b/test/execution_context.ts @@ -31,7 +31,6 @@ describe('executionContextMiddleware', () => { assert.strictEqual(req.executionId, validExecutionId); assert.strictEqual(req.spanId, testSpanId); - assert.strictEqual(req.traceId, testTrace); }); it('generates execution ID if not in header', () => { @@ -41,7 +40,6 @@ describe('executionContextMiddleware', () => { assert(req.executionId); assert.strictEqual(req.spanId, testSpanId); - assert.strictEqual(req.traceId, testTrace); }); it('req trace undefined if not in header', () => { @@ -51,6 +49,5 @@ describe('executionContextMiddleware', () => { assert(req.executionId); assert.strictEqual(req.spanId, undefined); - assert.strictEqual(req.traceId, undefined); }); }); diff --git a/test/logger.ts b/test/logger.ts index 52462cba7..9b163a777 100644 --- a/test/logger.ts +++ b/test/logger.ts @@ -47,14 +47,12 @@ describe('getModifiedData', () => { const sampleUint8Arr = new Uint8Array(Buffer.from(sampleText)); const expectedExecutionContext = { executionId: 'testExecutionId', - traceId: 'testTraceId', spanId: 'testSpanId', }; const expectedMetadata = { 'logging.googleapis.com/labels': { execution_id: 'testExecutionId', }, - 'logging.googleapis.com/trace': 'testTraceId', 'logging.googleapis.com/spanId': 'testSpanId', }; const expectedTextOutput = @@ -102,7 +100,6 @@ describe('getModifiedData', () => { user_label_1: 'value_1', execution_id: 'testExecutionId', }, - 'logging.googleapis.com/trace': 'testTraceId', 'logging.googleapis.com/spanId': 'testSpanId', }) + '\n'; const modifiedData = getModifiedData(data); From 2a0fd6bcc8fe79bbea966f3491f3a970ebb73498 Mon Sep 17 00:00:00 2001 From: Yun Liu Date: Thu, 26 Dec 2024 11:15:35 -0800 Subject: [PATCH 2/4] fix: remove trace id and respect logging span id field. --- src/execution_context.ts | 3 +-- src/functions.ts | 4 ---- src/logger.ts | 8 ++++---- test/logger.ts | 19 +++++++++++++++++++ 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/execution_context.ts b/src/execution_context.ts index 22665213e..252441466 100644 --- a/src/execution_context.ts +++ b/src/execution_context.ts @@ -28,8 +28,7 @@ export const executionContextMiddleware = ( if (cloudTraceContext) { const match = cloudTraceContext.match(TRACE_CONTEXT_PATTERN); if (match?.groups) { - const {traceId, spanId} = match.groups; - req.spanId = spanId; + req.spanId = match.groups.spanId; } } next(); diff --git a/src/functions.ts b/src/functions.ts index 66a4f3417..eae2e3d10 100644 --- a/src/functions.ts +++ b/src/functions.ts @@ -36,10 +36,6 @@ export interface Request extends ExpressRequest { * Request-specified execution ID. */ executionId?: string; - /** - * Cloud Trace trace ID. - */ - traceId?: string; /** * Cloud Trace span ID. */ diff --git a/src/logger.ts b/src/logger.ts index d054e6bd7..6f81b4cba 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -164,10 +164,10 @@ function getJSONWithContext(json: any, context: ExecutionContext) { } else { json[EXECUTION_CONTEXT_LABELS_KEY] = {execution_id: context.executionId}; } - return { - ...json, - [EXECUTION_CONTEXT_SPAN_ID_KEY]: context.spanId, - }; + if (!(EXECUTION_CONTEXT_SPAN_ID_KEY in json)) { + json[EXECUTION_CONTEXT_SPAN_ID_KEY] = context.spanId; + } + return json; } function processData(data: Uint8Array | string, encoding?: BufferEncoding) { diff --git a/test/logger.ts b/test/logger.ts index 9b163a777..eae8ce146 100644 --- a/test/logger.ts +++ b/test/logger.ts @@ -106,6 +106,25 @@ describe('getModifiedData', () => { assert.equal(modifiedData, expectedOutput); }); + it('json with user span id', () => { + const data = JSON.stringify({ + text: 'default text.', + component: 'arbitrary-property', + 'logging.googleapis.com/spanId': 'mySpanId', + }); + const expectedOutput = + JSON.stringify({ + text: 'default text.', + component: 'arbitrary-property', + 'logging.googleapis.com/spanId': 'mySpanId', + 'logging.googleapis.com/labels': { + execution_id: 'testExecutionId', + }, + }) + '\n'; + const modifiedData = getModifiedData(data); + assert.equal(modifiedData, expectedOutput); + }); + it('uint8array', () => { const modifiedData = getModifiedData(sampleUint8Arr); assert.equal(modifiedData, expectedTextOutput); From 3738c01dd854a63ad8c7e2a8b44c754f8a24586c Mon Sep 17 00:00:00 2001 From: Yun Liu Date: Thu, 26 Dec 2024 11:26:23 -0800 Subject: [PATCH 3/4] fix doc --- docs/generated/api.json | 27 --------------------------- docs/generated/api.md.api.md | 1 - 2 files changed, 28 deletions(-) diff --git a/docs/generated/api.json b/docs/generated/api.json index eb7d5885e..5543bdc8a 100644 --- a/docs/generated/api.json +++ b/docs/generated/api.json @@ -1952,33 +1952,6 @@ "startIndex": 1, "endIndex": 2 } - }, - { - "kind": "PropertySignature", - "canonicalReference": "@google-cloud/functions-framework!Request_2#traceId:member", - "docComment": "/**\n * Cloud Trace trace ID.\n */\n", - "excerptTokens": [ - { - "kind": "Content", - "text": "traceId?: " - }, - { - "kind": "Content", - "text": "string" - }, - { - "kind": "Content", - "text": ";" - } - ], - "isReadonly": false, - "isOptional": true, - "releaseTag": "Public", - "name": "traceId", - "propertyTypeTokenRange": { - "startIndex": 1, - "endIndex": 2 - } } ], "extendsTokenRanges": [ diff --git a/docs/generated/api.md.api.md b/docs/generated/api.md.api.md index 32386e50a..d9c05b63e 100644 --- a/docs/generated/api.md.api.md +++ b/docs/generated/api.md.api.md @@ -116,7 +116,6 @@ interface Request_2 extends Request_3 { executionId?: string; rawBody?: Buffer; spanId?: string; - traceId?: string; } export { Request_2 as Request } From 5b128d65574e2268f46484eadb3fc07585dfb25d Mon Sep 17 00:00:00 2001 From: Yun Liu Date: Thu, 26 Dec 2024 11:43:38 -0800 Subject: [PATCH 4/4] add test --- test/execution_context.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/execution_context.ts b/test/execution_context.ts index 4df83f1fb..9b304aed8 100644 --- a/test/execution_context.ts +++ b/test/execution_context.ts @@ -19,13 +19,15 @@ describe('executionContextMiddleware', () => { const testSpanId = '123'; const testTrace = 'testtrace'; const validExecutionId = 'xn1h9xdgv6zw'; - const headers = { - 'X-Cloud-Trace-Context': `${testTrace}/${testSpanId};o=1`, - 'function-execution-id': validExecutionId, - }; it('uses execution ID in header', () => { - const req = createRequest({}, headers); + const req = createRequest( + {}, + { + 'X-Cloud-Trace-Context': `${testTrace}/${testSpanId};o=1`, + 'function-execution-id': validExecutionId, + } + ); executionContextMiddleware(req as Request, {} as Response, next); @@ -34,7 +36,10 @@ describe('executionContextMiddleware', () => { }); it('generates execution ID if not in header', () => { - const req = createRequest({}, headers); + const req = createRequest( + {}, + {'X-Cloud-Trace-Context': `${testTrace}/${testSpanId}`} + ); executionContextMiddleware(req as Request, {} as Response, next);