Skip to content

fix(metrics-api-scaler): Fix division by 0 for expectedNbMetric#7743

Merged
wozniakjan merged 5 commits into
kedacore:mainfrom
dttung2905:fix-div-by-zero-metrics-api-scaler
May 25, 2026
Merged

fix(metrics-api-scaler): Fix division by 0 for expectedNbMetric#7743
wozniakjan merged 5 commits into
kedacore:mainfrom
dttung2905:fix-div-by-zero-metrics-api-scaler

Conversation

@dttung2905

@dttung2905 dttung2905 commented May 17, 2026 •
edited by rickbrouwer
Loading

Copy link
Copy Markdown
Contributor

When the metrics-api scaler aggregates metrics from multiple Kubernetes service endpoints with aggregationType: average (the default), and every endpoint request fails, aggregateMetricsFromMultipleEndpoints still divides the aggregated value by expectedNbMetrics after it has been decremented to 0.

keda/pkg/scalers/metrics_api_scaler.go

Lines 457 to 458 in ef9df93

if s.metadata.AggregationType == AverageAggregationType {
aggregation /= float64(expectedNbMetrics)

Checklist

  • When introducing a new scaler, I agree with the scaling governance policy
  • I have verified that my change is according to the deprecations & breaking changes policy
  • Tests have been added (if applicable)
  • Ensure make generate-scalers-schema has been run to update any outdated generated files
  • Changelog has been updated and is aligned with our changelog requirements, only when the change impacts end users
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #7742

@dttung2905 dttung2905 requested a review from a team as a code owner May 17, 2026 16:35
@github-actions

github-actions Bot commented May 17, 2026

Copy link
Copy Markdown

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

  • Add an entry in our changelog in alphabetical order and link related issue
  • Update the documentation, if needed
  • Add unit & e2e tests for your changes
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

@keda-automation keda-automation requested a review from a team May 17, 2026 16:35
@snyk-io

snyk-io Bot commented May 17, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@dttung2905
Add changelog
4a48174 
Signed-off-by: dttung2905 <[email protected]>
@dttung2905

dttung2905 commented May 18, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor Author

/run-e2e metrics_api
Update: You can check the progress here

rickbrouwer
rickbrouwer requested changes May 19, 2026
View reviewed changes
Comment thread CHANGELOG.md Outdated
@dttung2905 @rickbrouwer
Update CHANGELOG.md
1a6b1ef 
Co-authored-by: Rick Brouwer <[email protected]>
Signed-off-by: Dao Thanh Tung <[email protected]>
@keda-automation keda-automation requested a review from a team May 19, 2026 09:44
@rickbrouwer

rickbrouwer commented May 20, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

/run-e2e metrics_api
Update: You can check the progress here

@rickbrouwer

rickbrouwer commented May 20, 2026

Copy link
Copy Markdown
Member

@dttung2905 can you check the race error?

@dttung2905
Fix failed CI tests
fa18454 
Signed-off-by: dttung2905 <[email protected]>
@dttung2905

dttung2905 commented May 21, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor Author

/run-e2e metrics_api
Update: You can check the progress here

@dttung2905

dttung2905 commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

Hi @rickbrouwer , I have fixed the race condition in the CI test now. Can you help to check it again?

@rickbrouwer rickbrouwer changed the title fix(metrics-api-scaler) : Fix division by 0 for expectedNbMetric fix(metrics-api-scaler): Fix division by 0 for expectedNbMetric May 23, 2026
rickbrouwer
rickbrouwer approved these changes May 23, 2026
View reviewed changes

@rickbrouwer rickbrouwer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@rickbrouwer rickbrouwer added Awaiting/2nd-approval This PR needs one more approval review nice-to-have:keda-v2.20 Not strictly necessary, but nice if you can bring it along labels May 23, 2026
@keda-automation keda-automation requested a review from a team May 25, 2026 10:58
@wozniakjan

wozniakjan commented May 25, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

/run-e2e metrics_api
Update: You can check the progress here

passed tests: 1 
Execution of tests/scalers/metrics_api/metrics_api_test.go, has passed after "one" attempts
failed tests: 0

@wozniakjan wozniakjan enabled auto-merge (squash) May 25, 2026 10:58
@rickbrouwer rickbrouwer added ok-to-merge This PR can be merged waiting-for-e2e and removed Awaiting/2nd-approval This PR needs one more approval review labels May 25, 2026
JorTurFer
JorTurFer approved these changes May 25, 2026
View reviewed changes

@JorTurFer JorTurFer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I❤️🫖

@wozniakjan wozniakjan merged commit 8dae444 into kedacore:main May 25, 2026
25 checks passed
shcherbak pushed a commit to shcherbak/keda that referenced this pull request Jun 3, 2026
@dttung2905 @rickbrouwer
@wozniakjan @shcherbak
fix(metrics-api-scaler): Fix division by 0 for expectedNbMetric (keda…
818e08e 
…core#7743)

* Fix division by 0 for expectedNbMetric

Signed-off-by: dttung2905 <[email protected]>

* Add changelog

Signed-off-by: dttung2905 <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Rick Brouwer <[email protected]>
Signed-off-by: Dao Thanh Tung <[email protected]>

* Fix failed CI tests

Signed-off-by: dttung2905 <[email protected]>

---------

Signed-off-by: dttung2905 <[email protected]>
Signed-off-by: Dao Thanh Tung <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Co-authored-by: Rick Brouwer <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
Signed-off-by: Yurii Shcherbak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wozniakjan wozniakjan wozniakjan approved these changes
@JorTurFer JorTurFer JorTurFer approved these changes
@rickbrouwer rickbrouwer rickbrouwer approved these changes

Assignees

No one assigned

Labels

nice-to-have:keda-v2.20 Not strictly necessary, but nice if you can bring it along ok-to-merge This PR can be merged waiting-for-e2e

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Metrics API scaler: average aggregation returns +Inf when all kube service endpoints fail

4 participants

@dttung2905 @rickbrouwer @wozniakjan @JorTurFer