Skip to content

improv(metrics): Added runtime validations for the metrics utility functions #4181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/commons/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export {
isRecord,
isStrictEqual,
isString,
isStringUndefinedNullEmpty,
isTruthy,
} from './typeUtils.js';
export { Utility } from './Utility.js';
Expand Down
51 changes: 37 additions & 14 deletions packages/commons/src/typeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* }
* ```
*
* @param value The value to check
* @param value - The value to check
*/
const isRecord = (
value: unknown
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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);
Expand All @@ -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)) {
Expand Down Expand Up @@ -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);
Expand All @@ -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.
*
Expand All @@ -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)) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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>,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -314,6 +336,7 @@ export {
isTruthy,
isNull,
isNullOrUndefined,
isStringUndefinedNullEmpty,
getType,
isStrictEqual,
};
33 changes: 33 additions & 0 deletions packages/commons/tests/unit/typeUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
isRecord,
isStrictEqual,
isString,
isStringUndefinedNullEmpty,
isTruthy,
} from '../../src/index.js';

Expand Down Expand Up @@ -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
Expand Down
36 changes: 33 additions & 3 deletions packages/metrics/src/Metrics.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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`
);
Expand Down Expand Up @@ -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`
);
Expand Down Expand Up @@ -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();
}
Expand Down
10 changes: 10 additions & 0 deletions packages/metrics/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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,
Expand Down
80 changes: 80 additions & 0 deletions packages/metrics/tests/unit/creatingMetrics.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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(',')}`
)
);
});
});
Loading