Skip to content

Commit

Permalink
fix: change putDimensions to update existing dimension sets when dupl…
Browse files Browse the repository at this point in the history
…icate

This change ensures new dimension key-values are used for the EMF root node
by moving duplicates to the end of the collection.

Example:

```
[
  { "keyA": "value1" },
  { "keyA": "value2" },
]

// expected EMF target member: { "keyA": "value2 }
```

[TESTING]

Updated unit tests to check for this chase wherein putDimensions
may be triggered using various dimension set lengths, values, and order.
  • Loading branch information
Aaron Michael Lamb committed Nov 22, 2021
1 parent 8a07c48 commit 3b67d7f
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 13 deletions.
17 changes: 8 additions & 9 deletions src/logger/MetricsContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,27 +107,26 @@ export class MetricsContext {
/**
* Adds a new set of dimensions. Any time a new dimensions set
* is added, the set is first prepended by the default dimensions.
* Duplicate dimensions sets are removed before being added to the end of the collection.
* This ensures the latest dimension key-value is used as a target member on the root EMF node.
*
* @param dimensions
*/
public putDimensions(incomingDimensionSet: Record<string, string>): void {
const incomingDimensionSetKeys = Object.keys(incomingDimensionSet);

// Duplicate dimensions sets are removed before being added to the end of the collection.
// This ensures the latest dimension key-value is used as a target member on the root EMF node.
// This operation is O(n^2), but acceptable given sets are capped at 10 dimensions
const doesDimensionSetExist = this.dimensions.some(existingDimensionSet => {
const incomingDimensionSetKeys = Object.keys(incomingDimensionSet);
this.dimensions = this.dimensions.filter(existingDimensionSet => {
const existingDimensionSetKeys = Object.keys(existingDimensionSet);
if (existingDimensionSetKeys.length !== incomingDimensionSetKeys.length) {
return false;
return true;
}
return existingDimensionSetKeys.every(existingDimensionSetKey =>
return !existingDimensionSetKeys.every(existingDimensionSetKey =>
incomingDimensionSetKeys.includes(existingDimensionSetKey),
);
});

if (doesDimensionSetExist) {
return;
}

this.dimensions.push(incomingDimensionSet);
}

Expand Down
42 changes: 38 additions & 4 deletions src/logger/__tests__/MetricsContext.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,59 @@ test('putDimensions will not duplicate dimensions', () => {
const context = MetricsContext.empty();
const dimension1 = faker.random.word();
const dimension2 = faker.random.word();
const expectedDimension1 = { dimension1 };
const expectedDimension2 = { dimension1, dimension2 };
const expectedDimension3 = { dimension2 };
const expectedDimension1 = {};
const expectedDimension2 = { dimension1 };
const expectedDimension3 = { dimension2, dimension1 };
const expectedDimension4 = { dimension2 };

// act
context.putDimensions({});
context.putDimensions({ dimension1 });
context.putDimensions({ dimension1, dimension2 });
context.putDimensions({ dimension2, dimension1 });
context.putDimensions({ dimension2 });
context.putDimensions({});
context.putDimensions({ dimension1 });
context.putDimensions({ dimension1, dimension2 });
context.putDimensions({ dimension2, dimension1 });
context.putDimensions({ dimension2 });

// assert
expect(context.getDimensions().length).toBe(3);
expect(context.getDimensions().length).toBe(4);
expect(context.getDimensions()[0]).toStrictEqual(expectedDimension1);
expect(context.getDimensions()[1]).toStrictEqual(expectedDimension2);
expect(context.getDimensions()[2]).toStrictEqual(expectedDimension3);
expect(context.getDimensions()[3]).toStrictEqual(expectedDimension4);
});

test('putDimensions will order dimensions collection correctly', () => {
// arrange
const context = MetricsContext.empty();
const dimension1 = faker.random.word();
const dimension2 = faker.random.word();
const expectedDimension1 = { dimension2, dimension1 };
const expectedDimension2 = { dimension2 };
const expectedDimension3 = { dimension1 };
const expectedDimension4 = {};

// act
context.putDimensions({});
context.putDimensions({ dimension1 });
context.putDimensions({ dimension1, dimension2 });
context.putDimensions({ dimension2, dimension1 });
context.putDimensions({ dimension2 });
context.putDimensions({ dimension1, dimension2 });
context.putDimensions({ dimension2, dimension1 });
context.putDimensions({ dimension2 });
context.putDimensions({ dimension1 });
context.putDimensions({});

// assert
expect(context.getDimensions().length).toBe(4);
expect(context.getDimensions()[0]).toStrictEqual(expectedDimension1);
expect(context.getDimensions()[1]).toStrictEqual(expectedDimension2);
expect(context.getDimensions()[2]).toStrictEqual(expectedDimension3);
expect(context.getDimensions()[3]).toStrictEqual(expectedDimension4);
});

test('getDimensions returns default dimensions if custom dimensions not set', () => {
Expand Down

0 comments on commit 3b67d7f

Please sign in to comment.