From 127aad4698412d72368c093812dd4034839119ca Mon Sep 17 00:00:00 2001 From: Florian Chazal Date: Thu, 29 Sep 2022 11:50:01 +0200 Subject: [PATCH] fix: captureColdStartMetric and throwOnEmptyMetrics when set to false was interpreted as true (#1090) Co-authored-by: Florian Chazal --- packages/metrics/src/middleware/middy.ts | 4 +- packages/metrics/tests/unit/Metrics.test.ts | 26 ++ .../tests/unit/middleware/middy.test.ts | 334 +++++++++++++----- 3 files changed, 269 insertions(+), 95 deletions(-) diff --git a/packages/metrics/src/middleware/middy.ts b/packages/metrics/src/middleware/middy.ts index 5a0e1b7243..2b1bafb1e3 100644 --- a/packages/metrics/src/middleware/middy.ts +++ b/packages/metrics/src/middleware/middy.ts @@ -9,13 +9,13 @@ const logMetrics = (target: Metrics | Metrics[], options: ExtraOptions = {}): mi metricsInstances.forEach((metrics: Metrics) => { metrics.setFunctionName(request.context.functionName); const { throwOnEmptyMetrics, defaultDimensions, captureColdStartMetric } = options; - if (throwOnEmptyMetrics !== undefined) { + if (throwOnEmptyMetrics) { metrics.throwOnEmptyMetrics(); } if (defaultDimensions !== undefined) { metrics.setDefaultDimensions(defaultDimensions); } - if (captureColdStartMetric !== undefined) { + if (captureColdStartMetric) { metrics.captureColdStartMetric(); } }); diff --git a/packages/metrics/tests/unit/Metrics.test.ts b/packages/metrics/tests/unit/Metrics.test.ts index 345306714e..8ce2956969 100644 --- a/packages/metrics/tests/unit/Metrics.test.ts +++ b/packages/metrics/tests/unit/Metrics.test.ts @@ -371,6 +371,32 @@ describe('Class: Metrics', () => { } }); + test('Error should not be thrown on empty metrics if throwOnEmptyMetrics is set to false', async () => { + expect.assertions(1); + + const metrics = new Metrics({ namespace: 'test' }); + class LambdaFunction implements LambdaInterface { + @metrics.logMetrics({ throwOnEmptyMetrics: false }) + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + public handler( + _event: TEvent, + _context: Context, + _callback: Callback, + ): void | Promise { + return; + } + } + + try { + await new LambdaFunction().handler(dummyEvent, dummyContext.helloworldContext, () => console.log('Lambda invoked!')); + } catch (e) { + fail(`Should not throw but got the following Error: ${e}`); + } + const loggedData = JSON.parse(consoleSpy.mock.calls[0][0]); + expect(loggedData._aws.CloudWatchMetrics[0].Metrics.length).toBe(0); + }); + test('Error should be thrown on empty metrics when throwOnEmptyMetrics() is called', async () => { expect.assertions(1); diff --git a/packages/metrics/tests/unit/middleware/middy.test.ts b/packages/metrics/tests/unit/middleware/middy.test.ts index 3d4bed94f6..f322ce8bbd 100644 --- a/packages/metrics/tests/unit/middleware/middy.test.ts +++ b/packages/metrics/tests/unit/middleware/middy.test.ts @@ -16,15 +16,159 @@ const mockDate = new Date(1466424490000); const dateSpy = jest.spyOn(global, 'Date').mockImplementation(() => mockDate as unknown as string); describe('Middy middleware', () => { - beforeEach(() => { jest.resetModules(); consoleSpy.mockClear(); dateSpy.mockClear(); }); - describe('logMetrics', () => { + describe('throwOnEmptyMetrics', () => { + const getRandomInt = (): number => Math.floor(Math.random() * 1000000000); + const awsRequestId = getRandomInt().toString(); + + const context = { + callbackWaitsForEmptyEventLoop: true, + functionVersion: '$LATEST', + functionName: 'foo-bar-function', + memoryLimitInMB: '128', + logGroupName: '/aws/lambda/foo-bar-function', + logStreamName: '2021/03/09/[$LATEST]abcdef123456abcdef123456abcdef123456', + invokedFunctionArn: 'arn:aws:lambda:eu-west-1:123456789012:function:foo-bar-function', + awsRequestId: awsRequestId, + getRemainingTimeInMillis: () => 1234, + done: () => console.log('Done!'), + fail: () => console.log('Failed!'), + succeed: () => console.log('Succeeded!'), + }; + + test('should throw on empty metrics if set to true', async () => { + // Prepare + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); + + const lambdaHandler = (): void => { + console.log('do nothing'); + }; + + const handler = middy(lambdaHandler).use(logMetrics(metrics, { throwOnEmptyMetrics: true })); + + try { + await handler(event, context, () => console.log('Lambda invoked!')); + } catch (e) { + expect((e).message).toBe('The number of metrics recorded must be higher than zero'); + } + }); + test('should not throw on empty metrics if set to false', async () => { + // Prepare + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); + + const lambdaHandler = (): void => { + console.log('do nothing'); + }; + + const handler = middy(lambdaHandler).use(logMetrics(metrics, { throwOnEmptyMetrics: false })); + + try { + await handler(event, context, () => console.log('Lambda invoked!')); + } catch (e) { + fail(`Should not throw but got the following Error: ${e}`); + } + }); + + test('should not throw on empty metrics if not set', async () => { + // Prepare + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); + + const lambdaHandler = (): void => { + console.log('do nothing'); + }; + + const handler = middy(lambdaHandler).use(logMetrics(metrics)); + + try { + await handler(event, context, () => console.log('Lambda invoked!')); + } catch (e) { + fail(`Should not throw but got the following Error: ${e}`); + } + }); + }); + + describe('captureColdStartMetric', () => { + const getRandomInt = (): number => Math.floor(Math.random() * 1000000000); + const awsRequestId = getRandomInt().toString(); + + const context = { + callbackWaitsForEmptyEventLoop: true, + functionVersion: '$LATEST', + functionName: 'foo-bar-function', + memoryLimitInMB: '128', + logGroupName: '/aws/lambda/foo-bar-function', + logStreamName: '2021/03/09/[$LATEST]abcdef123456abcdef123456abcdef123456', + invokedFunctionArn: 'arn:aws:lambda:eu-west-1:123456789012:function:foo-bar-function', + awsRequestId: awsRequestId, + getRemainingTimeInMillis: () => 1234, + done: () => console.log('Done!'), + fail: () => console.log('Failed!'), + succeed: () => console.log('Succeeded!'), + }; + + test('should capture cold start metric if set to true', async () => { + // Prepare + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); + + const lambdaHandler = (): void => { + console.log('{"message": "do nothing"}'); + }; + + const handler = middy(lambdaHandler).use(logMetrics(metrics, { captureColdStartMetric: true })); + + await handler(event, context, () => console.log('Lambda invoked!')); + await handler(event, context, () => console.log('Lambda invoked! again')); + const loggedData = [ JSON.parse(consoleSpy.mock.calls[0][0]), JSON.parse(consoleSpy.mock.calls[1][0]) ]; + + expect(console.log).toBeCalledTimes(5); + expect(loggedData[0]._aws.CloudWatchMetrics[0].Metrics.length).toBe(1); + expect(loggedData[0]._aws.CloudWatchMetrics[0].Metrics[0].Name).toBe('ColdStart'); + expect(loggedData[0]._aws.CloudWatchMetrics[0].Metrics[0].Unit).toBe('Count'); + expect(loggedData[0].ColdStart).toBe(1); + }); + + test('should not capture cold start metrics if set to false', async () => { + // Prepare + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); + + const lambdaHandler = (): void => { + console.log('{"message": "do nothing"}'); + }; + + const handler = middy(lambdaHandler).use(logMetrics(metrics, { captureColdStartMetric: false })); + + await handler(event, context, () => console.log('Lambda invoked!')); + await handler(event, context, () => console.log('Lambda invoked! again')); + const loggedData = [ JSON.parse(consoleSpy.mock.calls[0][0]), JSON.parse(consoleSpy.mock.calls[1][0]) ]; + + expect(loggedData[0]._aws).toBe(undefined); + }); + + test('should not throw on empty metrics if not set', async () => { + // Prepare + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); + + const lambdaHandler = (): void => { + console.log('{"message": "do nothing"}'); + }; + + const handler = middy(lambdaHandler).use(logMetrics(metrics)); + + await handler(event, context, () => console.log('Lambda invoked!')); + await handler(event, context, () => console.log('Lambda invoked! again')); + const loggedData = [ JSON.parse(consoleSpy.mock.calls[0][0]), JSON.parse(consoleSpy.mock.calls[1][0]) ]; + + expect(loggedData[0]._aws).toBe(undefined); + }); + }); + + describe('logMetrics', () => { const getRandomInt = (): number => Math.floor(Math.random() * 1000000000); const awsRequestId = getRandomInt().toString(); @@ -45,7 +189,7 @@ describe('Middy middleware', () => { test('when a metrics instance receive multiple metrics with the same name, it prints multiple values in an array format', async () => { // Prepare - const metrics = new Metrics({ namespace:'serverlessAirline', serviceName:'orders' }); + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); const lambdaHandler = (): void => { metrics.addMetric('successfulBooking', MetricUnits.Count, 2); @@ -53,42 +197,41 @@ describe('Middy middleware', () => { }; const handler = middy(lambdaHandler).use(logMetrics(metrics)); - + // Act await handler(event, context, () => console.log('Lambda invoked!')); - + // Assess - expect(console.log).toHaveBeenNthCalledWith(1, JSON.stringify({ - '_aws': { - 'Timestamp': 1466424490000, - 'CloudWatchMetrics': [{ - 'Namespace': 'serverlessAirline', - 'Dimensions': [ - ['service'] + expect(console.log).toHaveBeenNthCalledWith( + 1, + JSON.stringify({ + _aws: { + Timestamp: 1466424490000, + CloudWatchMetrics: [ + { + Namespace: 'serverlessAirline', + Dimensions: [['service']], + Metrics: [{ Name: 'successfulBooking', Unit: 'Count' }], + }, ], - 'Metrics': [{ 'Name': 'successfulBooking', 'Unit': 'Count' }], - }], - }, - 'service': 'orders', - 'successfulBooking': [ - 2, - 1, - ], - })); + }, + service: 'orders', + successfulBooking: [2, 1], + }) + ); }); test('when a metrics instance is passed WITH custom options, it prints the metrics in the stdout', async () => { - // Prepare - const metrics = new Metrics({ namespace:'serverlessAirline', serviceName:'orders' }); + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); const lambdaHandler = (): void => { metrics.addMetric('successfulBooking', MetricUnits.Count, 1); }; const metricsOptions: ExtraOptions = { throwOnEmptyMetrics: true, - defaultDimensions: { environment : 'prod', aws_region: 'eu-west-1' }, - captureColdStartMetric: true + defaultDimensions: { environment: 'prod', aws_region: 'eu-west-1' }, + captureColdStartMetric: true, }; const handler = middy(lambdaHandler).use(logMetrics(metrics, metricsOptions)); @@ -96,46 +239,50 @@ describe('Middy middleware', () => { await handler(event, context, () => console.log('Lambda invoked!')); // Assess - expect(console.log).toHaveBeenNthCalledWith(1, JSON.stringify({ - '_aws': { - 'Timestamp': 1466424490000, - 'CloudWatchMetrics': [{ - 'Namespace': 'serverlessAirline', - 'Dimensions': [ - [ 'environment', 'aws_region', 'service', 'function_name' ] + expect(console.log).toHaveBeenNthCalledWith( + 1, + JSON.stringify({ + _aws: { + Timestamp: 1466424490000, + CloudWatchMetrics: [ + { + Namespace: 'serverlessAirline', + Dimensions: [['environment', 'aws_region', 'service', 'function_name']], + Metrics: [{ Name: 'ColdStart', Unit: 'Count' }], + }, ], - 'Metrics': [{ 'Name': 'ColdStart', 'Unit': 'Count' }], - }], - }, - 'environment': 'prod', - 'aws_region' : 'eu-west-1', - 'service': 'orders', - 'function_name': 'foo-bar-function', - 'ColdStart': 1, - })); - expect(console.log).toHaveBeenNthCalledWith(2, JSON.stringify({ - '_aws': { - 'Timestamp': 1466424490000, - 'CloudWatchMetrics': [{ - 'Namespace': 'serverlessAirline', - 'Dimensions': [ - [ 'environment', 'aws_region', 'service' ] + }, + environment: 'prod', + aws_region: 'eu-west-1', + service: 'orders', + function_name: 'foo-bar-function', + ColdStart: 1, + }) + ); + expect(console.log).toHaveBeenNthCalledWith( + 2, + JSON.stringify({ + _aws: { + Timestamp: 1466424490000, + CloudWatchMetrics: [ + { + Namespace: 'serverlessAirline', + Dimensions: [['environment', 'aws_region', 'service']], + Metrics: [{ Name: 'successfulBooking', Unit: 'Count' }], + }, ], - 'Metrics': [{ 'Name': 'successfulBooking', 'Unit': 'Count' }], - }], - }, - 'environment': 'prod', - 'aws_region' : 'eu-west-1', - 'service': 'orders', - 'successfulBooking': 1, - })); - + }, + environment: 'prod', + aws_region: 'eu-west-1', + service: 'orders', + successfulBooking: 1, + }) + ); }); test('when a metrics instance is passed WITHOUT custom options, it prints the metrics in the stdout', async () => { - // Prepare - const metrics = new Metrics({ namespace:'serverlessAirline', serviceName:'orders' }); + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); const lambdaHandler = (): void => { metrics.addMetric('successfulBooking', MetricUnits.Count, 1); @@ -147,33 +294,34 @@ describe('Middy middleware', () => { await handler(event, context, () => console.log('Lambda invoked!')); // Assess - expect(console.log).toHaveBeenNthCalledWith(1, JSON.stringify({ - '_aws': { - 'Timestamp': 1466424490000, - 'CloudWatchMetrics': [{ - 'Namespace': 'serverlessAirline', - 'Dimensions': [ - ['service'] + expect(console.log).toHaveBeenNthCalledWith( + 1, + JSON.stringify({ + _aws: { + Timestamp: 1466424490000, + CloudWatchMetrics: [ + { + Namespace: 'serverlessAirline', + Dimensions: [['service']], + Metrics: [{ Name: 'successfulBooking', Unit: 'Count' }], + }, ], - 'Metrics': [{ 'Name': 'successfulBooking', 'Unit': 'Count' }], - }], - }, - 'service': 'orders', - 'successfulBooking': 1, - })); - + }, + service: 'orders', + successfulBooking: 1, + }) + ); }); test('when an array of Metrics instances is passed, it prints the metrics in the stdout', async () => { - // Prepare - const metrics = new Metrics({ namespace:'serverlessAirline', serviceName:'orders' }); + const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' }); const lambdaHandler = (): void => { metrics.addMetric('successfulBooking', MetricUnits.Count, 1); }; const metricsOptions: ExtraOptions = { - throwOnEmptyMetrics: true + throwOnEmptyMetrics: true, }; const handler = middy(lambdaHandler).use(logMetrics([metrics], metricsOptions)); @@ -181,23 +329,23 @@ describe('Middy middleware', () => { await handler(event, context, () => console.log('Lambda invoked!')); // Assess - expect(console.log).toHaveBeenNthCalledWith(1, JSON.stringify({ - '_aws': { - 'Timestamp': 1466424490000, - 'CloudWatchMetrics': [{ - 'Namespace': 'serverlessAirline', - 'Dimensions': [ - ['service'] + expect(console.log).toHaveBeenNthCalledWith( + 1, + JSON.stringify({ + _aws: { + Timestamp: 1466424490000, + CloudWatchMetrics: [ + { + Namespace: 'serverlessAirline', + Dimensions: [['service']], + Metrics: [{ Name: 'successfulBooking', Unit: 'Count' }], + }, ], - 'Metrics': [{ 'Name': 'successfulBooking', 'Unit': 'Count' }], - }], - }, - 'service': 'orders', - 'successfulBooking': 1, - })); - + }, + service: 'orders', + successfulBooking: 1, + }) + ); }); - }); - -}); \ No newline at end of file +});