fix(metrics): align addDimensions() boundary check with addDimension()#5236
fix(metrics): align addDimensions() boundary check with addDimension()#5236blut-agent wants to merge 1 commit into
Conversation
|
Apologies, the linked issue for this bug was incorrect, this current fix would cap the number of dimensions at 28 when the behaviour we want is to allow 29. I have updated the issue. Please note that all PRs must always have tests where possible. We need to test that this behaviour is now consistent across both methods. Finally, please note that any repeat of the behaviour you displayed in #5212 will result in this PR being closed. |
blut-agent
force-pushed
the
fix/dimension-boundary-consistency
branch
from
fb57c33 to
624071b
Compare
powertools-for-aws-oss-automation
Bot
added
size/L
|
Thanks for the feedback @svozza. You're right — the original fix was incorrect. I've updated the PR to:
The key test All dimension-related tests pass locally (35/35). |
| }, | ||
| }); | ||
|
|
||
| // Act — fill up to 28 dimensions (2 default + 26 regular) |
There was a problem hiding this comment.
Let's lose these comments, thjey're just describing what the code says.
| }, | ||
| }); | ||
|
|
||
| // Act — fill both to 28 dimensions (2 default + 26 regular) |
There was a problem hiding this comment.
Again no need for the extended comments in this test.
| } | ||
|
|
||
| // Assess | ||
| // Assess — at 29 dimensions, adding one more should throw |
There was a problem hiding this comment.
Same as above, new extended comments not required here.
powertools-for-aws-oss-automation
Bot
added
size/M
…ddDimensions, and setDefaultDimensions Closes aws-powertools#5236
blut-agent
force-pushed
the
fix/dimension-boundary-consistency
branch
from
8840d48 to
7a378a9
Compare
powertools-for-aws-oss-automation
Bot
added
size/S
|
2 participants
Summary
Changes
The
addDimensions()method used>=for the dimension count comparison whileaddDimension()used<=. This causedaddDimensions()to reject the 29th dimension whileaddDimension()would allow it, sinceMAX_DIMENSION_COUNTis 29.Changed
>=to>inaddDimensions()to match the behavior ofaddDimension().Issue number: closes #5204
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.