Skip to content

Commit 1c3040a

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

File tree

3 files changed

+92
-97
lines changed

3 files changed

+92
-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: 59 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,58 @@ class MetricsStore {
3036
return this.#getStorage()[name];
3137
}
3238

33-
public setMetric(name: string, metric: StoredMetric): void {
34-
this.#getStorage()[name] = metric;
39+
/**
40+
* Adds a metric value to storage. If a metric with the same name already exists,
41+
* the value is appended to an array. Validates that the unit matches any existing metric.
42+
*
43+
* @example
44+
* ```typescript
45+
* store.setMetric('latency', MetricUnit.Milliseconds, 100);
46+
* // Returns: { name: 'latency', unit: 'Milliseconds', value: 100, resolution: 60 }
47+
*
48+
* store.setMetric('latency', MetricUnit.Milliseconds, 150);
49+
* // Returns: { name: 'latency', unit: 'Milliseconds', value: [100, 150], resolution: 60 }
50+
* ```
51+
*
52+
* @param name - The metric name
53+
* @param unit - The metric unit (must match existing metric if present)
54+
* @param value - The metric value to add
55+
* @param resolution - The metric resolution (defaults to Standard)
56+
* @returns The stored metric with updated values
57+
* @throws Error if unit doesn't match existing metric
58+
*/
59+
public setMetric(
60+
name: string,
61+
unit: MetricUnit,
62+
value: number,
63+
resolution: MetricResolution = MetricResolutions.Standard
64+
): StoredMetric {
65+
const storage = this.#getStorage();
66+
const existingMetric = storage[name];
67+
68+
if (!existingMetric) {
69+
const newMetric: StoredMetric = {
70+
name,
71+
unit,
72+
value,
73+
resolution,
74+
};
75+
storage[name] = newMetric;
76+
return newMetric;
77+
}
78+
79+
if (existingMetric.unit !== unit) {
80+
const currentUnit = existingMetric.unit;
81+
throw new Error(
82+
`Metric "${name}" has already been added with unit "${currentUnit}", but we received unit "${unit}". Did you mean to use metric unit "${currentUnit}"?`
83+
);
84+
}
85+
86+
if (!Array.isArray(existingMetric.value)) {
87+
existingMetric.value = [existingMetric.value];
88+
}
89+
existingMetric.value.push(value);
90+
return existingMetric;
3591
}
3692

3793
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)