From a9705f2cef5efcceec84f257bd38cb5d71485bbf Mon Sep 17 00:00:00 2001
From: Aaron Michael Lamb <aarolamb@amazon.com>
Date: Mon, 22 Nov 2021 12:49:39 -0800
Subject: [PATCH] fix: change putDimensions to update/sort existing dimension
 sets when duplicate

This change ensures new dimension key-values are used for the EMF root node
by removing duplicate dimension sets and adding input dimension set 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.
---
 src/logger/MetricsContext.ts                | 15 +++-----
 src/logger/__tests__/MetricsContext.test.ts | 42 +++++++++++++++++++--
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/src/logger/MetricsContext.ts b/src/logger/MetricsContext.ts
index dd9599f..79143ac 100644
--- a/src/logger/MetricsContext.ts
+++ b/src/logger/MetricsContext.ts
@@ -111,23 +111,20 @@ export class MetricsContext {
    * @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);
   }
 
diff --git a/src/logger/__tests__/MetricsContext.test.ts b/src/logger/__tests__/MetricsContext.test.ts
index 3484aea..ee70a02 100644
--- a/src/logger/__tests__/MetricsContext.test.ts
+++ b/src/logger/__tests__/MetricsContext.test.ts
@@ -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 sort dimensions 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', () => {