[AWS] Change SQS metrics statistic method#8521
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
🌐 Coverage report
|
|
totally happy with these changes, but does it matter that the metrics change meaning (and value) all of a sudden for existing users? (i don't see a way round it tbh, but just wondering if we have a way to communicate that something is changing) |
packages/aws/changelog.yml
Outdated
There was a problem hiding this comment.
Came here from the related SDH.
It appears that changes made in the PR warrant either a minor or a major version upgrade, instead of a patch upgrade.
Changing statistic method for a few metrics could have implications for backward compatibility, making it more of a breaking change. Therefore, a major upgrade would be more appropriate. What do you think?
Ref: https://semver.org/#summary
Additionally, +1 to Tom's comment.
|
@tommyers-elastic @shmsr I totally understand the concern here. I'm treating it as a bug(with a patch upgrade) because I believe these metrics with the wrong statistic method are not meaningful for users to use. The way to communicate I think will be the changelog. But with more content than just one sentence like what I have right now 😂
Hmm since we have the renaming processor, this change actually will not break the customer on the surface. For sure it will change how the customer uses these metrics. |
|
i agree kaiyan - let's just make it clear in the changelog that the meaning of these metrics is changing. i don't mind how we update the version number. |
Got it. Your point is correct too; as it is a bug, a patch upgrade is also fine because we are fixing the bug. But yes, the changelog message should convey that properly. |
|
I just updated the changelog with more information and will keep the version to a patch release then. @tommyers-elastic @shmsr Thank you for the review! |
|
Package aws - 2.9.1 containing this change is available at https://epr.elastic.co/search?package=aws |
Proposed commit message
This PR changes the statistic method that we use to pull SQS cloudwatch metrics.
ApproximateAgeOfOldestMessage: average -> max
NumberOfMessagesDeleted: average -> sum
NumberOfEmptyReceives: average -> sum
NumberOfMessagesReceived: average -> sum
NumberOfMessagesSent: average -> sum
Checklist
changelog.ymlfile.