feat(metrics): increase maximum dimensions to 29 #1072
Conversation
| @@ -459,6 +459,7 @@ describe('Class: Metrics', () => { | |||
| }); | |||
|
|
|||
| test('Should throw an error if the same metric name is added again with a different unit', ()=> { | |||
| expect.assertions(1); | |||
There was a problem hiding this comment.
Could you please move this assertion at the end of the test? This file specifically is a bit of an outlier, but generally speaking we try to do 1/ Setup, 2/ Action, 3/ Assess
There was a problem hiding this comment.
I just saw that the assertion is used at the beginning in other test cases of this file, we will be changing this in the context of #163 to make them more uniform to the other two utils.
You can choose to ignore the comment or address it, it won't be a blocker for this PR.
There was a problem hiding this comment.
As you already said, I added it at the beginning just as it's the case in other tests. If I hadn't seen this, I would have used the fail('reason') before the catch block but that's something to be adressed in the issue that you've referenced
Description of your changes
Amazon added support to up 30 dimensions per metric. In this PR we increase this limit
How to verify this change
Review the code changes and the results of the automated builds.
Related issues, RFCs
Issue number: #1044
PR status
Is this ready for review?: YES
Is it a breaking change?: NO
Checklist
Breaking change checklist
N/A
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.