Repository: awslabs/aws-embedded-metrics-node (ts)

Issue:
Disallow duplicate dimension sets.
There is no need for duplicate dimension sets like the following.

```json
{
  "Dimensions": [
    [ "A", "B"],
    [ "A", "B"],
    [ "B", "A"],
  ]
}
```
This would re-create the same metric 3 times and is equivalent to:
```json
{
  "Dimensions": [ [ "A", "B"] ]
}
```

This is needed for #14. It allows for things like the following while also allowing for re-use of the logger instance.


```js
const doWork = metricScope(metrics => () => {
  metrics.putDimensions(dimensions);
  // ...
});

// act
doWork();
doWork();
```

```js
logger.putMetric(metricKey, 0);
await logger.flush();

logger.putMetric(metricKey, 1);
await logger.flush();
```

Pull request description:
disallow duplicate dimensions sets
Closes #20

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


Patch:
diff --git a/src/logger/MetricsContext.ts b/src/logger/MetricsContext.ts
index d59f942..9fdb3a1 100644
--- a/src/logger/MetricsContext.ts
+++ b/src/logger/MetricsContext.ts
@@ -88,8 +88,34 @@ export class MetricsContext {
    *
    * @param dimensions
    */
-  public putDimensions(dimensions: Record<string, string>): void {
-    this.dimensions.push(dimensions);
+  public putDimensions(incomingDimensionSet: Record<string, string>): void {
+    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.
+      const existingDimensionSetKeys = Object.keys(existingDimensionSet);
+      const incomingDimensionSetKeys = Object.keys(incomingDimensionSet);
+      if (existingDimensionSetKeys.length !== incomingDimensionSetKeys.length) {
+        this.dimensions.push(incomingDimensionSet);
+        return;
+      }
+
+      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;
+        }
+      }
+    }
   }
 
   /**


Test patch:
diff --git a/src/logger/__tests__/MetricsContext.test.ts b/src/logger/__tests__/MetricsContext.test.ts
index 1bebd5e..3d7a8c8 100644
--- a/src/logger/__tests__/MetricsContext.test.ts
+++ b/src/logger/__tests__/MetricsContext.test.ts
@@ -29,6 +29,53 @@ 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', () => {
+  // arrange
+  const context = MetricsContext.empty();
+  const dimension = faker.random.word();
+  const expectedDimension = { dimension };
+
+  // act
+  context.putDimensions({ dimension });
+  context.putDimensions({ dimension });
+
+  // assert
+  expect(context.getDimensions().length).toBe(1);
+  expect(context.getDimensions()[0]).toStrictEqual(expectedDimension);
+});
+
+test('putDimension will not duplicate dimensions, multiple in different order', () => {
+  // arrange
+  const context = MetricsContext.empty();
+  const dimension1 = faker.random.word();
+  const dimension2 = faker.random.word();
+  const expectedDimension = { dimension1, dimension2 };
+
+  // act
+  context.putDimensions({ dimension1, dimension2 });
+  context.putDimensions({ dimension2, dimension1 });
+
+  // assert
+  expect(context.getDimensions().length).toBe(1);
+  expect(context.getDimensions()[0]).toStrictEqual(expectedDimension);
+});
+
+test('putDimension accepts multiple unique dimension sets', () => {
+  // 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() };
+
+  // act
+  context.putDimensions(expectedDimension1);
+  context.putDimensions(expectedDimension2);
+
+  // assert
+  expect(context.getDimensions().length).toBe(2);
+  expect(context.getDimensions()[0]).toStrictEqual(expectedDimension1);
+  expect(context.getDimensions()[1]).toStrictEqual(expectedDimension2);
+});
+
 test('getDimensions returns default dimensions if custom dimensions not set', () => {
   // arrange
   const context = MetricsContext.empty();
diff --git a/test/integ/agent/end-to-end.integ.ts b/test/integ/agent/end-to-end.integ.ts
index d7217c2..8621b89 100644
--- a/test/integ/agent/end-to-end.integ.ts
+++ b/test/integ/agent/end-to-end.integ.ts
@@ -23,41 +23,6 @@ const dimensionValue = 'Integ-Test-Agent';
 const dimensions: Record<string, string> = {};
 dimensions[dimensionKey] = dimensionValue;
 
-const metricExists = async (metricName: string, expectedSampleCount: number): Promise<boolean> => {
-  const request = {
-    Namespace: 'aws-embedded-metrics',
-    MetricName: metricName,
-    Dimensions: [
-      { Name: 'ServiceName', Value: serviceName },
-      { Name: 'ServiceType', Value: serviceType },
-      { Name: 'LogGroup', Value: logGroupName },
-      { Name: dimensionKey, Value: dimensionValue },
-    ],
-    Period: 60,
-    StartTime: new Date(startTime.getTime() - 5000),
-    EndTime: new Date(now()),
-    Statistics: ['SampleCount'],
-  };
-
-  const result = await cwmClient.getMetricStatistics(request).promise();
-
-  if (result && result.Datapoints && result.Datapoints.length > 0) {
-    const samples = result.Datapoints.map(dataPoint => dataPoint.SampleCount || 0).reduce((total, i) => total + i);
-    console.log(`Received ${samples} samples.`);
-    return samples === expectedSampleCount;
-  }
-
-  return false;
-};
-
-const waitForMetricExistence = async (metricName: string, expectedSampleCount: number): Promise<void> => {
-  let attempts = 0;
-  while (!(await metricExists(metricName, expectedSampleCount))) {
-    console.log('No metrics yet. Sleeping before trying again. Attempt #', attempts++);
-    await Sleep(2000);
-  }
-};
-
 test(
   'end to end integration test with agent over UDP',
   async () => {
@@ -86,23 +51,60 @@ test(
     // arrange
     const idleTimeout = 500;
     const metricName = 'TCP-MultipleFlushes';
-    const expectedSamples = 2;
+    const expectedSamples = 3;
 
     Configuration.agentEndpoint = 'tcp://0.0.0.0:25888';
 
-    const doWork = metricScope(metrics => () => {
-      metrics.putDimensions(dimensions);
-      metrics.putMetric(metricName, 100, 'Milliseconds');
-      metrics.setProperty('RequestId', '422b1569-16f6-4a03-b8f0-fe3fd9b100f8');
-    });
+const doWork = metricScope(metrics => () => {
+  metrics.putDimensions(dimensions);
+  metrics.putMetric(metricName, 100, 'Milliseconds');
+  metrics.setProperty('RequestId', '422b1569-16f6-4a03-b8f0-fe3fd9b100f8');
+});
 
-    // act
-    doWork();
-    await Sleep(idleTimeout);
-    doWork();
+// act
+doWork();
+doWork();
+await Sleep(idleTimeout);
+doWork();
 
     // assert
     await waitForMetricExistence(metricName, expectedSamples);
   },
   timeoutMillis,
 );
+
+
+const metricExists = async (metricName: string, expectedSampleCount: number): Promise<boolean> => {
+  const request = {
+    Namespace: 'aws-embedded-metrics',
+    MetricName: metricName,
+    Dimensions: [
+      { Name: 'ServiceName', Value: serviceName },
+      { Name: 'ServiceType', Value: serviceType },
+      { Name: 'LogGroup', Value: logGroupName },
+      { Name: dimensionKey, Value: dimensionValue },
+    ],
+    Period: 60,
+    StartTime: new Date(startTime.getTime() - 5000),
+    EndTime: new Date(now()),
+    Statistics: ['SampleCount'],
+  };
+
+  const result = await cwmClient.getMetricStatistics(request).promise();
+
+  if (result && result.Datapoints && result.Datapoints.length > 0) {
+    const samples = result.Datapoints.map(dataPoint => dataPoint.SampleCount || 0).reduce((total, i) => total + i);
+    console.log(`Received ${samples} samples.`);
+    return samples === expectedSampleCount;
+  }
+
+  return false;
+};
+
+const waitForMetricExistence = async (metricName: string, expectedSampleCount: number): Promise<void> => {
+  let attempts = 0;
+  while (!(await metricExists(metricName, expectedSampleCount))) {
+    console.log('No metrics yet. Sleeping before trying again. Attempt #', attempts++);
+    await Sleep(2000);
+  }
+};