[AWS] Change SQS metrics statistic method

[kaiyan-sheng]

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

• I have reviewed tips for building integrations and this pull request is aligned with them.
• I have verified that all data streams collect metrics or logs.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2023-11-16T18:37:56.288+0000

• Duration: 85 min 28 sec

Test stats 🧪

Test	Results
Failed	0
Passed	223
Skipped	3
Total	226

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

[elasticmachine]

🌐 Coverage report

Name	Metrics % (covered/total)	Diff
Packages	100.0% (17/17)	💚
Files	94.444% (17/18)	❕
Classes	94.444% (17/18)	❕
Methods	89.701% (270/301)	❕
Lines	86.083% (7571/8795)	❕
Conditionals	100.0% (0/0)	💚

[tommyers-elastic]

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)

[shmsr]

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.

[kaiyan-sheng]

@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 😂

Changing statistic method for a few metrics could have implications for backward compatibility, making it more of a breaking change.

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.

[tommyers-elastic]

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.

[shmsr]

@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 😂

Changing statistic method for a few metrics could have implications for backward compatibility, making it more of a breaking change.

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.

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.

[kaiyan-sheng]

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!

[elasticmachine]

Package aws - 2.9.1 containing this change is available at https://epr.elastic.co/search?package=aws