Skip to content

Commit 20cd5c9

Browse files
committed
fix: address PR review comments
- Align addDimensions() validation with addDimension() method - Add proper value validation and duplicate dimension warnings - Update getCurrentDimensionsCount() to include dimensionSets - Fix dimension count validation logic
1 parent 2655769 commit 20cd5c9

File tree

1 file changed

+35
-17
lines changed

1 file changed

+35
-17
lines changed

packages/metrics/src/Metrics.ts

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -273,19 +273,35 @@ class Metrics extends Utility implements MetricsInterface {
273273
* @param dimensions - An object with key-value pairs of dimensions
274274
*/
275275
public addDimensions(dimensions: Dimensions): void {
276-
const newDimensionKeys = Object.keys(dimensions).filter(
277-
(key) => !this.defaultDimensions[key] && !this.dimensions[key]
278-
);
276+
const newDimensionSet: Dimensions = {};
277+
for (const [key, value] of Object.entries(dimensions)) {
278+
if (!value) {
279+
this.#logger.warn(
280+
`The dimension ${key} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
281+
);
282+
continue;
283+
}
284+
if (
285+
Object.hasOwn(this.dimensions, key) ||
286+
Object.hasOwn(this.defaultDimensions, key) ||
287+
Object.hasOwn(newDimensionSet, key)
288+
) {
289+
this.#logger.warn(
290+
`Dimension "${key}" has already been added. The previous value will be overwritten.`
291+
);
292+
}
293+
newDimensionSet[key] = value;
294+
}
279295

280-
if (
281-
newDimensionKeys.length + this.getCurrentDimensionsCount() >=
282-
MAX_DIMENSION_COUNT
283-
) {
296+
const currentCount = this.getCurrentDimensionsCount();
297+
const newSetCount = Object.keys(newDimensionSet).length;
298+
if (currentCount + newSetCount >= MAX_DIMENSION_COUNT) {
284299
throw new RangeError(
285300
`The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
286301
);
287302
}
288-
this.dimensionSets.push(dimensions);
303+
304+
this.dimensionSets.push(newDimensionSet);
289305
}
290306

291307
/**
@@ -746,14 +762,11 @@ class Metrics extends Utility implements MetricsInterface {
746762
},
747763
],
748764
},
749-
...Object.assign(
750-
{},
751-
this.defaultDimensions,
752-
this.dimensions,
753-
this.dimensionSets.reduce((acc, dims) => Object.assign(acc, dims), {}),
754-
metricValues,
755-
this.metadata
756-
),
765+
...this.defaultDimensions,
766+
...this.dimensions,
767+
...this.dimensionSets.reduce((acc, dims) => Object.assign(acc, dims), {}),
768+
...metricValues,
769+
...this.metadata,
757770
};
758771
}
759772

@@ -864,9 +877,14 @@ class Metrics extends Utility implements MetricsInterface {
864877
* Gets the current number of dimensions count.
865878
*/
866879
private getCurrentDimensionsCount(): number {
880+
const dimensionSetsCount = this.dimensionSets.reduce(
881+
(total, dimensionSet) => total + Object.keys(dimensionSet).length,
882+
0
883+
);
867884
return (
868885
Object.keys(this.dimensions).length +
869-
Object.keys(this.defaultDimensions).length
886+
Object.keys(this.defaultDimensions).length +
887+
dimensionSetsCount
870888
);
871889
}
872890

0 commit comments

Comments
 (0)