Skip to content

Commit

Permalink
fix: prevent putDimensions from storing duplicate dimensions (#88)
Browse files Browse the repository at this point in the history
* fix: prevent putDimensions from storing duplicate dimensions

This is the Java-equivalent bug-fix for prior issue in Node:
awslabs/aws-embedded-metrics-node#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.

* 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.

* Update src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java

* Update src/test/java/software/amazon/cloudwatchlogs/emf/model/MetricDirectiveTest.java

Co-authored-by: Aaron Michael Lamb <[email protected]>
Co-authored-by: Mark Kuhn <[email protected]>
  • Loading branch information
3 people authored Aug 15, 2022
1 parent 4c242fc commit 68ab8f5
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,16 @@ class MetricDirective {
shouldUseDefaultDimension = true;
}

/**
* Adds a dimension set to the end of the collection.
*
* @param dimensionSet
*/
void putDimensionSet(DimensionSet dimensionSet) {
// 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -114,6 +114,57 @@ public void testPutMultipleDimensionSets() throws JsonProcessingException {
"{\"Dimensions\":[[\"Region\"],[\"Instance\"]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}");
}

@Test
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"));
metricDirective.putDimensionSet(
DimensionSet.of("Instance", "inst-1", "Region", "us-east-1"));
metricDirective.putDimensionSet(DimensionSet.of("Instance", "inst-1"));

String serializedMetricDirective = objectMapper.writeValueAsString(metricDirective);

assertEquals(
"{\"Dimensions\":[[],[\"Region\"],[\"Instance\",\"Region\"],[\"Instance\"]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}",
serializedMetricDirective);
}

@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(
"{\"Dimensions\":[[\"Instance\",\"Region\"],[\"Instance\"],[\"Region\"],[]],\"Metrics\":[],\"Namespace\":\"aws-embedded-metrics\"}",
serializedMetricDirective);
}

@Test
public void testGetDimensionAfterSetDimensions() {
MetricDirective metricDirective = new MetricDirective();
Expand Down

0 comments on commit 68ab8f5

Please sign in to comment.