Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: prevent putDimensions from storing duplicate dimensions #104

Merged
merged 3 commits into from
Aug 12, 2022
Merged

fix: prevent putDimensions from storing duplicate dimensions #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a07c48
fix: prevent putDimensions from storing duplicate dimensions
Nov 22, 2021
a9705f2
fix: change putDimensions to update/sort existing dimension sets when…
Nov 22, 2021
da7cf71
Merge branch 'master' into fix/duplicate-dimensions
markkuhn Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 12 additions & 24 deletions src/logger/MetricsContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,35 +124,23 @@ export class MetricsContext {
* @param dimensions
*/
public putDimensions(incomingDimensionSet: Record<string, string>): void {
MetricsContext.validateDimensionSet(incomingDimensionSet)
MetricsContext.validateDimensionSet(incomingDimensionSet);

if (this.dimensions.length === 0) {
this.dimensions.push(incomingDimensionSet);
return;
}

for (let i = 0; i < this.dimensions.length; i++) {
const existingDimensionSet = this.dimensions[i];

// check for duplicate dimensions when putting
// this is an O(n^2) operation, but since we never expect to have more than
// 10 dimensions, this is acceptable for almost all cases.
// This makes re-using loggers much easier.
// 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 incomingDimensionSetKeys = Object.keys(incomingDimensionSet);
this.dimensions = this.dimensions.filter(existingDimensionSet => {
const existingDimensionSetKeys = Object.keys(existingDimensionSet);
const incomingDimensionSetKeys = Object.keys(incomingDimensionSet);
if (existingDimensionSetKeys.length !== incomingDimensionSetKeys.length) {
this.dimensions.push(incomingDimensionSet);
return;
return true;
}
return !existingDimensionSetKeys.every(existingDimensionSetKey =>
incomingDimensionSetKeys.includes(existingDimensionSetKey),
);
});

for (let j = 0; j < existingDimensionSetKeys.length; j++) {
if (!incomingDimensionSetKeys.includes(existingDimensionSetKeys[j])) {
// we're done now because we know that the dimensions keys are not identical
this.dimensions.push(incomingDimensionSet);
return;
}
}
}
this.dimensions.push(incomingDimensionSet);
}

/**
Expand Down
64 changes: 47 additions & 17 deletions src/logger/__tests__/MetricsContext.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ test('setDimensions allows 30 dimensions', () => {
});

test('putDimension adds key to dimension and sets the dimension as a property', () => {

// arrange
const context = MetricsContext.empty();
const dimension = faker.random.word();
Expand All @@ -43,51 +44,80 @@ test('putDimension adds key to dimension and sets the dimension as a property',
expect(context.getDimensions()[0]).toStrictEqual(expectedDimension);
});

test('putDimension will not duplicate dimensions', () => {
test('putDimensions accepts multiple unique dimension sets', () => {
// arrange
const context = MetricsContext.empty();
const dimension = faker.random.word();
const expectedDimension = { dimension };
const expectedDimension1 = { d1: faker.random.word(), d2: faker.random.word() };
const expectedDimension2 = { d2: faker.random.word(), d3: faker.random.word() };

// act
context.putDimensions({ dimension });
context.putDimensions({ dimension });
context.putDimensions(expectedDimension1);
context.putDimensions(expectedDimension2);

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

test('putDimension will not duplicate dimensions, multiple in different order', () => {
test('putDimensions will not duplicate dimensions', () => {
// arrange
const context = MetricsContext.empty();
const dimension1 = faker.random.word();
const dimension2 = faker.random.word();
const expectedDimension = { dimension1, 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(1);
expect(context.getDimensions()[0]).toStrictEqual(expectedDimension);
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('putDimension accepts multiple unique dimension sets', () => {
test('putDimensions will sort dimensions correctly', () => {
// arrange
const context = MetricsContext.empty();
const expectedDimension1 = { d1: faker.random.word(), d2: faker.random.word() };
const expectedDimension2 = { d2: faker.random.word(), d3: faker.random.word() };
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(expectedDimension1);
context.putDimensions(expectedDimension2);
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(2);
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