Skip to content

Commit 62d0f60

Browse files
svozzasvozza
committed
fix sonarqube complexity finding
1 parent bc27232 commit 62d0f60

File tree

3 files changed

+72
-97
lines changed

3 files changed

+72
-97
lines changed

‎packages/metrics/src/Metrics.ts‎

Lines changed: 11 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -892,34 +892,6 @@ class Metrics extends Utility implements MetricsInterface {
892892
return this.customConfigService;
893893
}
894894

895-
/**
896-
* Check if a metric is new or not.
897-
*
898-
* A metric is considered new if there is no metric with the same name already stored.
899-
*
900-
* When a metric is not new, we also check if the unit is consistent with the stored metric with
901-
* the same name. If the units are inconsistent, we throw an error as this is likely a bug or typo.
902-
* This can happen if a metric is added without using the `MetricUnit` helper in JavaScript codebases.
903-
*
904-
* @param name - The name of the metric
905-
* @param unit - The unit of the metric
906-
*/
907-
private isNewMetric(name: string, unit: MetricUnit): boolean {
908-
const storedMetric = this.#storedMetrics.getMetric(name);
909-
910-
if (storedMetric) {
911-
if (storedMetric.unit !== unit) {
912-
const currentUnit = storedMetric.unit;
913-
throw new Error(
914-
`Metric "${name}" has already been added with unit "${currentUnit}", but we received unit "${unit}". Did you mean to use metric unit "${currentUnit}"?`
915-
);
916-
}
917-
918-
return false;
919-
}
920-
return true;
921-
}
922-
923895
/**
924896
* Initialize the console property as an instance of the internal version of `Console()` class (PR #748)
925897
* or as the global node console if the `POWERTOOLS_DEV' env variable is set and has truthy value.
@@ -1097,24 +1069,17 @@ class Metrics extends Utility implements MetricsInterface {
10971069
this.publishStoredMetrics();
10981070
}
10991071

1100-
if (this.isNewMetric(name, unit)) {
1101-
this.#storedMetrics.setMetric(name, {
1102-
unit,
1103-
value,
1104-
name,
1105-
resolution,
1106-
});
1107-
} else {
1108-
const storedMetric = this.#storedMetrics.getMetric(name);
1109-
if (storedMetric) {
1110-
if (!Array.isArray(storedMetric.value)) {
1111-
storedMetric.value = [storedMetric.value];
1112-
}
1113-
storedMetric.value.push(value);
1114-
if (storedMetric.value.length === MAX_METRIC_VALUES_SIZE) {
1115-
this.publishStoredMetrics();
1116-
}
1117-
}
1072+
const storedMetric = this.#storedMetrics.setMetric(
1073+
name,
1074+
unit,
1075+
value,
1076+
resolution
1077+
);
1078+
if (
1079+
Array.isArray(storedMetric.value) &&
1080+
storedMetric.value.length === MAX_METRIC_VALUES_SIZE
1081+
) {
1082+
this.publishStoredMetrics();
11181083
}
11191084
}
11201085

‎packages/metrics/src/MetricsStore.ts‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { InvokeStore } from '@aws/lambda-invoke-store';
2-
import type { StoredMetric, StoredMetrics } from './types/index.js';
2+
import { MetricResolution as MetricResolutions } from './constants.js';
3+
import type {
4+
MetricResolution,
5+
MetricUnit,
6+
StoredMetric,
7+
StoredMetrics,
8+
} from './types/index.js';
39

410
/**
511
* Manages storage of metrics with automatic context detection.
@@ -30,8 +36,38 @@ class MetricsStore {
3036
return this.#getStorage()[name];
3137
}
3238

33-
public setMetric(name: string, metric: StoredMetric): void {
34-
this.#getStorage()[name] = metric;
39+
public setMetric(
40+
name: string,
41+
unit: MetricUnit,
42+
value: number,
43+
resolution: MetricResolution = MetricResolutions.Standard
44+
): StoredMetric {
45+
const storage = this.#getStorage();
46+
const existingMetric = storage[name];
47+
48+
if (!existingMetric) {
49+
const newMetric: StoredMetric = {
50+
name,
51+
unit,
52+
value,
53+
resolution,
54+
};
55+
storage[name] = newMetric;
56+
return newMetric;
57+
}
58+
59+
if (existingMetric.unit !== unit) {
60+
const currentUnit = existingMetric.unit;
61+
throw new Error(
62+
`Metric "${name}" has already been added with unit "${currentUnit}", but we received unit "${unit}". Did you mean to use metric unit "${currentUnit}"?`
63+
);
64+
}
65+
66+
if (!Array.isArray(existingMetric.value)) {
67+
existingMetric.value = [existingMetric.value];
68+
}
69+
existingMetric.value.push(value);
70+
return existingMetric;
3571
}
3672

3773
public getMetricNames(): string[] {

‎packages/metrics/tests/unit/concurrency/metricsStore.test.ts‎

Lines changed: 22 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ describe('MetricsStore concurrent invocation isolation', () => {
1616
expectedResult1: {
1717
name: 'count',
1818
unit: MetricUnit.Count,
19-
value: 2,
19+
value: [1, 2],
2020
resolution: 60,
2121
},
2222
expectedResult2: {
2323
name: 'count',
2424
unit: MetricUnit.Count,
25-
value: 2,
25+
value: [1, 2],
2626
resolution: 60,
2727
},
2828
},
@@ -47,27 +47,19 @@ describe('MetricsStore concurrent invocation isolation', () => {
4747
async ({ useInvokeStore, expectedResult1, expectedResult2 }) => {
4848
// Prepare
4949
const store = new MetricsStore();
50-
const metric1: StoredMetric = {
51-
name: 'count',
52-
unit: MetricUnit.Count,
53-
value: 1,
54-
resolution: 60,
55-
};
56-
const metric2: StoredMetric = {
57-
name: 'count',
58-
unit: MetricUnit.Count,
59-
value: 2,
60-
resolution: 60,
61-
};
6250

6351
// Act
6452
const [result1, result2] = await sequence(
6553
{
66-
sideEffects: [() => store.setMetric('count', metric1)],
54+
sideEffects: [
55+
() => store.setMetric('count', MetricUnit.Count, 1, 60),
56+
],
6757
return: () => store.getMetric('count'),
6858
},
6959
{
70-
sideEffects: [() => store.setMetric('count', metric2)],
60+
sideEffects: [
61+
() => store.setMetric('count', MetricUnit.Count, 2, 60),
62+
],
7163
return: () => store.getMetric('count'),
7264
},
7365
{ useInvokeStore }
@@ -122,16 +114,16 @@ describe('MetricsStore concurrent invocation isolation', () => {
122114
{
123115
sideEffects: [
124116
() => {
125-
store.setMetric('count', countMetric);
126-
store.setMetric('latency', latencyMetric);
117+
store.setMetric('count', MetricUnit.Count, 1, 60);
118+
store.setMetric('latency', MetricUnit.Milliseconds, 100, 60);
127119
},
128120
],
129121
return: () => store.getAllMetrics(),
130122
},
131123
{
132124
sideEffects: [
133125
() => {
134-
store.setMetric('errors', errorMetric);
126+
store.setMetric('errors', MetricUnit.Count, 1, 60);
135127
},
136128
],
137129
return: () => store.getAllMetrics(),
@@ -203,19 +195,13 @@ describe('MetricsStore concurrent invocation isolation', () => {
203195
async ({ useInvokeStore, expectedResult1, expectedResult2 }) => {
204196
// Prepare
205197
const store = new MetricsStore();
206-
const countMetric: StoredMetric = {
207-
name: 'count',
208-
unit: MetricUnit.Count,
209-
value: 1,
210-
resolution: 60,
211-
};
212198

213199
// Act
214200
const [result1, result2] = await sequence(
215201
{
216202
sideEffects: [
217203
() => {
218-
store.setMetric('count', countMetric);
204+
store.setMetric('count', MetricUnit.Count, 1, 60);
219205
store.setTimestamp(1000);
220206
},
221207
() => {}, // Wait for inv2 to add
@@ -230,7 +216,7 @@ describe('MetricsStore concurrent invocation isolation', () => {
230216
sideEffects: [
231217
() => {}, // Wait for inv1 to add
232218
() => {
233-
store.setMetric('errors', errorMetric);
219+
store.setMetric('errors', MetricUnit.Count, 1, 60);
234220
store.setTimestamp(2000);
235221
},
236222
() => {}, // Wait for clear
@@ -267,17 +253,13 @@ describe('MetricsStore concurrent invocation isolation', () => {
267253
async ({ useInvokeStore, expectedResult1, expectedResult2 }) => {
268254
// Prepare
269255
const store = new MetricsStore();
270-
const metric: StoredMetric = {
271-
name: 'count',
272-
unit: MetricUnit.Count,
273-
value: 1,
274-
resolution: 60,
275-
};
276256

277257
// Act
278258
const [result1, result2] = await sequence(
279259
{
280-
sideEffects: [() => store.setMetric('count', metric)],
260+
sideEffects: [
261+
() => store.setMetric('count', MetricUnit.Count, 1, 60),
262+
],
281263
return: () => store.hasMetrics(),
282264
},
283265
{
@@ -311,27 +293,19 @@ describe('MetricsStore concurrent invocation isolation', () => {
311293
async ({ useInvokeStore, expectedResult1, expectedResult2 }) => {
312294
// Prepare
313295
const store = new MetricsStore();
314-
const metric1: StoredMetric = {
315-
name: 'count',
316-
unit: MetricUnit.Count,
317-
value: 1,
318-
resolution: 60,
319-
};
320-
const metric2: StoredMetric = {
321-
name: 'errors',
322-
unit: MetricUnit.Count,
323-
value: 1,
324-
resolution: 60,
325-
};
326296

327297
// Act
328298
const [result1, result2] = await sequence(
329299
{
330-
sideEffects: [() => store.setMetric('count', metric1)],
300+
sideEffects: [
301+
() => store.setMetric('count', MetricUnit.Count, 1, 60),
302+
],
331303
return: () => store.getMetricsCount(),
332304
},
333305
{
334-
sideEffects: [() => store.setMetric('errors', metric2)],
306+
sideEffects: [
307+
() => store.setMetric('errors', MetricUnit.Count, 1, 60),
308+
],
335309
return: () => store.getMetricsCount(),
336310
},
337311
{ useInvokeStore }

0 commit comments

Comments
 (0)