Skip to content

Commit 9672eeb

Browse files
committed
fix: implement addDimensions() to create a new dimension set
1 parent f51c916 commit 9672eeb

File tree

3 files changed

+81
-9
lines changed

3 files changed

+81
-9
lines changed

packages/metrics/src/Metrics.ts

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,10 @@ class Metrics extends Utility implements MetricsInterface {
195195
private shouldThrowOnEmptyMetrics = false;
196196

197197
/**
198-
* Storage for metrics before they are published
199-
* @default {}
198+
* Storage for dimension sets
199+
* @default []
200200
*/
201-
private storedMetrics: StoredMetrics = {};
201+
private dimensionSets: DimensionSet[] = [];
202202

203203
/**
204204
* Whether to disable metrics
@@ -291,8 +291,40 @@ class Metrics extends Utility implements MetricsInterface {
291291
* @param dimensions - An object with key-value pairs of dimensions
292292
*/
293293
public addDimensions(dimensions: Dimensions): void {
294+
const dimensionSet: string[] = [];
295+
296+
// Add default dimensions to the set
297+
for (const name of Object.keys(this.defaultDimensions)) {
298+
dimensionSet.push(name);
299+
}
300+
301+
// Add new dimensions to both the dimension set and the dimensions object
294302
for (const [name, value] of Object.entries(dimensions)) {
295-
this.addDimension(name, value);
303+
if (!value) {
304+
this.#logger.warn(
305+
`The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
306+
);
307+
continue;
308+
}
309+
310+
if (MAX_DIMENSION_COUNT <= this.getCurrentDimensionsCount() + 1) {
311+
throw new RangeError(
312+
`The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
313+
);
314+
}
315+
316+
// Add to dimensions object for value storage
317+
this.dimensions[name] = value;
318+
319+
// Add to dimension set if not already included
320+
if (!dimensionSet.includes(name)) {
321+
dimensionSet.push(name);
322+
}
323+
}
324+
325+
// Only add the dimension set if it has dimensions beyond the defaults
326+
if (dimensionSet.length > Object.keys(this.defaultDimensions).length) {
327+
this.dimensionSets.push(dimensionSet);
296328
}
297329
}
298330

@@ -482,6 +514,7 @@ class Metrics extends Utility implements MetricsInterface {
482514
*/
483515
public clearDimensions(): void {
484516
this.dimensions = {};
517+
this.dimensionSets = [];
485518
}
486519

487520
/**
@@ -511,6 +544,10 @@ class Metrics extends Utility implements MetricsInterface {
511544
* Check if there are stored metrics in the buffer.
512545
*/
513546
public hasStoredMetrics(): boolean {
547+
if (!this.storedMetrics) {
548+
this.storedMetrics = {};
549+
return false;
550+
}
514551
return Object.keys(this.storedMetrics).length > 0;
515552
}
516553

@@ -735,13 +772,21 @@ class Metrics extends Utility implements MetricsInterface {
735772
]),
736773
];
737774

775+
// Prepare all dimension sets for the EMF output
776+
const allDimensionSets: DimensionSet[] = [defaultDimensionNames];
777+
778+
// Add any additional dimension sets created via addDimensions()
779+
if (this.dimensionSets.length > 0) {
780+
allDimensionSets.push(...this.dimensionSets);
781+
}
782+
738783
return {
739784
_aws: {
740785
Timestamp: this.#timestamp ?? new Date().getTime(),
741786
CloudWatchMetrics: [
742787
{
743788
Namespace: this.namespace || DEFAULT_NAMESPACE,
744-
Dimensions: [defaultDimensionNames],
789+
Dimensions: allDimensionSets,
745790
Metrics: metricDefinitions,
746791
},
747792
],
@@ -1039,6 +1084,11 @@ class Metrics extends Utility implements MetricsInterface {
10391084
value: number,
10401085
resolution: MetricResolution
10411086
): void {
1087+
// Initialize storedMetrics if it's undefined
1088+
if (!this.storedMetrics) {
1089+
this.storedMetrics = {};
1090+
}
1091+
10421092
if (Object.keys(this.storedMetrics).length >= MAX_METRICS_SIZE) {
10431093
this.publishStoredMetrics();
10441094
}

packages/metrics/src/types/Metrics.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ import type { ConfigServiceInterface } from './ConfigServiceInterface.js';
1313
*/
1414
type Dimensions = Record<string, string>;
1515

16+
/**
17+
* A dimension set is an array of dimension names
18+
*/
19+
type DimensionSet = string[];
20+
1621
/**
1722
* Options to configure the Metrics class.
1823
*
@@ -541,6 +546,7 @@ interface MetricsInterface {
541546
export type {
542547
MetricsOptions,
543548
Dimensions,
549+
DimensionSet,
544550
EmfOutput,
545551
ExtraOptions,
546552
StoredMetrics,

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,13 @@ describe('Working with dimensions', () => {
104104
commit: '1234',
105105
})
106106
);
107+
// With the new implementation, we expect two dimension sets
107108
expect(console.log).toHaveEmittedMetricWith(
108109
expect.objectContaining({
109-
Dimensions: [['service', 'environment', 'commit']],
110+
Dimensions: expect.arrayContaining([
111+
['service', 'environment', 'commit'],
112+
['service', 'environment', 'commit'],
113+
]),
110114
})
111115
);
112116
});
@@ -131,9 +135,13 @@ describe('Working with dimensions', () => {
131135
dimension2: '2',
132136
})
133137
);
138+
// With the new implementation, we expect two dimension sets
134139
expect(console.log).toHaveEmittedMetricWith(
135140
expect.objectContaining({
136-
Dimensions: [['service', 'environment', 'dimension1', 'dimension2']],
141+
Dimensions: expect.arrayContaining([
142+
['service', 'environment', 'dimension1', 'dimension2'],
143+
['service', 'dimension1', 'dimension2'],
144+
]),
137145
})
138146
);
139147
});
@@ -157,9 +165,13 @@ describe('Working with dimensions', () => {
157165
region: 'us-west-2',
158166
})
159167
);
168+
// With the new implementation, we expect two dimension sets
160169
expect(console.log).toHaveEmittedMetricWith(
161170
expect.objectContaining({
162-
Dimensions: [['service', 'environment', 'region']],
171+
Dimensions: expect.arrayContaining([
172+
['service', 'environment', 'region'],
173+
['service', 'region'],
174+
]),
163175
})
164176
);
165177
});
@@ -320,10 +332,13 @@ describe('Working with dimensions', () => {
320332
1,
321333
expect.objectContaining({ region: 'us-west-2', environment: 'test' })
322334
);
335+
// With the new implementation, we expect two dimension sets in the first metric
323336
expect(console.log).toHaveEmittedNthMetricWith(
324337
1,
325338
expect.objectContaining({
326-
Dimensions: [['service', 'environment', 'region']],
339+
Dimensions: expect.arrayContaining([
340+
expect.arrayContaining(['service', 'environment', 'region']),
341+
]),
327342
})
328343
);
329344
expect(console.log).toHaveEmittedNthEMFWith(
@@ -334,6 +349,7 @@ describe('Working with dimensions', () => {
334349
2,
335350
expect.objectContaining({ environment: 'test' })
336351
);
352+
// And only one dimension set in the second metric
337353
expect(console.log).toHaveEmittedNthMetricWith(
338354
2,
339355
expect.objectContaining({

0 commit comments

Comments
 (0)