improv(metrics): add runtime validations when adding dimensions (#4181)

Author: sdangol

packages/commons/src/index.ts

@@ -22,6 +22,7 @@ export {
   isRecord,
   isStrictEqual,
   isString,
+  isStringUndefinedNullEmpty,
   isTruthy,
 } from './typeUtils.js';
 export { Utility } from './Utility.js';

packages/commons/src/typeUtils.ts

@@ -11,7 +11,7 @@
  * }
  * ```
  *
- * @param value The value to check
+ * @param value - The value to check
  */
 const isRecord = (
   value: unknown
@@ -35,7 +35,7 @@ const isRecord = (
  * }
  * ```
  *
- * @param value The value to check
+ * @param value - The value to check
  */
 const isString = (value: unknown): value is string => {
   return typeof value === 'string';
@@ -54,7 +54,7 @@ const isString = (value: unknown): value is string => {
  * }
  * ```
  *
- * @param value The value to check
+ * @param value - The value to check
  */
 const isNumber = (value: unknown): value is number => {
   return typeof value === 'number';
@@ -73,7 +73,7 @@ const isNumber = (value: unknown): value is number => {
  * }
  * ```
  *
- * @param value The value to check
+ * @param value - The value to check
  */
 const isIntegerNumber = (value: unknown): value is number => {
   return isNumber(value) && Number.isInteger(value);
@@ -94,7 +94,7 @@ const isIntegerNumber = (value: unknown): value is number => {
  *
  * @see https://github.com/getify/You-Dont-Know-JS/blob/2nd-ed/types-grammar/ch4.md#toboolean
  *
- * @param value The value to check
+ * @param value - The value to check
  */
 const isTruthy = (value: unknown): boolean => {
   if (isString(value)) {
@@ -129,7 +129,7 @@ const isTruthy = (value: unknown): boolean => {
  * }
  * ```
  *
- * @param value The value to check
+ * @param value - The value to check
  */
 const isNull = (value: unknown): value is null => {
   return Object.is(value, null);
@@ -148,12 +148,34 @@ const isNull = (value: unknown): value is null => {
  * }
  * ```
  *
- * @param value The value to check
+ * @param value - The value to check
  */
 const isNullOrUndefined = (value: unknown): value is null | undefined => {
   return isNull(value) || Object.is(value, undefined);
 };
 
+/**
+ * Check if string is undefined, null, empty.
+ *
+ * @example
+ * ```typescript
+ * import { isStringUndefinedNullEmpty } from '@aws-lambda-powertools/commons/typeUtils';
+ *
+ * const value = 'foo';
+ * if (isStringUndefinedNullEmpty(value)) {
+ *   // value is either undefined, null, or an empty string
+ * }
+ * ```
+ *
+ * @param value - The value to check
+ */
+const isStringUndefinedNullEmpty = (value: unknown) => {
+  if (isNullOrUndefined(value)) return true;
+  if (!isString(value)) return true;
+  if (value.trim().length === 0) return true;
+  return false;
+};
+
 /**
  * Get the type of a value as a string.
  *
@@ -167,7 +189,7 @@ const isNullOrUndefined = (value: unknown): value is null | undefined => {
  * const unknownType = getType(Symbol('foo')); // 'unknown'
  * ```
  *
- * @param value The value to check
+ * @param value - The value to check
  */
 const getType = (value: unknown): string => {
   if (Array.isArray(value)) {
@@ -210,8 +232,8 @@ const getType = (value: unknown): string => {
  * const otherEqual = areArraysEqual(otherLeft, otherRight); // false
  * ```
  *
- * @param left The left array to compare
- * @param right The right array to compare
+ * @param left - The left array to compare
+ * @param right - The right array to compare
  */
 const areArraysEqual = (left: unknown[], right: unknown[]): boolean => {
   if (left.length !== right.length) {
@@ -237,8 +259,8 @@ const areArraysEqual = (left: unknown[], right: unknown[]): boolean => {
  * const otherEqual = areRecordsEqual(otherLeft, otherRight); // false
  * ```
  *
- * @param left The left record to compare
- * @param right The right record to compare
+ * @param left - The left record to compare
+ * @param right - The right record to compare
  */
 const areRecordsEqual = (
   left: Record<string, unknown>,
@@ -283,8 +305,8 @@ const areRecordsEqual = (
  * const yetAnotherEqual = isStrictEqual(yetAnotherLeft, yetAnotherRight); // true
  * ```
  *
- * @param left Left side of strict equality comparison
- * @param right Right side of strict equality comparison
+ * @param left - Left side of strict equality comparison
+ * @param right - Right side of strict equality comparison
  */
 const isStrictEqual = (left: unknown, right: unknown): boolean => {
   if (left === right) {
@@ -314,6 +336,7 @@ export {
   isTruthy,
   isNull,
   isNullOrUndefined,
+  isStringUndefinedNullEmpty,
   getType,
   isStrictEqual,
 };

packages/commons/tests/unit/typeUtils.test.ts

@@ -8,6 +8,7 @@ import {
   isRecord,
   isStrictEqual,
   isString,
+  isStringUndefinedNullEmpty,
   isTruthy,
 } from '../../src/index.js';
 
@@ -119,6 +120,38 @@ describe('Functions: typeUtils', () => {
     });
   });
 
+  describe('Function: isStringUndefinedNullEmpty', () => {
+    it('returns true if input is undefined', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty(undefined)).toBe(true);
+    });
+
+    it('returns true if input is null', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty(null)).toBe(true);
+    });
+
+    it('returns true if input is an empty string', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty('')).toBe(true);
+    });
+
+    it('returns true if input is a whitespace', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty(' ')).toBe(true);
+    });
+
+    it('returns true if input is not a string', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty(1)).toBe(true);
+    });
+
+    it('returns false if input is not undefined, null, or an empty string', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty('test')).toBe(false);
+    });
+  });
+
   describe('Function: isNumber', () => {
     it('returns true when the passed value is a number', () => {
       // Prepare

packages/metrics/src/Metrics.ts

@@ -1,5 +1,11 @@
 import { Console } from 'node:console';
-import { isIntegerNumber, Utility } from '@aws-lambda-powertools/commons';
+import {
+  isIntegerNumber,
+  isNumber,
+  isString,
+  isStringUndefinedNullEmpty,
+  Utility,
+} from '@aws-lambda-powertools/commons';
 import type {
   GenericLogger,
   HandlerMethodDecorator,
@@ -12,10 +18,12 @@ import {
   EMF_MAX_TIMESTAMP_FUTURE_AGE,
   EMF_MAX_TIMESTAMP_PAST_AGE,
   MAX_DIMENSION_COUNT,
+  MAX_METRIC_NAME_LENGTH,
   MAX_METRIC_VALUES_SIZE,
   MAX_METRICS_SIZE,
   MetricResolution as MetricResolutions,
   MetricUnit as MetricUnits,
+  MIN_METRIC_NAME_LENGTH,
 } from './constants.js';
 import type {
   ConfigServiceInterface,
@@ -238,7 +246,7 @@ class Metrics extends Utility implements MetricsInterface {
    * @param value - The value of the dimension
    */
   public addDimension(name: string, value: string): void {
-    if (!value) {
+    if (isStringUndefinedNullEmpty(name) || isStringUndefinedNullEmpty(value)) {
       this.#logger.warn(
         `The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
       );
@@ -275,7 +283,10 @@ class Metrics extends Utility implements MetricsInterface {
   public addDimensions(dimensions: Dimensions): void {
     const newDimensionSet: Dimensions = {};
     for (const [key, value] of Object.entries(dimensions)) {
-      if (!value) {
+      if (
+        isStringUndefinedNullEmpty(key) ||
+        isStringUndefinedNullEmpty(value)
+      ) {
         this.#logger.warn(
           `The dimension ${key} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
         );
@@ -1067,6 +1078,25 @@ class Metrics extends Utility implements MetricsInterface {
     value: number,
     resolution: MetricResolution
   ): void {
+    if (!isString(name)) throw new Error(`${name} is not a valid string`);
+    if (
+      name.length < MIN_METRIC_NAME_LENGTH ||
+      name.length > MAX_METRIC_NAME_LENGTH
+    )
+      throw new RangeError(
+        `The metric name should be between ${MIN_METRIC_NAME_LENGTH} and ${MAX_METRIC_NAME_LENGTH} characters`
+      );
+    if (!isNumber(value))
+      throw new RangeError(`${value} is not a valid number`);
+    if (!Object.values(MetricUnits).includes(unit))
+      throw new RangeError(
+        `Invalid metric unit '${unit}', expected either option: ${Object.values(MetricUnits).join(',')}`
+      );
+    if (!Object.values(MetricResolutions).includes(resolution))
+      throw new RangeError(
+        `Invalid metric resolution '${resolution}', expected either option: ${Object.values(MetricResolutions).join(',')}`
+      );
+
     if (Object.keys(this.storedMetrics).length >= MAX_METRICS_SIZE) {
       this.publishStoredMetrics();
     }

packages/metrics/src/constants.ts

@@ -6,6 +6,14 @@ const COLD_START_METRIC = 'ColdStart';
  * The default namespace for metrics.
  */
 const DEFAULT_NAMESPACE = 'default_namespace';
+/**
+ * The minimum length constraint of the metric name
+ */
+const MIN_METRIC_NAME_LENGTH = 1;
+/**
+ * The maximum length constraint of the metric name
+ */
+const MAX_METRIC_NAME_LENGTH = 255;
 /**
  * The maximum number of metrics that can be emitted in a single EMF blob.
  */
@@ -78,6 +86,8 @@ const MetricResolution = {
 export {
   COLD_START_METRIC,
   DEFAULT_NAMESPACE,
+  MIN_METRIC_NAME_LENGTH,
+  MAX_METRIC_NAME_LENGTH,
   MAX_METRICS_SIZE,
   MAX_METRIC_VALUES_SIZE,
   MAX_DIMENSION_COUNT,

packages/metrics/tests/unit/creatingMetrics.test.ts

@@ -1,8 +1,10 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import {
   DEFAULT_NAMESPACE,
+  MAX_METRIC_NAME_LENGTH,
   MAX_METRICS_SIZE,
   MetricResolution,
+  MIN_METRIC_NAME_LENGTH,
 } from '../../src/constants.js';
 import { Metrics, MetricUnit } from '../../src/index.js';
 
@@ -267,4 +269,82 @@ describe('Creating metrics', () => {
       })
     );
   });
+
+  it('throws when an invalid metric name is passed', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    // @ts-expect-error - Testing runtime behavior with non-numeric metric value
+    expect(() => metrics.addMetric(1, MetricUnit.Count, 1)).toThrowError(
+      '1 is not a valid string'
+    );
+  });
+
+  it('throws when an empty string is passed in the metric name', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    expect(() => metrics.addMetric('', MetricUnit.Count, 1)).toThrowError(
+      `The metric name should be between ${MIN_METRIC_NAME_LENGTH} and ${MAX_METRIC_NAME_LENGTH} characters`
+    );
+  });
+
+  it(`throws when a string of more than ${MAX_METRIC_NAME_LENGTH} characters is passed in the metric name`, () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    expect(() =>
+      metrics.addMetric(
+        'Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis,.',
+        MetricUnit.Count,
+        1
+      )
+    ).toThrowError(
+      new RangeError(
+        `The metric name should be between ${MIN_METRIC_NAME_LENGTH} and ${MAX_METRIC_NAME_LENGTH} characters`
+      )
+    );
+  });
+
+  it('throws when a non-numeric metric value is passed', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    expect(() =>
+      // @ts-expect-error - Testing runtime behavior with non-numeric metric value
+      metrics.addMetric('test', MetricUnit.Count, 'one')
+    ).toThrowError(new RangeError('one is not a valid number'));
+  });
+
+  it('throws when an invalid unit is passed', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    // @ts-expect-error - Testing runtime behavior with invalid metric unit
+    expect(() => metrics.addMetric('test', 'invalid-unit', 1)).toThrowError(
+      new RangeError(
+        `Invalid metric unit 'invalid-unit', expected either option: ${Object.values(MetricUnit).join(',')}`
+      )
+    );
+  });
+
+  it('throws when an invalid resolution is passed', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    expect(() =>
+      // @ts-expect-error - Testing runtime behavior with invalid metric unit
+      metrics.addMetric('test', MetricUnit.Count, 1, 'invalid-resolution')
+    ).toThrowError(
+      new RangeError(
+        `Invalid metric resolution 'invalid-resolution', expected either option: ${Object.values(MetricResolution).join(',')}`
+      )
+    );
+  });
 });