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
Copy link
Copy Markdown
Contributor

@dttung2905 dttung2905 commented May 17, 2026

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.

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

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905 dttung2905 requested a review from a team as a code owner May 17, 2026 16:35
@github-actions
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
Copy link
Copy Markdown

snyk-io Bot commented May 17, 2026

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.

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Copy link
Copy Markdown
Contributor Author

dttung2905 commented May 18, 2026

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

Comment thread CHANGELOG.md Outdated
Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Dao Thanh Tung <ttdao.2015@accountancy.smu.edu.sg>
@keda-automation keda-automation requested a review from a team May 19, 2026 09:44
@rickbrouwer
Copy link
Copy Markdown
Member

rickbrouwer commented May 20, 2026

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

@rickbrouwer
Copy link
Copy Markdown
Member

@dttung2905 can you check the race error?

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Copy link
Copy Markdown
Contributor Author

dttung2905 commented May 21, 2026

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

@dttung2905
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
Copy link
Copy Markdown
Member

@rickbrouwer rickbrouwer left a comment

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
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
@keda-automation keda-automation requested a review from a team May 25, 2026 10:58
@wozniakjan
Copy link
Copy Markdown
Member

wozniakjan commented May 25, 2026

/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
Copy link
Copy Markdown
Member

@JorTurFer JorTurFer left a comment

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

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