From 49bc5c5d82168a010c764f7b99022af6da1757c3 Mon Sep 17 00:00:00 2001 From: Aaron Michael Lamb Date: Mon, 22 Nov 2021 03:21:23 -0800 Subject: [PATCH 1/4] fix: prevent putDimensions from storing duplicate dimensions This is the Java-equivalent bug-fix for prior issue in Node: https://github.com/awslabs/aws-embedded-metrics-node/issues/20 New conditions check for any matching dimension set before skipping put dimensions. This prevents duplicates from being stored. [TESTING] Unit test updated to address edge case; multiple dimension sets of variable size are added and checked. Ran integration tests using Docker and compared results in CloudWatch. --- .../emf/model/MetricDirective.java | 5 ++++ .../emf/model/MetricDirectiveTest.java | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java b/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java index 8d52094a..3f0a6383 100644 --- a/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java +++ b/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java @@ -52,6 +52,11 @@ class MetricDirective { } void putDimensionSet(DimensionSet dimensionSet) { + if (dimensions.stream() + .anyMatch(dim -> dim.getDimensionKeys().equals(dimensionSet.getDimensionKeys()))) { + return; + } + dimensions.add(dimensionSet); } diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java index 50d1bf3b..eee5011f 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java @@ -114,6 +114,29 @@ public void testPutMultipleDimensionSets() throws JsonProcessingException { "{\"Dimensions\":[[\"Region\"],[\"Instance\"]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}"); } + @Test + public void testPutMultipleDuplicateDimensionSets() throws JsonProcessingException { + MetricDirective metricDirective = new MetricDirective(); + metricDirective.putDimensionSet(DimensionSet.of("Region", "us-east-1")); + metricDirective.putDimensionSet( + DimensionSet.of("Region", "us-east-1", "Instance", "inst-1")); + metricDirective.putDimensionSet( + DimensionSet.of("Instance", "inst-1", "Region", "us-east-1")); + metricDirective.putDimensionSet(DimensionSet.of("Instance", "inst-1")); + metricDirective.putDimensionSet(DimensionSet.of("Region", "us-east-1")); + metricDirective.putDimensionSet( + DimensionSet.of("Region", "us-east-1", "Instance", "inst-1")); + metricDirective.putDimensionSet( + DimensionSet.of("Instance", "inst-1", "Region", "us-east-1")); + metricDirective.putDimensionSet(DimensionSet.of("Instance", "inst-1")); + + String serializedMetricDirective = objectMapper.writeValueAsString(metricDirective); + + assertEquals( + serializedMetricDirective, + "{\"Dimensions\":[[\"Region\"],[\"Region\",\"Instance\"],[\"Instance\"]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}"); + } + @Test public void testGetDimensionAfterSetDimensions() { MetricDirective metricDirective = new MetricDirective(); From d8c2d09f94b7e98940d5255a1e113ddd29c921bf Mon Sep 17 00:00:00 2001 From: Aaron Michael Lamb Date: Mon, 22 Nov 2021 12:43:59 -0800 Subject: [PATCH 2/4] 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 target member: { "keyA": "value2" } ``` [TESTING] Updated unit tests to check for this case wherein putDimensions may be triggered using various dimension set lengths, values, and order. --- .../emf/model/MetricDirective.java | 14 +++++--- .../emf/model/MetricDirectiveTest.java | 34 +++++++++++++++++-- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java b/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java index 3f0a6383..e3e056e9 100644 --- a/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java +++ b/src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java @@ -51,12 +51,16 @@ class MetricDirective { shouldUseDefaultDimension = true; } + /** + * Adds a dimension set to the end of the collection. + * + * @param dimensionSet + */ void putDimensionSet(DimensionSet dimensionSet) { - if (dimensions.stream() - .anyMatch(dim -> dim.getDimensionKeys().equals(dimensionSet.getDimensionKeys()))) { - return; - } - + // Duplicate dimensions sets are removed before being added to the end of the collection. + // This ensures only latest dimension 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 + dimensions.removeIf(dim -> dim.getDimensionKeys().equals(dimensionSet.getDimensionKeys())); dimensions.add(dimensionSet); } diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java index eee5011f..c835c609 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java @@ -102,7 +102,7 @@ public void testPutDimensions() throws JsonProcessingException { } @Test - public void testPutMultipleDimensionSets() throws JsonProcessingException { + public void testPutDimensionSetWhenMultipleDimensionSets() throws JsonProcessingException { MetricDirective metricDirective = new MetricDirective(); metricDirective.putDimensionSet(DimensionSet.of("Region", "us-east-1")); metricDirective.putDimensionSet(DimensionSet.of("Instance", "inst-1")); @@ -115,14 +115,16 @@ public void testPutMultipleDimensionSets() throws JsonProcessingException { } @Test - public void testPutMultipleDuplicateDimensionSets() throws JsonProcessingException { + public void testPutDimensionSetWhenDuplicateDimensionSets() throws JsonProcessingException { MetricDirective metricDirective = new MetricDirective(); + metricDirective.putDimensionSet(new DimensionSet()); metricDirective.putDimensionSet(DimensionSet.of("Region", "us-east-1")); metricDirective.putDimensionSet( DimensionSet.of("Region", "us-east-1", "Instance", "inst-1")); metricDirective.putDimensionSet( DimensionSet.of("Instance", "inst-1", "Region", "us-east-1")); metricDirective.putDimensionSet(DimensionSet.of("Instance", "inst-1")); + metricDirective.putDimensionSet(new DimensionSet()); metricDirective.putDimensionSet(DimensionSet.of("Region", "us-east-1")); metricDirective.putDimensionSet( DimensionSet.of("Region", "us-east-1", "Instance", "inst-1")); @@ -134,7 +136,33 @@ public void testPutMultipleDuplicateDimensionSets() throws JsonProcessingExcepti assertEquals( serializedMetricDirective, - "{\"Dimensions\":[[\"Region\"],[\"Region\",\"Instance\"],[\"Instance\"]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}"); + "{\"Dimensions\":[[],[\"Region\"],[\"Instance\",\"Region\"],[\"Instance\"]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}"); + } + + @Test + public void testPutDimensionSetWhenDuplicateDimensionSetsWillSortCorrectly() + throws JsonProcessingException { + MetricDirective metricDirective = new MetricDirective(); + metricDirective.putDimensionSet(new DimensionSet()); + metricDirective.putDimensionSet(DimensionSet.of("Region", "us-east-1")); + metricDirective.putDimensionSet( + DimensionSet.of("Region", "us-east-1", "Instance", "inst-1")); + metricDirective.putDimensionSet( + DimensionSet.of("Instance", "inst-1", "Region", "us-east-1")); + metricDirective.putDimensionSet(DimensionSet.of("Instance", "inst-1")); + metricDirective.putDimensionSet( + DimensionSet.of("Region", "us-east-1", "Instance", "inst-1")); + metricDirective.putDimensionSet( + DimensionSet.of("Instance", "inst-1", "Region", "us-east-1")); + metricDirective.putDimensionSet(DimensionSet.of("Instance", "inst-1")); + metricDirective.putDimensionSet(DimensionSet.of("Region", "us-east-1")); + metricDirective.putDimensionSet(new DimensionSet()); + + String serializedMetricDirective = objectMapper.writeValueAsString(metricDirective); + + assertEquals( + serializedMetricDirective, + "{\"Dimensions\":[[\"Instance\",\"Region\"],[\"Instance\"],[\"Region\"],[]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}"); } @Test From 5fd52b1ba2e36519c9cb69bbee40772fcbecbbd6 Mon Sep 17 00:00:00 2001 From: Mark Kuhn Date: Mon, 15 Aug 2022 09:13:36 -0700 Subject: [PATCH 3/4] Update src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java --- .../amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java index c835c609..e2ca1824 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java @@ -135,8 +135,8 @@ public void testPutDimensionSetWhenDuplicateDimensionSets() throws JsonProcessin String serializedMetricDirective = objectMapper.writeValueAsString(metricDirective); assertEquals( - serializedMetricDirective, - "{\"Dimensions\":[[],[\"Region\"],[\"Instance\",\"Region\"],[\"Instance\"]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}"); + "{\"Dimensions\":[[],[\"Region\"],[\"Instance\",\"Region\"],[\"Instance\"]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}", + serializedMetricDirective); } @Test From a6ec45e387170926f95c7d99b97258331727fdd8 Mon Sep 17 00:00:00 2001 From: Mark Kuhn Date: Mon, 15 Aug 2022 09:13:48 -0700 Subject: [PATCH 4/4] Update src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java --- .../amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java index e2ca1824..31ab8d94 100644 --- a/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java +++ b/src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java @@ -161,8 +161,8 @@ public void testPutDimensionSetWhenDuplicateDimensionSetsWillSortCorrectly() String serializedMetricDirective = objectMapper.writeValueAsString(metricDirective); assertEquals( - serializedMetricDirective, - "{\"Dimensions\":[[\"Instance\",\"Region\"],[\"Instance\"],[\"Region\"],[]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}"); + "{\"Dimensions\":[[\"Instance\",\"Region\"],[\"Instance\"],[\"Region\"],[]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}", + serializedMetricDirective); } @Test