feat(metrics): log warning on empty metrics (#1397)

Author: dreamorosi

Diff for: packages/metrics/src/Metrics.ts

@@ -330,6 +330,12 @@ class Metrics extends Utility implements MetricsInterface {
    * ```
    */
   public publishStoredMetrics(): void {
+    if (!this.shouldThrowOnEmptyMetrics && Object.keys(this.storedMetrics).length === 0) {
+      console.warn(
+        'No application metrics to publish. The cold-start metric may be published if enabled. ' +
+        'If application metrics should never be empty, consider using \'throwOnEmptyMetrics\'',
+      );
+    }
     const target = this.serializeMetrics();
     console.log(JSON.stringify(target));
     this.clearMetrics();

Diff for: packages/metrics/tests/unit/Metrics.test.ts

@@ -25,7 +25,7 @@ describe('Class: Metrics', () => {
   const event = dummyEvent.Custom.CustomEvent;
 
   beforeEach(() => {
-    consoleSpy.mockClear();
+    jest.clearAllMocks();
   });
 
   beforeAll(() => {
@@ -464,6 +464,65 @@ describe('Class: Metrics', () => {
         expect((<Error>e).message).toBe('The number of metrics recorded must be higher than zero');
       }
     });
+
+    test('when decorator is used with throwOnEmptyMetrics set to false, a warning should be logged', async () => {
+
+      // Prepare
+      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 async handler<TEvent, TResult>(
+          _event: TEvent,
+          _context: Context,
+        ): Promise<void | TResult> {
+          return;
+        }
+      }
+      const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
+
+      // Act
+      await new LambdaFunction().handler(event, context);
+
+      // Assess
+      expect(consoleWarnSpy).toBeCalledTimes(1);
+      expect(consoleWarnSpy).toBeCalledWith(
+        'No application metrics to publish. The cold-start metric may be published if enabled. If application metrics should never be empty, consider using \'throwOnEmptyMetrics\'',
+      );
+
+    });
+
+    test('when decorator is used with throwOnEmptyMetrics set to false & captureColdStartMetric set to true, a warning should be logged', async () => {
+
+      // Prepare
+      const metrics = new Metrics({ namespace: 'test' });
+      class LambdaFunction implements LambdaInterface {
+        @metrics.logMetrics({
+          throwOnEmptyMetrics: false,
+          captureColdStartMetric: true
+        })
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
+        public async handler<TEvent, TResult>(
+          _event: TEvent,
+          _context: Context,
+        ): Promise<void | TResult> {
+          return;
+        }
+      }
+      const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
+
+      // Act
+      await new LambdaFunction().handler(event, context);
+
+      // Assess
+      expect(consoleWarnSpy).toBeCalledTimes(1);
+      expect(consoleWarnSpy).toBeCalledWith(
+        'No application metrics to publish. The cold-start metric may be published if enabled. If application metrics should never be empty, consider using \'throwOnEmptyMetrics\'',
+      );
+
+    });
   });
 
   describe('Feature: Auto log at limit', () => {

Diff for: packages/metrics/tests/unit/middleware/middy.test.ts

@@ -13,8 +13,9 @@ import {
 import { ExtraOptions } from '../../../src/types';
 
 const consoleSpy = jest.spyOn(console, 'log').mockImplementation();
+const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
 const mockDate = new Date(1466424490000);
-const dateSpy = jest.spyOn(global, 'Date').mockImplementation(() => mockDate);
+jest.spyOn(global, 'Date').mockImplementation(() => mockDate);
 
 describe('Middy middleware', () => {
 
@@ -40,8 +41,7 @@ describe('Middy middleware', () => {
 
   beforeEach(() => {
     jest.resetModules();
-    consoleSpy.mockClear();
-    dateSpy.mockClear();
+    jest.clearAllMocks();
   });
 
   describe('throwOnEmptyMetrics', () => {
@@ -80,21 +80,22 @@ describe('Middy middleware', () => {
       }
     });
 
-    test('should not throw on empty metrics if not set', async () => {
+    test('should not throw on empty metrics if not set, but should log a warning', async () => {
+
       // Prepare
       const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' });
-
-      const lambdaHandler = (): void => {
+      const lambdaHandler = async (): Promise<void> => {
         console.log('do nothing');
       };
-
       const handler = middy(lambdaHandler).use(logMetrics(metrics));
 
-      try {
-        await handler(dummyEvent, dummyContext, () => console.log('Lambda invoked!'));
-      } catch (e) {
-        fail(`Should not throw but got the following Error: ${e}`);
-      }
+      // Act & Assess
+      await expect(handler(dummyEvent, dummyContext)).resolves.not.toThrowError();
+      expect(consoleWarnSpy).toBeCalledTimes(1);
+      expect(consoleWarnSpy).toBeCalledWith(
+        'No application metrics to publish. The cold-start metric may be published if enabled. If application metrics should never be empty, consider using \'throwOnEmptyMetrics\'',
+      );
+
     });
   });