Skip to content

Commit 532d4ed

Browse files
committed
fix(metrics): Support multiple addMetric() call with the same metric name
1 parent a274c4e commit 532d4ed

File tree

6 files changed

+115
-12
lines changed

6 files changed

+115
-12
lines changed

packages/metrics/package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/metrics/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"commit": "commit",
1111
"test": "jest --group=unit --detectOpenHandles --coverage --verbose",
1212
"test:e2e": "jest --group=e2e",
13-
"watch": "jest --watch",
13+
"watch": "jest --group=unit --watch ",
1414
"build": "tsc",
1515
"lint": "eslint --ext .ts --fix --no-error-on-unmatched-pattern src tests",
1616
"format": "eslint --fix --ext .ts --fix --no-error-on-unmatched-pattern src tests",

packages/metrics/src/Metrics.ts

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ class Metrics implements MetricsInterface {
311311
if (!this.namespace) console.warn('Namespace should be defined, default used');
312312

313313
const metricValues = Object.values(this.storedMetrics).reduce(
314-
(result: { [key: string]: number }, { name, value }: { name: string; value: number }) => {
314+
(result: { [key: string]: number | number[] }, { name, value }: { name: string; value: number | number[] }) => {
315315
result[name] = value;
316316

317317
return result;
@@ -390,6 +390,19 @@ class Metrics implements MetricsInterface {
390390
return <EnvironmentVariablesService> this.envVarsService;
391391
}
392392

393+
private isNewMetric(name: string, unit: MetricUnit): boolean {
394+
if (this.storedMetrics[name]){
395+
// Inconsistent units indicates a bug or typos. We want to flag this to users early early
396+
if (this.storedMetrics[name].unit !== unit) {
397+
throw new Error('The same metric name has been added before with a different unit.');
398+
}
399+
400+
return false;
401+
} else {
402+
return true;
403+
}
404+
}
405+
393406
private setCustomConfigService(customConfigService?: ConfigServiceInterface): void {
394407
this.customConfigService = customConfigService ? customConfigService : undefined;
395408
}
@@ -430,12 +443,22 @@ class Metrics implements MetricsInterface {
430443
if (Object.keys(this.storedMetrics).length >= MAX_METRICS_SIZE) {
431444
this.purgeStoredMetrics();
432445
}
433-
this.storedMetrics[name] = {
434-
unit,
435-
value,
436-
name,
437-
};
446+
447+
if (this.isNewMetric(name, unit)) {
448+
this.storedMetrics[name] = {
449+
unit,
450+
value,
451+
name,
452+
};
453+
} else {
454+
const storedMetric = this.storedMetrics[name];
455+
if (!Array.isArray(storedMetric.value)) {
456+
storedMetric.value = [storedMetric.value];
457+
}
458+
storedMetric.value.push(value);
459+
}
438460
}
461+
439462
}
440463

441464
export { Metrics, MetricUnits };

packages/metrics/src/types/Metrics.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type ExtraOptions = {
5959
type StoredMetric = {
6060
name: string
6161
unit: MetricUnit
62-
value: number
62+
value: number | number[]
6363
};
6464

6565
type StoredMetrics = {

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,55 @@ describe('Class: Metrics', () => {
435435
expect(serializedMetrics._aws.CloudWatchMetrics[0].Namespace).toBe(DEFAULT_NAMESPACE);
436436
expect(console.warn).toHaveBeenNthCalledWith(1, 'Namespace should be defined, default used');
437437
});
438+
439+
test('Should contain a metric value if added once', ()=> {
440+
const metrics = new Metrics();
441+
442+
metrics.addMetric('test_name', MetricUnits.Count, 1);
443+
const serializedMetrics = metrics.serializeMetrics();
444+
445+
expect(serializedMetrics._aws.CloudWatchMetrics[0].Metrics.length).toBe(1);
446+
447+
expect(serializedMetrics['test_name']).toBe(1);
448+
});
449+
450+
test('Should convert multiple metrics with the same name and unit into an array', ()=> {
451+
const metrics = new Metrics();
452+
453+
metrics.addMetric('test_name', MetricUnits.Count, 2);
454+
metrics.addMetric('test_name', MetricUnits.Count, 1);
455+
const serializedMetrics = metrics.serializeMetrics();
456+
457+
expect(serializedMetrics._aws.CloudWatchMetrics[0].Metrics.length).toBe(1);
458+
expect(serializedMetrics['test_name']).toStrictEqual([ 2, 1 ]);
459+
});
460+
461+
test('Should throw an error if the same metric name is added again with a different unit', ()=> {
462+
const metrics = new Metrics();
463+
464+
metrics.addMetric('test_name', MetricUnits.Count, 2);
465+
try {
466+
metrics.addMetric('test_name', MetricUnits.Seconds, 10);
467+
} catch (e) {
468+
expect((<Error>e).message).toBe('The same metric name has been added before with a different unit.');
469+
}
470+
});
471+
472+
test('Should contain multiple metric values if added with multiple names', ()=> {
473+
const metrics = new Metrics();
474+
475+
metrics.addMetric('test_name', MetricUnits.Count, 1);
476+
metrics.addMetric('test_name2', MetricUnits.Count, 2);
477+
const serializedMetrics = metrics.serializeMetrics();
478+
479+
expect(serializedMetrics._aws.CloudWatchMetrics[0].Metrics).toStrictEqual([
480+
{ Name: 'test_name', Unit: 'Count' },
481+
{ Name: 'test_name2', Unit: 'Count' },
482+
]);
483+
484+
expect(serializedMetrics['test_name']).toBe(1);
485+
expect(serializedMetrics['test_name2']).toBe(2);
486+
});
438487
});
439488

440489
describe('Feature: Clearing Metrics ', () => {

packages/metrics/tests/unit/middleware/middy.test.ts

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,47 @@ describe('Middy middleware', () => {
4444
succeed: () => console.log('Succeeded!'),
4545
};
4646

47+
test('when a metrics instance receive multiple metrics with the same name, it prints multiple values in an array format', async () => {
48+
// Prepare
49+
const metrics = new Metrics({ namespace:'serverlessAirline', service:'orders' });
50+
51+
const lambdaHandler = (): void => {
52+
metrics.addMetric('successfulBooking', MetricUnits.Count, 2);
53+
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
54+
};
55+
56+
const handler = middy(lambdaHandler).use(logMetrics(metrics));
57+
58+
// Act
59+
await handler(event, context, () => console.log('Lambda invoked!'));
60+
61+
// Assess
62+
expect(console.log).toHaveBeenNthCalledWith(1, JSON.stringify({
63+
'_aws': {
64+
'Timestamp': 1466424490000,
65+
'CloudWatchMetrics': [{
66+
'Namespace': 'serverlessAirline',
67+
'Dimensions': [
68+
['service']
69+
],
70+
'Metrics': [{ 'Name': 'successfulBooking', 'Unit': 'Count' }],
71+
}],
72+
},
73+
'service': 'orders',
74+
'successfulBooking': [
75+
2,
76+
1,
77+
],
78+
}));
79+
});
80+
4781
test('when a metrics instance is passed WITH custom options, it prints the metrics in the stdout', async () => {
4882

4983
// Prepare
5084
const metrics = new Metrics({ namespace:'serverlessAirline', service:'orders' });
5185

5286
const lambdaHandler = (): void => {
5387
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
54-
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
5588
};
5689
const metricsOptions: ExtraOptions = {
5790
raiseOnEmptyMetrics: true,
@@ -107,7 +140,6 @@ describe('Middy middleware', () => {
107140

108141
const lambdaHandler = (): void => {
109142
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
110-
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
111143
};
112144

113145
const handler = middy(lambdaHandler).use(logMetrics(metrics));
@@ -140,7 +172,6 @@ describe('Middy middleware', () => {
140172

141173
const lambdaHandler = (): void => {
142174
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
143-
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
144175
};
145176
const metricsOptions: ExtraOptions = {
146177
raiseOnEmptyMetrics: true

0 commit comments

Comments
 (0)