Skip to content

fix(data_classes): return None from CloudWatchAlarmMetric.metric_stat for expression entries - #8691

Open
ShussMiao wants to merge 1 commit into
aws-powertools:developfrom
ShussMiao:fix/cw-alarm-metric-stat-expression-entries
Open

ShussMiao wants to merge 1 commit into
aws-powertools:developfrom
ShussMiao:fix/cw-alarm-metric-stat-expression-entries

Conversation

@ShussMiao

Copy link
Copy Markdown

Issue number: closes #8673

Summary

Changes

CloudWatchAlarmMetric.metric_stat read self["metricStat"] directly, so it raised KeyError: 'metricStat' on the expression entries of a metric math alarm. expression and label in the same class already use self.get(...) for optional keys, so this makes metric_stat consistent with them and widens the return type to CloudWatchAlarmMetricStat | None.

The added test uses the repository's existing cloudWatchAlarmEventSingleMetric.json fixture — its first metric (e1, expression: "m1/m2") has no metricStat — so no synthetic payload is needed.

Note on triage: this issue is not labelled triage yet, so flagging it here. The failure is reproducible from the repo's own event fixture with no AWS access, and origin/develop still reads self["metricStat"] at line 94 of the file.

User experience

Looping over alarm_data.configuration.metrics and reading metric_stat now works for metric math alarms instead of failing on the first expression entry.


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.

… for expression entries

`metric_stat` read `self["metricStat"]` directly, so it raised `KeyError` on the
expression entries of a metric math alarm. `expression` and `label` in the same
class already use `self.get(...)` for optional keys; this makes `metric_stat`
consistent with them.

The repository's own `cloudWatchAlarmEventSingleMetric.json` fixture has this
shape — its first metric (`e1`, `expression: "m1/m2"`) has no `metricStat` — so
the new test reproduces it without any synthetic payload.

Signed-off-by: ShussMiao <168898188+ShussMiao@users.noreply.github.com>
@ShussMiao
ShussMiao requested a review from a team as a code owner October 28, 2026 10:48
@ShussMiao
ShussMiao requested a review from svozza October 28, 2026 10:48
@powertools-for-aws-oss-automation powertools-for-aws-oss-automation Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 28, 2026
@boring-cyborg boring-cyborg Bot added the tests label Oct 28, 2026
@sonarqubecloud

Copy link
Copy Markdown

@leandrodamascena leandrodamascena added this to the 3.39.0 milestone Oct 28, 2026
@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.95%. Comparing base (26deef8) to head (eeeac64).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #8691   +/-   ##
========================================
  Coverage    96.95%   96.95%           
========================================
  Files          321      321           
  Lines        16253    16254    +1     
  Branches      1453     1453           
========================================
+ Hits         15758    15759    +1     
  Misses         353      353           
  Partials       142      142           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ShussMiao, and welcome. I did not stop at the fixture: I created a real metric math alarm with a Lambda action, forced it into ALARM, and the event CloudWatch delivered has the expression entry without metricStat, exactly like the fixture. With your change the loop over metrics works and the two real metrics still resolve. Returning None keeps metric_stat consistent with expression and label. Approving, this goes out in 3.39.0, no rebase needed, and you'll be credited in the release notes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: CloudWatchAlarmMetric.metric_stat raises KeyError for metric math expression entries

2 participants