Skip to content

fix(metrics): emit warning when default dimensions are overwritten #4222

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
40 changes: 35 additions & 5 deletions packages/metrics/src/Metrics.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Console } from 'node:console';
import {
isIntegerNumber,
isNullOrUndefined,
isNumber,
isRecord,
isString,
isStringUndefinedNullEmpty,
Utility,
Expand Down Expand Up @@ -240,8 +242,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);
}

/**
Expand Down Expand Up @@ -824,13 +828,41 @@ class Metrics extends Utility implements MetricsInterface {
* @param dimensions - The dimensions to be added to the default dimensions object
*/
public setDefaultDimensions(dimensions: Dimensions | undefined): void {
if (isNullOrUndefined(dimensions) || !isRecord(dimensions)) {
return;
}

const cleanedDimensions: 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.defaultDimensions, key)) {
this.#logger.warn(
`Dimension "${key}" has already been added. The previous value will be overwritten.`
);
}

cleanedDimensions[key] = value;
}

const targetDimensions = {
...this.defaultDimensions,
...dimensions,
...cleanedDimensions,
};
if (MAX_DIMENSION_COUNT <= Object.keys(targetDimensions).length) {

if (Object.keys(targetDimensions).length >= MAX_DIMENSION_COUNT) {
throw new Error('Max dimension count hit');
}

this.defaultDimensions = targetDimensions;
}

Expand Down Expand Up @@ -1058,8 +1090,6 @@ class Metrics extends Utility implements MetricsInterface {
functionName,
} = options;

this.setEnvConfig();
this.setConsole();
this.setCustomConfigService(customConfigService);
this.setDisabled();
this.setNamespace(namespace);
Expand Down
8 changes: 6 additions & 2 deletions packages/metrics/tests/unit/customTimestamp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ describe('Setting custom timestamp', () => {

it('logs a warning when the provided timestamp is too far in the past', () => {
// Prepare
const metrics = new Metrics({ singleMetric: true });
const metrics = new Metrics({
singleMetric: true,
});

// Act
metrics.setTimestamp(Date.now() - EMF_MAX_TIMESTAMP_PAST_AGE - 1000);
Expand All @@ -63,7 +65,9 @@ describe('Setting custom timestamp', () => {

it('logs a warning when the provided timestamp is too far in the future', () => {
// Prepare
const metrics = new Metrics({ singleMetric: true });
const metrics = new Metrics({
singleMetric: true,
});

// Act
metrics.setTimestamp(Date.now() + EMF_MAX_TIMESTAMP_FUTURE_AGE + 1000);
Expand Down
100 changes: 100 additions & 0 deletions packages/metrics/tests/unit/dimensions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -552,4 +552,104 @@ 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('returns immediately if dimensions is undefined', () => {
// Prepare
const metrics = new Metrics({
singleMetric: true,
namespace: DEFAULT_NAMESPACE,
});

// Act
metrics.addMetric('myMetric', MetricUnit.Count, 1);

// Assert
expect(console.warn).not.toHaveBeenCalled();

expect(console.log).toHaveEmittedEMFWith(
expect.objectContaining({
service: 'hello-world',
})
);
});

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();

// Assert
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 })
);
}
);
it('returns immediately without logging if dimensions is not a plain object', () => {
// Prepare
const metrics = new Metrics({
singleMetric: true,
namespace: DEFAULT_NAMESPACE,
});

// Act
// @ts-expect-error – simulate runtime misuse
metrics.setDefaultDimensions('not-an-object');

// Assert
expect(console.warn).not.toHaveBeenCalled();

metrics.addMetric('someMetric', MetricUnit.Count, 1);

expect(console.log).toHaveEmittedEMFWith(
expect.objectContaining({
service: 'hello-world',
})
);
});
});
4 changes: 3 additions & 1 deletion packages/metrics/tests/unit/initializeMetrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ describe('Initialize Metrics', () => {

it('uses the default namespace when none is provided', () => {
// Prepare
const metrics = new Metrics({ singleMetric: true });
const metrics = new Metrics({
singleMetric: true,
});

// Act
metrics.addMetric('test', MetricUnit.Count, 1);
Expand Down
Loading