From 21924c87e463085307437c0a461bb598dcd38a9b Mon Sep 17 00:00:00 2001 From: ijemmy Date: Wed, 25 May 2022 17:00:17 +0200 Subject: [PATCH 01/15] fix(all): replicate issue on node 12 --- .github/workflows/run-e2e-tests.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/run-e2e-tests.yml b/.github/workflows/run-e2e-tests.yml index b6420d71f0..6a7c7c46d0 100644 --- a/.github/workflows/run-e2e-tests.yml +++ b/.github/workflows/run-e2e-tests.yml @@ -1,6 +1,10 @@ name: run-e2e-tests on: workflow_dispatch: {} + # TODO: remove this once debugging completed + push: + branches: + - ijemmy/build-on-all-node-versions jobs: example-and-package-check: runs-on: ubuntu-latest @@ -47,15 +51,15 @@ jobs: contents: read strategy: matrix: - package: [logger, metrics, tracer] + package: [logger] version: [12, 14, 16] steps: - name: "Checkout" uses: actions/checkout@v3 - - name: "Use NodeJS 16" + - name: "Use NodeJS" uses: actions/setup-node@v3 with: - node-version: 16 + node-version: ${{ matrix.version }} - name: "Install npm@8.x" run: npm i -g npm@next-8 - name: "Install monorepo packages" From 2e7cbddac295c8cbe664dce4b53652f025b199c3 Mon Sep 17 00:00:00 2001 From: ijemmy Date: Thu, 26 May 2022 09:54:42 +0200 Subject: [PATCH 02/15] build: switch to nodejs 12 --- examples/cdk/tsconfig.json | 4 ++-- examples/lambda-functions/tsconfig.json | 4 ++-- examples/sam/src/tsconfig.json | 4 ++-- examples/sam/tsconfig.json | 4 ++-- packages/commons/tsconfig-dev.json | 4 ++-- packages/commons/tsconfig.json | 4 ++-- packages/logger/tsconfig-dev.json | 4 ++-- packages/logger/tsconfig.es.json | 4 ++-- packages/logger/tsconfig.json | 4 ++-- packages/metrics/tsconfig.es.json | 4 ++-- packages/metrics/tsconfig.json | 4 ++-- packages/tracer/tsconfig.es.json | 4 ++-- packages/tracer/tsconfig.json | 4 ++-- 13 files changed, 26 insertions(+), 26 deletions(-) diff --git a/examples/cdk/tsconfig.json b/examples/cdk/tsconfig.json index 3d8421df47..ce59f26c4a 100644 --- a/examples/cdk/tsconfig.json +++ b/examples/cdk/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "outDir": "lib", @@ -22,7 +22,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, -"lib": [ "es2020" ], +"lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/examples/lambda-functions/tsconfig.json b/examples/lambda-functions/tsconfig.json index 6fe6024ff3..b253fc49a1 100644 --- a/examples/lambda-functions/tsconfig.json +++ b/examples/lambda-functions/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -21,7 +21,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "node" ] diff --git a/examples/sam/src/tsconfig.json b/examples/sam/src/tsconfig.json index 6fe6024ff3..b253fc49a1 100644 --- a/examples/sam/src/tsconfig.json +++ b/examples/sam/src/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -21,7 +21,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "node" ] diff --git a/examples/sam/tsconfig.json b/examples/sam/tsconfig.json index 6fe6024ff3..b253fc49a1 100644 --- a/examples/sam/tsconfig.json +++ b/examples/sam/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -21,7 +21,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "node" ] diff --git a/packages/commons/tsconfig-dev.json b/packages/commons/tsconfig-dev.json index 802f18e8f9..7743a7b77d 100644 --- a/packages/commons/tsconfig-dev.json +++ b/packages/commons/tsconfig-dev.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -23,7 +23,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/packages/commons/tsconfig.json b/packages/commons/tsconfig.json index cbd9922f32..19330d4c1b 100644 --- a/packages/commons/tsconfig.json +++ b/packages/commons/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -23,7 +23,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/packages/logger/tsconfig-dev.json b/packages/logger/tsconfig-dev.json index 4ea999b37f..7c6046c8bc 100644 --- a/packages/logger/tsconfig-dev.json +++ b/packages/logger/tsconfig-dev.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -22,7 +22,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/packages/logger/tsconfig.es.json b/packages/logger/tsconfig.es.json index 4ea999b37f..7c6046c8bc 100644 --- a/packages/logger/tsconfig.es.json +++ b/packages/logger/tsconfig.es.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -22,7 +22,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/packages/logger/tsconfig.json b/packages/logger/tsconfig.json index 20da6e39a9..3d7d8b8b05 100644 --- a/packages/logger/tsconfig.json +++ b/packages/logger/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "outDir": "lib", @@ -22,7 +22,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/packages/metrics/tsconfig.es.json b/packages/metrics/tsconfig.es.json index d28abaa6d5..65215a1c76 100644 --- a/packages/metrics/tsconfig.es.json +++ b/packages/metrics/tsconfig.es.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -23,7 +23,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/packages/metrics/tsconfig.json b/packages/metrics/tsconfig.json index 582618277f..23252b808b 100644 --- a/packages/metrics/tsconfig.json +++ b/packages/metrics/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -24,7 +24,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/packages/tracer/tsconfig.es.json b/packages/tracer/tsconfig.es.json index 802f18e8f9..7743a7b77d 100644 --- a/packages/tracer/tsconfig.es.json +++ b/packages/tracer/tsconfig.es.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -23,7 +23,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" diff --git a/packages/tracer/tsconfig.json b/packages/tracer/tsconfig.json index 8b93f8c299..c3cb48b6fb 100644 --- a/packages/tracer/tsconfig.json +++ b/packages/tracer/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "experimentalDecorators": true, "noImplicitAny": true, - "target": "ES2020", + "target": "ES2019", "module": "commonjs", "declaration": true, "declarationMap": true, @@ -24,7 +24,7 @@ "watchDirectory": "useFsEvents", "fallbackPolling": "dynamicPriority" }, - "lib": [ "es2020" ], + "lib": [ "es2019" ], "types": [ "jest", "node" From 974943415a116e1bcb500a8a0368674d62b34f5f Mon Sep 17 00:00:00 2001 From: ijemmy Date: Mon, 30 May 2022 17:42:00 +0200 Subject: [PATCH 03/15] build: force type on descriptor to make apply metho not optional --- packages/metrics/src/Metrics.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index 36aad90687..c7ac4e3fe0 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -1,5 +1,5 @@ import { Callback, Context } from 'aws-lambda'; -import { Utility } from '@aws-lambda-powertools/commons'; +import { LambdaInterface, Utility } from '@aws-lambda-powertools/commons'; import { MetricsInterface } from '.'; import { ConfigServiceInterface, EnvironmentVariablesService } from './config'; import { @@ -237,7 +237,10 @@ class Metrics extends Utility implements MetricsInterface { } return (target, _propertyKey, descriptor) => { - const originalMethod = descriptor.value; + // Without explicit type, the apply method + const originalMethod = descriptor.value as { + apply: (target: LambdaInterface, any: unknown[]) => Promise + }; descriptor.value = ( async (event: unknown, context: Context, callback: Callback): Promise => { this.functionName = context.functionName; @@ -245,7 +248,7 @@ class Metrics extends Utility implements MetricsInterface { let result: unknown; try { - result = await originalMethod?.apply(target, [ event, context, callback ]); + result = await originalMethod.apply(target, [ event, context, callback ]); } catch (error) { throw error; } finally { From 146150cabb7771cf847cfe9eeb942093d2a66869 Mon Sep 17 00:00:00 2001 From: ijemmy Date: Mon, 30 May 2022 18:48:15 +0200 Subject: [PATCH 04/15] TMP: figuring out why all ?. are broken --- packages/logger/src/Logger.ts | 10 +++++++--- packages/metrics/src/DescriptorInterface.ts | 12 ++++++++++++ packages/metrics/src/Metrics.ts | 9 +++++---- packages/tracer/src/Tracer.ts | 10 +++++++--- 4 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 packages/metrics/src/DescriptorInterface.ts diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index e26fdfac27..ab970ae980 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -1,6 +1,6 @@ import { Console } from 'console'; import type { Context } from 'aws-lambda'; -import { Utility } from '@aws-lambda-powertools/commons'; +import { LambdaInterface, Utility } from '@aws-lambda-powertools/commons'; import { LogFormatterInterface, PowertoolLogFormatter } from './formatter'; import { LogItem } from './log'; import cloneDeep from 'lodash.clonedeep'; @@ -252,12 +252,16 @@ class Logger extends Utility implements ClassThatLogs { */ public injectLambdaContext(): HandlerMethodDecorator { return (target, _propertyKey, descriptor) => { - const originalMethod = descriptor.value; + /** + * The descriptor.value is the method this decorator decorates, it cannot be undefined. + */ + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const originalMethod = descriptor.value!; descriptor.value = (event, context, callback) => { this.addContext(context); - return originalMethod?.apply(target, [ event, context, callback ]); + return originalMethod.apply(target, [ event, context, callback ]); }; }; } diff --git a/packages/metrics/src/DescriptorInterface.ts b/packages/metrics/src/DescriptorInterface.ts new file mode 100644 index 0000000000..8d5efda52e --- /dev/null +++ b/packages/metrics/src/DescriptorInterface.ts @@ -0,0 +1,12 @@ +/** + * The "descriptor" of decorator should always have "value" field. + * If not, the transpiler will add if/else statement and a branch coverage will be missed. + * It doesn't make sense to add a test case when descriptor doesn't have value. + */ +type WithRequired = T & { [P in K]-?: T[P] }; + +type TypedPropertyDescriptorWithValue = WithRequired>, 'value'>; + +export { + TypedPropertyDescriptorWithValue as TypedPropertyDescriptorWithApply, +}; \ No newline at end of file diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index c7ac4e3fe0..1fac4f6c6c 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -237,10 +237,11 @@ class Metrics extends Utility implements MetricsInterface { } return (target, _propertyKey, descriptor) => { - // Without explicit type, the apply method - const originalMethod = descriptor.value as { - apply: (target: LambdaInterface, any: unknown[]) => Promise - }; + /** + * The descriptor.value is the method this decorator decorates, it cannot be undefined. + */ + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const originalMethod = descriptor.value!; descriptor.value = ( async (event: unknown, context: Context, callback: Callback): Promise => { this.functionName = context.functionName; diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index b25d940777..08a6ca0728 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -340,11 +340,15 @@ class Tracer extends Utility implements TracerInterface { */ public captureLambdaHandler(): HandlerMethodDecorator { return (target, _propertyKey, descriptor) => { - const originalMethod = descriptor.value; + /** + * The descriptor.value is the method this decorator decorates, it cannot be undefined. + */ + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const originalMethod = descriptor.value!; descriptor.value = ((event, context, callback) => { if (!this.isTracingEnabled()) { - return originalMethod?.apply(target, [ event, context, callback ]); + return originalMethod.apply(target, [ event, context, callback ]); } return this.provider.captureAsyncFunc(`## ${process.env._HANDLER}`, async subsegment => { @@ -352,7 +356,7 @@ class Tracer extends Utility implements TracerInterface { this.addServiceNameAnnotation(); let result: unknown; try { - result = await originalMethod?.apply(target, [ event, context, callback ]); + result = await originalMethod.apply(target, [ event, context, callback ]); this.addResponseAsMetadata(result, process.env._HANDLER); } catch (error) { this.addErrorAsMetadata(error as Error); From 1ed2ea559c8beea7a2a37b71c4fc9320f501296b Mon Sep 17 00:00:00 2001 From: ijemmy Date: Mon, 30 May 2022 19:40:36 +0200 Subject: [PATCH 05/15] build: fix missing branch coverage in logger --- packages/logger/src/Logger.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index ab970ae980..78c748c99d 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -450,11 +450,11 @@ class Logger extends Utility implements ClassThatLogs { * @returns {number} */ private getSampleRateValue(): number { - if (!this.powertoolLogData?.sampleRateValue) { + if (!this.powertoolLogData.sampleRateValue) { this.setSampleRateValue(); } - return this.powertoolLogData?.sampleRateValue; + return this.powertoolLogData.sampleRateValue; } /** From 0ed41114cde47eddd82e0b8f2d217a17e92667ce Mon Sep 17 00:00:00 2001 From: ijemmy Date: Mon, 30 May 2022 20:02:17 +0200 Subject: [PATCH 06/15] build: fix all tracer branch coverage issue except segment undefined --- packages/tracer/src/Tracer.ts | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index 08a6ca0728..ba554f72ae 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -123,7 +123,8 @@ class Tracer extends Utility implements TracerInterface { private customConfigService?: ConfigServiceInterface; - private envVarsService?: EnvironmentVariablesService; + // envVarsService is always initialized in the constructor in setOptions() + private envVarsService!: EnvironmentVariablesService; private serviceName?: string; @@ -411,17 +412,19 @@ class Tracer extends Utility implements TracerInterface { */ public captureMethod(): MethodDecorator { return (target, _propertyKey, descriptor) => { - const originalMethod = descriptor.value; + // The descriptor.value is the method this decorator decorates, it cannot be undefined. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const originalMethod = descriptor.value!; descriptor.value = (...args: unknown[]) => { if (!this.isTracingEnabled()) { - return originalMethod?.apply(target, [...args]); + return originalMethod.apply(target, [...args]); } return this.provider.captureAsyncFunc(`### ${originalMethod.name}`, async subsegment => { let result; try { - result = await originalMethod?.apply(this, [...args]); + result = await originalMethod.apply(this, [...args]); this.addResponseAsMetadata(result, originalMethod.name); } catch (error) { this.addErrorAsMetadata(error as Error); @@ -513,7 +516,7 @@ class Tracer extends Utility implements TracerInterface { return; } - document?.addAnnotation(key, value); + document.addAnnotation(key, value); } /** @@ -548,7 +551,7 @@ class Tracer extends Utility implements TracerInterface { } namespace = namespace || this.serviceName; - document?.addMetadata(key, value, namespace); + document.addMetadata(key, value, namespace); } /** @@ -592,7 +595,7 @@ class Tracer extends Utility implements TracerInterface { * Used internally during initialization. */ private getEnvVarsService(): EnvironmentVariablesService { - return this.envVarsService; + return this.envVarsService; } /** @@ -600,7 +603,7 @@ class Tracer extends Utility implements TracerInterface { * Used internally during initialization. */ private isLambdaExecutionEnv(): boolean { - return this.getEnvVarsService()?.getAwsExecutionEnv() !== ''; + return this.getEnvVarsService().getAwsExecutionEnv() !== ''; } /** @@ -608,7 +611,7 @@ class Tracer extends Utility implements TracerInterface { * Used internally during initialization. */ private isLambdaSamCli(): boolean { - return this.getEnvVarsService()?.getSamLocal() !== ''; + return this.getEnvVarsService().getSamLocal() !== ''; } /** @@ -633,7 +636,7 @@ class Tracer extends Utility implements TracerInterface { return; } - const envVarsValue = this.getEnvVarsService()?.getTracingCaptureError(); + const envVarsValue = this.getEnvVarsService().getTracingCaptureError(); if (envVarsValue.toLowerCase() === 'false') { this.captureError = false; @@ -666,7 +669,7 @@ class Tracer extends Utility implements TracerInterface { return; } - const envVarsValue = this.getEnvVarsService()?.getCaptureHTTPsRequests(); + const envVarsValue = this.getEnvVarsService().getCaptureHTTPsRequests(); if (envVarsValue.toLowerCase() === 'false') { this.captureHTTPsRequests = false; @@ -686,7 +689,7 @@ class Tracer extends Utility implements TracerInterface { return; } - const envVarsValue = this.getEnvVarsService()?.getTracingCaptureResponse(); + const envVarsValue = this.getEnvVarsService().getTracingCaptureResponse(); if (envVarsValue.toLowerCase() === 'false') { this.captureResponse = false; @@ -757,7 +760,7 @@ class Tracer extends Utility implements TracerInterface { return; } - const envVarsValue = this.getEnvVarsService()?.getServiceName(); + const envVarsValue = this.getEnvVarsService().getServiceName(); if (envVarsValue !== undefined && Tracer.isValidServiceName(envVarsValue)) { this.serviceName = envVarsValue; @@ -785,7 +788,7 @@ class Tracer extends Utility implements TracerInterface { return; } - const envVarsValue = this.getEnvVarsService()?.getTracingEnabled(); + const envVarsValue = this.getEnvVarsService().getTracingEnabled(); if (envVarsValue.toLowerCase() === 'false') { this.tracingEnabled = false; From 9535fcab057424f7c46d0ac1e8872907cf60ac5a Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 14:35:21 +0200 Subject: [PATCH 07/15] build: fix branch coverage (confirmed that subsegment cannot be undefined) --- packages/tracer/src/Tracer.ts | 19 ++++++++++++++++--- packages/tracer/src/middleware/middy.ts | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index ba554f72ae..c0cc5fecfa 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -363,8 +363,15 @@ class Tracer extends Utility implements TracerInterface { this.addErrorAsMetadata(error as Error); throw error; } finally { - subsegment?.close(); - subsegment?.flush(); + // if (subsegment) { + // console.log('## captureLambdaHandler() subsegment NOT empty'); + // } else { + // console.log('## captureLambdaHandler() subsegment IS empty'); + // } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + subsegment!.close(); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + subsegment!.flush(); } return result; @@ -431,7 +438,13 @@ class Tracer extends Utility implements TracerInterface { // TODO: should this error be thrown?? If thrown we get a ERR_UNHANDLED_REJECTION. If not aren't we are basically catching a Customer error? // throw error; } finally { - subsegment?.close(); + // if (subsegment) { + // console.log('## captureMethod() subsegment NOT empty'); + // } else { + // console.log('## captureMethod() subsegment IS empty'); + // } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + subsegment!.close(); } return result; diff --git a/packages/tracer/src/middleware/middy.ts b/packages/tracer/src/middleware/middy.ts index 2114a80533..bac2490177 100644 --- a/packages/tracer/src/middleware/middy.ts +++ b/packages/tracer/src/middleware/middy.ts @@ -37,7 +37,7 @@ const captureLambdaHandler = (target: Tracer): middy.MiddlewareObj => { const close = (): void => { const subsegment = target.getSegment(); - subsegment?.close(); + subsegment.close(); target.setSegment(lambdaSegment as Segment); }; From cc8553067bcaef4d06902e6c2b278d8843dc2fb9 Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 14:35:56 +0200 Subject: [PATCH 08/15] build: fix linting --- packages/metrics/src/Metrics.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index 1fac4f6c6c..aba0e0d865 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -1,5 +1,5 @@ import { Callback, Context } from 'aws-lambda'; -import { LambdaInterface, Utility } from '@aws-lambda-powertools/commons'; +import { Utility } from '@aws-lambda-powertools/commons'; import { MetricsInterface } from '.'; import { ConfigServiceInterface, EnvironmentVariablesService } from './config'; import { From 3f193c8bdab0af5221ef5e60ecdb7c2dddd17aac Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 18:44:25 +0200 Subject: [PATCH 09/15] build: Fix linting and missing branch coverage --- packages/logger/src/Logger.ts | 2 +- packages/tracer/src/Tracer.ts | 27 +++++++++++------------ packages/tracer/tests/unit/Tracer.test.ts | 19 ++++++++++++---- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 78c748c99d..f83e0d3b8f 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -1,6 +1,6 @@ import { Console } from 'console'; import type { Context } from 'aws-lambda'; -import { LambdaInterface, Utility } from '@aws-lambda-powertools/commons'; +import { Utility } from '@aws-lambda-powertools/commons'; import { LogFormatterInterface, PowertoolLogFormatter } from './formatter'; import { LogItem } from './log'; import cloneDeep from 'lodash.clonedeep'; diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index c0cc5fecfa..48f08934f7 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -363,15 +363,15 @@ class Tracer extends Utility implements TracerInterface { this.addErrorAsMetadata(error as Error); throw error; } finally { - // if (subsegment) { - // console.log('## captureLambdaHandler() subsegment NOT empty'); - // } else { - // console.log('## captureLambdaHandler() subsegment IS empty'); - // } + if (subsegment) { + console.log('## captureLambdaHandler() subsegment NOT empty'); + subsegment.close(); + subsegment.flush(); + } else { + console.log('## captureLambdaHandler() subsegment IS empty'); + } // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - subsegment!.close(); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - subsegment!.flush(); } return result; @@ -438,13 +438,12 @@ class Tracer extends Utility implements TracerInterface { // TODO: should this error be thrown?? If thrown we get a ERR_UNHANDLED_REJECTION. If not aren't we are basically catching a Customer error? // throw error; } finally { - // if (subsegment) { - // console.log('## captureMethod() subsegment NOT empty'); - // } else { - // console.log('## captureMethod() subsegment IS empty'); - // } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - subsegment!.close(); + if (subsegment) { + console.log('## captureMethod() subsegment NOT empty'); + subsegment.close(); + } else { + console.log('## captureMethod() subsegment IS empty'); + } } return result; diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 377ff13136..fb34bfb71f 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -8,11 +8,22 @@ import { Tracer } from '../../src'; import { Callback, Context, Handler } from 'aws-lambda/handler'; import { Segment, setContextMissingStrategy, Subsegment } from 'aws-xray-sdk-core'; import { DynamoDB } from 'aws-sdk'; +import { ProviderServiceInterface } from '../../src/provider'; interface LambdaInterface { handler: Handler } +type CaptureAsyncFuncMock = jest.SpyInstance unknown, parent?: Segment | Subsegment]>; +const createCaptureAsyncFuncMock = function(provider: ProviderServiceInterface): CaptureAsyncFuncMock { + return jest.spyOn(provider, 'captureAsyncFunc') + .mockImplementation((methodName, callBackFn) => { + const subsegment = new Subsegment(`### ${methodName}`); + jest.spyOn(subsegment, 'flush').mockImplementation(() => null); + callBackFn(subsegment); + }); +}; + jest.spyOn(console, 'debug').mockImplementation(() => null); jest.spyOn(console, 'warn').mockImplementation(() => null); jest.spyOn(console, 'error').mockImplementation(() => null); @@ -579,7 +590,7 @@ describe('Class: Tracer', () => { describe('Method: captureLambdaHandler', () => { test('when used as decorator while tracing is disabled, it does nothing', async () => { - + // Prepare const tracer: Tracer = new Tracer({ enabled: false }); jest.spyOn(tracer.provider, 'getSegment').mockImplementation(() => new Segment('facade', process.env._X_AMZN_TRACE_ID || null)); @@ -763,7 +774,7 @@ describe('Class: Tracer', () => { .mockImplementationOnce(() => newSubsegmentFirstInvocation) .mockImplementation(() => newSubsegmentSecondInvocation); setContextMissingStrategy(() => null); - const captureAsyncFuncSpy = jest.spyOn(tracer.provider, 'captureAsyncFunc'); + const captureAsyncFuncSpy = createCaptureAsyncFuncMock(tracer.provider); const putAnnotationSpy = jest.spyOn(tracer, 'putAnnotation'); class Lambda implements LambdaInterface { @@ -814,7 +825,7 @@ describe('Class: Tracer', () => { jest.spyOn(tracer.provider, 'getSegment') .mockImplementation(() => newSubsegment); setContextMissingStrategy(() => null); - const captureAsyncFuncSpy = jest.spyOn(tracer.provider, 'captureAsyncFunc'); + const captureAsyncFuncSpy = createCaptureAsyncFuncMock(tracer.provider); class Lambda implements LambdaInterface { @tracer.captureLambdaHandler() @@ -930,7 +941,7 @@ describe('Class: Tracer', () => { jest.spyOn(tracer.provider, 'getSegment') .mockImplementation(() => newSubsegment); setContextMissingStrategy(() => null); - const captureAsyncFuncSpy = jest.spyOn(tracer.provider, 'captureAsyncFunc'); + const captureAsyncFuncSpy = createCaptureAsyncFuncMock(tracer.provider); const addErrorSpy = jest.spyOn(newSubsegment, 'addError'); class Lambda implements LambdaInterface { From 3cbd39059635fb1146dd91fe77abcc9bcbe59a0a Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 19:02:27 +0200 Subject: [PATCH 10/15] build(logger): switch to use uuid.v4 instead of randomUUID() --- .github/workflows/run-e2e-tests.yml | 1 + package-lock.json | 117 +++++++++++------- package.json | 4 +- .../tests/e2e/basicFeatures.middy.test.ts | 5 +- .../tests/e2e/childLogger.manual.test.ts | 4 +- .../tests/e2e/sampleRate.decorator.test.ts | 3 +- 6 files changed, 85 insertions(+), 49 deletions(-) diff --git a/.github/workflows/run-e2e-tests.yml b/.github/workflows/run-e2e-tests.yml index 6a7c7c46d0..e50306378e 100644 --- a/.github/workflows/run-e2e-tests.yml +++ b/.github/workflows/run-e2e-tests.yml @@ -53,6 +53,7 @@ jobs: matrix: package: [logger] version: [12, 14, 16] + fail-fast: false steps: - name: "Checkout" uses: actions/checkout@v3 diff --git a/package-lock.json b/package-lock.json index 8868345202..e36f0711fe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,6 +25,7 @@ "@types/aws-lambda": "^8.10.72", "@types/jest": "^27.4.0", "@types/node": "^17.0.8", + "@types/uuid": "^8.3.4", "@typescript-eslint/eslint-plugin": "^5.12.1", "@typescript-eslint/parser": "^5.12.1", "archiver": "^5.3.0", @@ -48,7 +49,8 @@ "ts-node": "^10.0.0", "typedoc": "^0.22.11", "typedoc-plugin-missing-exports": "^0.22.6", - "typescript": "^4.1.3" + "typescript": "^4.1.3", + "uuid": "^8.3.2" }, "engines": { "node": ">=12" @@ -373,15 +375,6 @@ "node": ">=12.0.0" } }, - "node_modules/@aws-sdk/client-dynamodb/node_modules/uuid": { - "version": "8.3.2", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz", - "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==", - "dev": true, - "bin": { - "uuid": "dist/bin/uuid" - } - }, "node_modules/@aws-sdk/client-sso": { "version": "3.58.0", "resolved": "https://registry.npmjs.org/@aws-sdk/client-sso/-/client-sso-3.58.0.tgz", @@ -736,15 +729,6 @@ "node": ">= 12.0.0" } }, - "node_modules/@aws-sdk/middleware-retry/node_modules/uuid": { - "version": "8.3.2", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz", - "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==", - "dev": true, - "bin": { - "uuid": "dist/bin/uuid" - } - }, "node_modules/@aws-sdk/middleware-sdk-sts": { "version": "3.58.0", "resolved": "https://registry.npmjs.org/@aws-sdk/middleware-sdk-sts/-/middleware-sdk-sts-3.58.0.tgz", @@ -3997,6 +3981,12 @@ "integrity": "sha512-Hl219/BT5fLAaz6NDkSuhzasy49dwQS/DSdu4MdggFB8zcXv7vflBI3xp7FEmkmdDkBUI2bPUNeMttp2knYdxw==", "dev": true }, + "node_modules/@types/uuid": { + "version": "8.3.4", + "resolved": "https://registry.npmjs.org/@types/uuid/-/uuid-8.3.4.tgz", + "integrity": "sha512-c/I8ZRb51j+pYGAu5CrFMRxqZ2ke4y2grEBO5AUjgSkSk+qT2Ea+OdWElz/OiMf5MNpn2b17kuVBwZLQJXzihw==", + "dev": true + }, "node_modules/@types/yargs": { "version": "16.0.4", "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-16.0.4.tgz", @@ -4963,6 +4953,16 @@ "node": ">= 10.0.0" } }, + "node_modules/aws-sdk/node_modules/uuid": { + "version": "3.3.2", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.3.2.tgz", + "integrity": "sha512-yXJmeNaw3DnnKAOKJE51sL/ZaYfWJRl1pK9dr19YFCu0ObS231AB1/LbqTKRAQ5kw8A90rA6fr4riOUpTZvQZA==", + "deprecated": "Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.", + "dev": true, + "bin": { + "uuid": "bin/uuid" + } + }, "node_modules/aws-sign2": { "version": "0.7.0", "resolved": "https://registry.npmjs.org/aws-sign2/-/aws-sign2-0.7.0.tgz", @@ -13474,6 +13474,16 @@ "node": ">=0.8" } }, + "node_modules/request/node_modules/uuid": { + "version": "3.4.0", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz", + "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==", + "deprecated": "Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.", + "dev": true, + "bin": { + "uuid": "bin/uuid" + } + }, "node_modules/require-directory": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/require-directory/-/require-directory-2.1.1.tgz", @@ -14337,6 +14347,16 @@ "node": ">=8" } }, + "node_modules/temp-write/node_modules/uuid": { + "version": "3.4.0", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz", + "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==", + "deprecated": "Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.", + "dev": true, + "bin": { + "uuid": "bin/uuid" + } + }, "node_modules/terminal-link": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/terminal-link/-/terminal-link-2.1.1.tgz", @@ -14926,13 +14946,12 @@ } }, "node_modules/uuid": { - "version": "3.3.2", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.3.2.tgz", - "integrity": "sha512-yXJmeNaw3DnnKAOKJE51sL/ZaYfWJRl1pK9dr19YFCu0ObS231AB1/LbqTKRAQ5kw8A90rA6fr4riOUpTZvQZA==", - "deprecated": "Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.", + "version": "8.3.2", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz", + "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==", "dev": true, "bin": { - "uuid": "bin/uuid" + "uuid": "dist/bin/uuid" } }, "node_modules/v8-compile-cache": { @@ -15849,14 +15868,6 @@ "@aws-sdk/util-waiter": "3.55.0", "tslib": "^2.3.1", "uuid": "^8.3.2" - }, - "dependencies": { - "uuid": { - "version": "8.3.2", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz", - "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==", - "dev": true - } } }, "@aws-sdk/client-sso": { @@ -16157,14 +16168,6 @@ "@aws-sdk/util-middleware": "3.55.0", "tslib": "^2.3.1", "uuid": "^8.3.2" - }, - "dependencies": { - "uuid": { - "version": "8.3.2", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz", - "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==", - "dev": true - } } }, "@aws-sdk/middleware-sdk-sts": { @@ -18829,6 +18832,12 @@ "integrity": "sha512-Hl219/BT5fLAaz6NDkSuhzasy49dwQS/DSdu4MdggFB8zcXv7vflBI3xp7FEmkmdDkBUI2bPUNeMttp2knYdxw==", "dev": true }, + "@types/uuid": { + "version": "8.3.4", + "resolved": "https://registry.npmjs.org/@types/uuid/-/uuid-8.3.4.tgz", + "integrity": "sha512-c/I8ZRb51j+pYGAu5CrFMRxqZ2ke4y2grEBO5AUjgSkSk+qT2Ea+OdWElz/OiMf5MNpn2b17kuVBwZLQJXzihw==", + "dev": true + }, "@types/yargs": { "version": "16.0.4", "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-16.0.4.tgz", @@ -19505,6 +19514,14 @@ "url": "0.10.3", "uuid": "3.3.2", "xml2js": "0.4.19" + }, + "dependencies": { + "uuid": { + "version": "3.3.2", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.3.2.tgz", + "integrity": "sha512-yXJmeNaw3DnnKAOKJE51sL/ZaYfWJRl1pK9dr19YFCu0ObS231AB1/LbqTKRAQ5kw8A90rA6fr4riOUpTZvQZA==", + "dev": true + } } }, "aws-sign2": { @@ -26190,6 +26207,12 @@ "psl": "^1.1.28", "punycode": "^2.1.1" } + }, + "uuid": { + "version": "3.4.0", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz", + "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==", + "dev": true } } }, @@ -26838,6 +26861,14 @@ "make-dir": "^3.0.0", "temp-dir": "^1.0.0", "uuid": "^3.3.2" + }, + "dependencies": { + "uuid": { + "version": "3.4.0", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz", + "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==", + "dev": true + } } }, "terminal-link": { @@ -27280,9 +27311,9 @@ } }, "uuid": { - "version": "3.3.2", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.3.2.tgz", - "integrity": "sha512-yXJmeNaw3DnnKAOKJE51sL/ZaYfWJRl1pK9dr19YFCu0ObS231AB1/LbqTKRAQ5kw8A90rA6fr4riOUpTZvQZA==", + "version": "8.3.2", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz", + "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==", "dev": true }, "v8-compile-cache": { diff --git a/package.json b/package.json index 2e7676dfad..d7a987463b 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "@types/aws-lambda": "^8.10.72", "@types/jest": "^27.4.0", "@types/node": "^17.0.8", + "@types/uuid": "^8.3.4", "@typescript-eslint/eslint-plugin": "^5.12.1", "@typescript-eslint/parser": "^5.12.1", "archiver": "^5.3.0", @@ -78,7 +79,8 @@ "ts-node": "^10.0.0", "typedoc": "^0.22.11", "typedoc-plugin-missing-exports": "^0.22.6", - "typescript": "^4.1.3" + "typescript": "^4.1.3", + "uuid": "^8.3.2" }, "files": [ "lib/**/*" diff --git a/packages/logger/tests/e2e/basicFeatures.middy.test.ts b/packages/logger/tests/e2e/basicFeatures.middy.test.ts index ab36645acd..8c2909d07e 100644 --- a/packages/logger/tests/e2e/basicFeatures.middy.test.ts +++ b/packages/logger/tests/e2e/basicFeatures.middy.test.ts @@ -8,9 +8,9 @@ */ import path from 'path'; -import { randomUUID } from 'crypto'; import { App, Stack } from 'aws-cdk-lib'; import { APIGatewayAuthorizerResult } from 'aws-lambda'; +import { v4 } from 'uuid'; import { createStackWithLambdaFunction, generateUniqueName, @@ -27,6 +27,7 @@ import { TEARDOWN_TIMEOUT } from './constants'; + const runtime: string = process.env.RUNTIME || 'nodejs16x'; if (!isValidRuntimeKey(runtime)) { @@ -35,7 +36,7 @@ if (!isValidRuntimeKey(runtime)) { const LEVEL = InvocationLogs.LEVEL; -const uuid = randomUUID(); +const uuid = v4(); const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'BasicFeatures-Middy'); const functionName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'BasicFeatures-Middy'); const lambdaFunctionCodeFile = 'basicFeatures.middy.test.FunctionCode.ts'; diff --git a/packages/logger/tests/e2e/childLogger.manual.test.ts b/packages/logger/tests/e2e/childLogger.manual.test.ts index b8bc59c47d..325f9cf4ad 100644 --- a/packages/logger/tests/e2e/childLogger.manual.test.ts +++ b/packages/logger/tests/e2e/childLogger.manual.test.ts @@ -8,8 +8,8 @@ */ import path from 'path'; -import { randomUUID } from 'crypto'; import { App, Stack } from 'aws-cdk-lib'; +import { v4 } from 'uuid'; import { createStackWithLambdaFunction, generateUniqueName, @@ -34,7 +34,7 @@ if (!isValidRuntimeKey(runtime)) { const LEVEL = InvocationLogs.LEVEL; -const uuid = randomUUID(); +const uuid = v4(); const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'ChildLogger-Manual'); const functionName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'ChildLogger-Manual'); const lambdaFunctionCodeFile = 'childLogger.manual.test.FunctionCode.ts'; diff --git a/packages/logger/tests/e2e/sampleRate.decorator.test.ts b/packages/logger/tests/e2e/sampleRate.decorator.test.ts index 038d61b4b5..363315f981 100644 --- a/packages/logger/tests/e2e/sampleRate.decorator.test.ts +++ b/packages/logger/tests/e2e/sampleRate.decorator.test.ts @@ -10,6 +10,7 @@ import path from 'path'; import { randomUUID } from 'crypto'; import { App, Stack } from 'aws-cdk-lib'; +import { v4 } from 'uuid'; import { createStackWithLambdaFunction, generateUniqueName, @@ -34,7 +35,7 @@ if (!isValidRuntimeKey(runtime)) { const LEVEL = InvocationLogs.LEVEL; -const uuid = randomUUID(); +const uuid = v4(); const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'SampleRate-Decorator'); const functionName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'SampleRate-Decorator'); const lambdaFunctionCodeFile = 'sampleRate.decorator.test.FunctionCode.ts'; From 037146ef26797cbb221f93ea81cd0b2740a46883 Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 19:28:59 +0200 Subject: [PATCH 11/15] build: change workflow to run on all packages --- .github/workflows/run-e2e-tests.yml | 6 +----- packages/logger/tests/e2e/basicFeatures.middy.test.ts | 1 - packages/logger/tests/e2e/sampleRate.decorator.test.ts | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/run-e2e-tests.yml b/.github/workflows/run-e2e-tests.yml index e50306378e..7b17f76092 100644 --- a/.github/workflows/run-e2e-tests.yml +++ b/.github/workflows/run-e2e-tests.yml @@ -1,10 +1,6 @@ name: run-e2e-tests on: workflow_dispatch: {} - # TODO: remove this once debugging completed - push: - branches: - - ijemmy/build-on-all-node-versions jobs: example-and-package-check: runs-on: ubuntu-latest @@ -51,7 +47,7 @@ jobs: contents: read strategy: matrix: - package: [logger] + package: [logger, metrics, tracer] version: [12, 14, 16] fail-fast: false steps: diff --git a/packages/logger/tests/e2e/basicFeatures.middy.test.ts b/packages/logger/tests/e2e/basicFeatures.middy.test.ts index 8c2909d07e..2595b92ec6 100644 --- a/packages/logger/tests/e2e/basicFeatures.middy.test.ts +++ b/packages/logger/tests/e2e/basicFeatures.middy.test.ts @@ -27,7 +27,6 @@ import { TEARDOWN_TIMEOUT } from './constants'; - const runtime: string = process.env.RUNTIME || 'nodejs16x'; if (!isValidRuntimeKey(runtime)) { diff --git a/packages/logger/tests/e2e/sampleRate.decorator.test.ts b/packages/logger/tests/e2e/sampleRate.decorator.test.ts index 363315f981..1b349b8579 100644 --- a/packages/logger/tests/e2e/sampleRate.decorator.test.ts +++ b/packages/logger/tests/e2e/sampleRate.decorator.test.ts @@ -8,7 +8,6 @@ */ import path from 'path'; -import { randomUUID } from 'crypto'; import { App, Stack } from 'aws-cdk-lib'; import { v4 } from 'uuid'; import { From 817c5a097664b32b7f55d8338ce0cd48cedd8e3b Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 19:29:55 +0200 Subject: [PATCH 12/15] build: run on unit tests in all NodeJS environment --- .github/workflows/pr_lint_and_test.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr_lint_and_test.yml b/.github/workflows/pr_lint_and_test.yml index df1cd9dd3b..7341f07d95 100644 --- a/.github/workflows/pr_lint_and_test.yml +++ b/.github/workflows/pr_lint_and_test.yml @@ -7,12 +7,16 @@ jobs: runs-on: ubuntu-latest env: NODE_ENV: dev + strategy: + matrix: + version: [12, 14, 16] + fail-fast: false steps: - uses: actions/checkout@v3 - - name: "Use NodeJS 16" + - name: "Use NodeJS" uses: actions/setup-node@v3 with: - node-version: '16' + node-version: ${{ matrix.version }} - name: Install npm@8.x run: npm i -g npm@next-8 - name: "Setup npm" From 5ff8739a528dd941b2d7cd186a8c9a80e8fa4354 Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 19:45:22 +0200 Subject: [PATCH 13/15] build: remove all randomUUID as it breaks in NodeJS12 --- .../metrics/tests/e2e/basicFeatures.decorators.test.ts | 4 ++-- .../metrics/tests/e2e/basicFeatures.manual.test.ts | 4 ++-- .../tracer/tests/e2e/allFeatures.decorator.test.ts | 10 +++++----- packages/tracer/tests/e2e/allFeatures.manual.test.ts | 4 ++-- packages/tracer/tests/e2e/allFeatures.middy.test.ts | 10 +++++----- .../tracer/tests/e2e/asyncHandler.decorator.test.ts | 6 +++--- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/metrics/tests/e2e/basicFeatures.decorators.test.ts b/packages/metrics/tests/e2e/basicFeatures.decorators.test.ts index 5d53d812d5..bf7afa1bdb 100644 --- a/packages/metrics/tests/e2e/basicFeatures.decorators.test.ts +++ b/packages/metrics/tests/e2e/basicFeatures.decorators.test.ts @@ -8,10 +8,10 @@ */ import path from 'path'; -import { randomUUID } from 'crypto'; import { Tracing } from 'aws-cdk-lib/aws-lambda'; import { App, Stack } from 'aws-cdk-lib'; import * as AWS from 'aws-sdk'; +import { v4 } from 'uuid'; import { generateUniqueName, isValidRuntimeKey, @@ -35,7 +35,7 @@ if (!isValidRuntimeKey(runtime)) { throw new Error(`Invalid runtime key value: ${runtime}`); } -const uuid = randomUUID(); +const uuid = v4(); const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'decorator'); const functionName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'decorator'); const lambdaFunctionCodeFile = 'basicFeatures.decorator.test.functionCode.ts'; diff --git a/packages/metrics/tests/e2e/basicFeatures.manual.test.ts b/packages/metrics/tests/e2e/basicFeatures.manual.test.ts index f0ebecfdfc..6a43fb665f 100644 --- a/packages/metrics/tests/e2e/basicFeatures.manual.test.ts +++ b/packages/metrics/tests/e2e/basicFeatures.manual.test.ts @@ -8,10 +8,10 @@ */ import path from 'path'; -import { randomUUID } from 'crypto'; import { Tracing } from 'aws-cdk-lib/aws-lambda'; import { App, Stack } from 'aws-cdk-lib'; import * as AWS from 'aws-sdk'; +import { v4 } from 'uuid'; import { generateUniqueName, isValidRuntimeKey, @@ -35,7 +35,7 @@ if (!isValidRuntimeKey(runtime)) { throw new Error(`Invalid runtime key value: ${runtime}`); } -const uuid = randomUUID(); +const uuid = v4(); const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'manual'); const functionName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'manual'); const lambdaFunctionCodeFile = 'basicFeatures.manual.test.functionCode.ts'; diff --git a/packages/tracer/tests/e2e/allFeatures.decorator.test.ts b/packages/tracer/tests/e2e/allFeatures.decorator.test.ts index 535a9660e1..5c2272f18a 100644 --- a/packages/tracer/tests/e2e/allFeatures.decorator.test.ts +++ b/packages/tracer/tests/e2e/allFeatures.decorator.test.ts @@ -4,11 +4,11 @@ * @group e2e/tracer/decorator */ -import { randomUUID } from 'crypto'; import path from 'path'; import { Table, AttributeType, BillingMode } from 'aws-cdk-lib/aws-dynamodb'; import { App, Stack, RemovalPolicy } from 'aws-cdk-lib'; import * as AWS from 'aws-sdk'; +import { v4 } from 'uuid'; import { deployStack, destroyStack } from '../../../commons/tests/utils/cdk-cli'; import { getTraces, @@ -54,27 +54,27 @@ if (!isValidRuntimeKey(runtime)) { * Each stack must use a unique `serviceName` as it's used to for retrieving the trace. * Using the same one will result in traces from different test cases mixing up. */ -const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, randomUUID(), runtime, 'AllFeatures-Decorator'); +const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, v4(), runtime, 'AllFeatures-Decorator'); const lambdaFunctionCodeFile = 'allFeatures.decorator.test.functionCode.ts'; let startTime: Date; /** * Function #1 is with all flags enabled. */ -const uuidFunction1 = randomUUID(); +const uuidFunction1 = v4(); const functionNameWithAllFlagsEnabled = generateUniqueName(RESOURCE_NAME_PREFIX, uuidFunction1, runtime, 'AllFeatures-Decoratory-AllFlagsEnabled'); const serviceNameWithAllFlagsEnabled = functionNameWithAllFlagsEnabled; /** * Function #2 doesn't capture error or response */ -const uuidFunction2 = randomUUID(); +const uuidFunction2 = v4(); const functionNameWithNoCaptureErrorOrResponse = generateUniqueName(RESOURCE_NAME_PREFIX, uuidFunction2, runtime, 'AllFeatures-Decorator-NoCaptureErrorOrResponse'); const serviceNameWithNoCaptureErrorOrResponse = functionNameWithNoCaptureErrorOrResponse; /** * Function #3 disables tracer */ -const uuidFunction3 = randomUUID(); +const uuidFunction3 = v4(); const functionNameWithTracerDisabled = generateUniqueName(RESOURCE_NAME_PREFIX, uuidFunction3, runtime, 'AllFeatures-Decorator-TracerDisabled'); const serviceNameWithTracerDisabled = functionNameWithNoCaptureErrorOrResponse; diff --git a/packages/tracer/tests/e2e/allFeatures.manual.test.ts b/packages/tracer/tests/e2e/allFeatures.manual.test.ts index de8920f55b..ed97abb3ad 100644 --- a/packages/tracer/tests/e2e/allFeatures.manual.test.ts +++ b/packages/tracer/tests/e2e/allFeatures.manual.test.ts @@ -4,11 +4,11 @@ * @group e2e/tracer/manual */ -import { randomUUID } from 'crypto'; import path from 'path'; import { Table, AttributeType, BillingMode } from 'aws-cdk-lib/aws-dynamodb'; import { App, Stack, RemovalPolicy } from 'aws-cdk-lib'; import * as AWS from 'aws-sdk'; +import { v4 } from 'uuid'; import { deployStack, destroyStack } from '../../../commons/tests/utils/cdk-cli'; import { getTraces, @@ -47,7 +47,7 @@ if (!isValidRuntimeKey(runtime)) { throw new Error(`Invalid runtime key value: ${runtime}`); } -const uuid = randomUUID(); +const uuid = v4(); const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'AllFeatures-Manual'); const functionName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'AllFeatures-Manual'); const lambdaFunctionCodeFile = 'allFeatures.manual.test.functionCode.ts'; diff --git a/packages/tracer/tests/e2e/allFeatures.middy.test.ts b/packages/tracer/tests/e2e/allFeatures.middy.test.ts index 8cb26b84b6..2f4b7cf931 100644 --- a/packages/tracer/tests/e2e/allFeatures.middy.test.ts +++ b/packages/tracer/tests/e2e/allFeatures.middy.test.ts @@ -4,11 +4,11 @@ * @group e2e/tracer/middy */ -import { randomUUID } from 'crypto'; import path from 'path'; import { Table, AttributeType, BillingMode } from 'aws-cdk-lib/aws-dynamodb'; import { App, Stack, RemovalPolicy } from 'aws-cdk-lib'; import * as AWS from 'aws-sdk'; +import { v4 } from 'uuid'; import { deployStack, destroyStack } from '../../../commons/tests/utils/cdk-cli'; import { getTraces, @@ -54,27 +54,27 @@ if (!isValidRuntimeKey(runtime)) { * Each stack must use a unique `serviceName` as it's used to for retrieving the trace. * Using the same one will result in traces from different test cases mixing up. */ -const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, randomUUID(), runtime, 'AllFeatures-Middy'); +const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, v4(), runtime, 'AllFeatures-Middy'); const lambdaFunctionCodeFile = 'allFeatures.middy.test.functionCode.ts'; let startTime: Date; /** * Function #1 is with all flags enabled. */ -const uuidFunction1 = randomUUID(); +const uuidFunction1 = v4(); const functionNameWithAllFlagsEnabled = generateUniqueName(RESOURCE_NAME_PREFIX, uuidFunction1, runtime, 'AllFeatures-Middy-AllFlagsEnabled'); const serviceNameWithAllFlagsEnabled = functionNameWithAllFlagsEnabled; /** * Function #2 doesn't capture error or response */ -const uuidFunction2 = randomUUID(); +const uuidFunction2 = v4(); const functionNameWithNoCaptureErrorOrResponse = generateUniqueName(RESOURCE_NAME_PREFIX, uuidFunction2, runtime, 'AllFeatures-Middy-NoCaptureErrorOrResponse'); const serviceNameWithNoCaptureErrorOrResponse = functionNameWithNoCaptureErrorOrResponse; /** * Function #3 disables tracer */ -const uuidFunction3 = randomUUID(); +const uuidFunction3 = v4(); const functionNameWithTracerDisabled = generateUniqueName(RESOURCE_NAME_PREFIX, uuidFunction3, runtime, 'AllFeatures-Middy-TracerDisabled'); const serviceNameWithTracerDisabled = functionNameWithNoCaptureErrorOrResponse; diff --git a/packages/tracer/tests/e2e/asyncHandler.decorator.test.ts b/packages/tracer/tests/e2e/asyncHandler.decorator.test.ts index 96f4de7abc..21cd44840c 100644 --- a/packages/tracer/tests/e2e/asyncHandler.decorator.test.ts +++ b/packages/tracer/tests/e2e/asyncHandler.decorator.test.ts @@ -4,11 +4,11 @@ * @group e2e/tracer/decorator-async-handler */ -import { randomUUID } from 'crypto'; import path from 'path'; import { Table, AttributeType, BillingMode } from 'aws-cdk-lib/aws-dynamodb'; import { App, Stack, RemovalPolicy } from 'aws-cdk-lib'; import * as AWS from 'aws-sdk'; +import { v4 } from 'uuid'; import { deployStack, destroyStack } from '../../../commons/tests/utils/cdk-cli'; import { getTraces, @@ -46,11 +46,11 @@ if (!isValidRuntimeKey(runtime)) { throw new Error(`Invalid runtime key value: ${runtime}`); } -const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, randomUUID(), runtime, 'AllFeatures-Decorator'); +const stackName = generateUniqueName(RESOURCE_NAME_PREFIX, v4(), runtime, 'AllFeatures-Decorator'); const lambdaFunctionCodeFile = 'asyncHandler.decorator.test.functionCode.ts'; let startTime: Date; -const uuid = randomUUID(); +const uuid = v4(); const functionName = generateUniqueName(RESOURCE_NAME_PREFIX, uuid, runtime, 'AllFeatures-Decoratory-AllFlagsEnabled'); const serviceName = functionName; From 7a23545356ce04a8dbbf8254268217400eae551c Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 19:47:54 +0200 Subject: [PATCH 14/15] build: remove unused file --- packages/metrics/src/DescriptorInterface.ts | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 packages/metrics/src/DescriptorInterface.ts diff --git a/packages/metrics/src/DescriptorInterface.ts b/packages/metrics/src/DescriptorInterface.ts deleted file mode 100644 index 8d5efda52e..0000000000 --- a/packages/metrics/src/DescriptorInterface.ts +++ /dev/null @@ -1,12 +0,0 @@ -/** - * The "descriptor" of decorator should always have "value" field. - * If not, the transpiler will add if/else statement and a branch coverage will be missed. - * It doesn't make sense to add a test case when descriptor doesn't have value. - */ -type WithRequired = T & { [P in K]-?: T[P] }; - -type TypedPropertyDescriptorWithValue = WithRequired>, 'value'>; - -export { - TypedPropertyDescriptorWithValue as TypedPropertyDescriptorWithApply, -}; \ No newline at end of file From cbb478adefdf71d6fa787ed277c01ce67d978973 Mon Sep 17 00:00:00 2001 From: ijemmy Date: Tue, 31 May 2022 19:50:45 +0200 Subject: [PATCH 15/15] chore(tracer): remove debug statements --- packages/tracer/src/Tracer.ts | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index 48f08934f7..ba554f72ae 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -363,15 +363,8 @@ class Tracer extends Utility implements TracerInterface { this.addErrorAsMetadata(error as Error); throw error; } finally { - if (subsegment) { - console.log('## captureLambdaHandler() subsegment NOT empty'); - subsegment.close(); - subsegment.flush(); - } else { - console.log('## captureLambdaHandler() subsegment IS empty'); - } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + subsegment?.close(); + subsegment?.flush(); } return result; @@ -438,12 +431,7 @@ class Tracer extends Utility implements TracerInterface { // TODO: should this error be thrown?? If thrown we get a ERR_UNHANDLED_REJECTION. If not aren't we are basically catching a Customer error? // throw error; } finally { - if (subsegment) { - console.log('## captureMethod() subsegment NOT empty'); - subsegment.close(); - } else { - console.log('## captureMethod() subsegment IS empty'); - } + subsegment?.close(); } return result;