diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index a309186f0..27ad55a47 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -240,8 +240,10 @@ class Metrics extends Utility implements MetricsInterface { super(); this.dimensions = {}; - this.setOptions(options); + this.setEnvConfig(); + this.setConsole(); this.#logger = options.logger || this.console; + this.setOptions(options); } /** @@ -293,38 +295,16 @@ class Metrics extends Utility implements MetricsInterface { * @param dimensions - An object with key-value pairs of dimensions */ public addDimensions(dimensions: Dimensions): void { - const newDimensionSet: Dimensions = {}; - for (const [key, value] of Object.entries(dimensions)) { - 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` - ); - continue; - } - if ( - Object.hasOwn(this.dimensions, key) || - Object.hasOwn(this.defaultDimensions, key) || - Object.hasOwn(newDimensionSet, key) - ) { - this.#logger.warn( - `Dimension "${key}" has already been added. The previous value will be overwritten.` - ); - } - newDimensionSet[key] = value; - } - + const newDimensions = this.#sanitizeDimensions(dimensions); const currentCount = this.getCurrentDimensionsCount(); - const newSetCount = Object.keys(newDimensionSet).length; + const newSetCount = Object.keys(newDimensions).length; if (currentCount + newSetCount >= MAX_DIMENSION_COUNT) { throw new RangeError( `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}` ); } - this.dimensionSets.push(newDimensionSet); + this.dimensionSets.push(newDimensions); } /** @@ -823,15 +803,20 @@ class Metrics extends Utility implements MetricsInterface { * * @param dimensions - The dimensions to be added to the default dimensions object */ - public setDefaultDimensions(dimensions: Dimensions | undefined): void { - const targetDimensions = { + public setDefaultDimensions(dimensions: Dimensions): void { + const newDimensions = this.#sanitizeDimensions(dimensions); + const currentCount = Object.keys(this.defaultDimensions).length; + const newSetCount = Object.keys(newDimensions).length; + if (currentCount + newSetCount >= MAX_DIMENSION_COUNT) { + throw new RangeError( + `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}` + ); + } + + this.defaultDimensions = { ...this.defaultDimensions, - ...dimensions, + ...newDimensions, }; - if (MAX_DIMENSION_COUNT <= Object.keys(targetDimensions).length) { - throw new Error('Max dimension count hit'); - } - this.defaultDimensions = targetDimensions; } /** @@ -1058,13 +1043,11 @@ class Metrics extends Utility implements MetricsInterface { functionName, } = options; - this.setEnvConfig(); - this.setConsole(); this.setCustomConfigService(customConfigService); this.setDisabled(); this.setNamespace(namespace); this.setService(serviceName); - this.setDefaultDimensions(defaultDimensions); + this.setDefaultDimensions(defaultDimensions || {}); this.setFunctionNameForColdStartMetric(functionName); this.isSingleMetric = singleMetric || false; @@ -1072,7 +1055,7 @@ class Metrics extends Utility implements MetricsInterface { } /** - * Set the service to be used. + * Set the service dimension that will be included in the metrics. * * @param service - The service to be used */ @@ -1083,7 +1066,7 @@ class Metrics extends Utility implements MetricsInterface { this.#envConfig.serviceName || this.defaultServiceName; if (targetService.length > 0) { - this.setDefaultDimensions({ service: targetService }); + this.defaultDimensions = { service: targetService }; } } @@ -1191,6 +1174,38 @@ class Metrics extends Utility implements MetricsInterface { **/ return 0; } + + /** + * Sanitizes the dimensions by removing invalid entries and skipping duplicates. + * + * @param dimensions - The dimensions to sanitize. + */ + #sanitizeDimensions(dimensions: Dimensions): Dimensions { + const newDimensions: Dimensions = {}; + for (const [key, value] of Object.entries(dimensions)) { + 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` + ); + continue; + } + if ( + Object.hasOwn(this.dimensions, key) || + Object.hasOwn(this.defaultDimensions, key) || + Object.hasOwn(newDimensions, key) + ) { + this.#logger.warn( + `Dimension "${key}" has already been added. The previous value will be overwritten.` + ); + } + newDimensions[key] = value; + } + + return newDimensions; + } } export { Metrics }; diff --git a/packages/metrics/tests/unit/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts index 1a874ec02..5dc3a7b79 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -358,7 +358,7 @@ describe('Working with dimensions', () => { // Assess expect(() => metrics.setDefaultDimensions({ extra: 'test' })).toThrowError( - 'Max dimension count hit' + 'The number of metric dimensions must be lower than 29' ); }); @@ -552,4 +552,62 @@ describe('Working with dimensions', () => { }) ); }); + + it('warns when setDefaultDimensions overwrites existing dimensions', () => { + // Prepare + const metrics = new Metrics({ + namespace: DEFAULT_NAMESPACE, + defaultDimensions: { environment: 'prod' }, + }); + + // Act + metrics.setDefaultDimensions({ region: 'us-east-1' }); + metrics.setDefaultDimensions({ + environment: 'staging', // overwrites default dimension + }); + + // Assess + expect(console.warn).toHaveBeenCalledOnce(); + expect(console.warn).toHaveBeenCalledWith( + 'Dimension "environment" has already been added. The previous value will be overwritten.' + ); + }); + + it.each([ + { value: undefined, name: 'valid-name' }, + { value: null, name: 'valid-name' }, + { value: '', name: 'valid-name' }, + { value: 'valid-value', name: '' }, + ])( + 'skips invalid default dimension values in setDefaultDimensions ($name)', + ({ value, name }) => { + // Arrange + const metrics = new Metrics({ + singleMetric: true, + namespace: DEFAULT_NAMESPACE, + }); + + // Act + metrics.setDefaultDimensions({ + validDimension: 'valid', + [name as string]: value as string, + }); + + metrics.addMetric('test', MetricUnit.Count, 1); + metrics.publishStoredMetrics(); + + // Assess + expect(console.warn).toHaveBeenCalledWith( + `The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings` + ); + + expect(console.log).toHaveEmittedEMFWith( + expect.objectContaining({ validDimension: 'valid' }) + ); + + expect(console.log).toHaveEmittedEMFWith( + expect.not.objectContaining({ [name]: value }) + ); + } + ); });