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.

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

fix: prevent putDimensions from storing duplicate dimensions #88

Merged
merged 5 commits into from
Aug 15, 2022

Conversation

aaronlna
Copy link
Contributor

@aaronlna aaronlna commented Nov 22, 2021

Issue

#35 (related)

Description of changes

This is the Java-equivalent bug-fix for prior issue in Node:

This change introduces the following behaviors:

  1. dimension sets added through #putDimensions do not duplicate
  2. duplicate dimension sets replace (and are sorted after) existing dimension sets such
    that newer/latest dimension values are used by the root EMF node

Example:

putDimensions({ "keyA": "valueA1" })
putDimensions({ "keyA": "valueA2" })
putDimensions({ "keyA": "valueA3", "keyB": "valueB1" })
putDimensions({ "keyB": "valueB2", "keyA": "valueA4" })

// expected MetricsDirective dimensions:
[
  { "keyA": "valueA2" },
  { "keyB": "valueB2", "keyA": "valueA4" },
]

// expected EMF target members:
{
  "keyA": "valueA4",
  "keyB": "valueB2",
}

Description of how you validated changes

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.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

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.
… 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.
@miguelcss
Copy link

Ping, anyone at aws-embedded-metrics-java project with admin permissions can have a look at this PR?
Some more coming up :)

@markkuhn markkuhn self-requested a review August 9, 2022 17:41
@markkuhn markkuhn merged commit 68ab8f5 into awslabs:master Aug 15, 2022
@markkuhn markkuhn self-assigned this Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants