Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metrics Scaler: Improve Error Handling on not-ok response #2317

Closed
alexkunde opened this issue Nov 23, 2021 · 7 comments · Fixed by #2739
Closed

Metrics Scaler: Improve Error Handling on not-ok response #2317

alexkunde opened this issue Nov 23, 2021 · 7 comments · Fixed by #2739
Labels
feature-request All issues for new features that have not been committed to help wanted Looking for support from community

Comments

@alexkunde
Copy link

Proposal

What's even going on?

The Metrics API Scaler is pretty quit on non-200 API responses. (See this link for code)

At the moment we are using this for around 2000 scale targets and whenever something on API side goes wrong its a real hassle to even find out what the query was, when this is your response:

2021-11-23T11:59:33.926Z    ERROR    metrics_api_scaler    Error when checking metric value: api returned 403    {"error": "api returned 403"} 
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).isScaledObjectActive
    /workspace/pkg/scaling/scale_handler.go:228
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).checkScalers
    /workspace/pkg/scaling/scale_handler.go:211
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).startScaleLoop
    /workspace/pkg/scaling/scale_handler.go:145

Suggestion

Integrate the requested API endpoint and an optional response body into the scale error to get decent info on why the query didn't go through.

Possible new log

2021-11-23T11:59:33.926Z    ERROR    metrics_api_scaler    Error when checking metric value for /api/v1/files/on/server: api returned 403    {"error": "403 forbidden - cannot change into directory"} 
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).isScaledObjectActive
    /workspace/pkg/scaling/scale_handler.go:228
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).checkScalers
    /workspace/pkg/scaling/scale_handler.go:211
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).startScaleLoop
    /workspace/pkg/scaling/scale_handler.go:145

Use-Case

  • Improve debugging of big scaled systems
  • No need to have access to logs of metric endpoint to debug

Anything else?

No response

@alexkunde alexkunde added feature-request All issues for new features that have not been committed to needs-discussion labels Nov 23, 2021
@zroubalik
Copy link
Member

This seems like a fair request, are you willing to contribut this?

@zroubalik zroubalik added help wanted Looking for support from community and removed needs-discussion labels Dec 9, 2021
@alexkunde
Copy link
Author

I'd be happy to contribute, I'll look into the process and will link a PR

@ahmelsayed
Copy link
Contributor

Also while you're at it, this defer

defer r.Body.Close()
should move above that if r.StatusCode != http.StatusOK

@stale
Copy link

stale bot commented Feb 7, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Feb 7, 2022
@zroubalik
Copy link
Member

@schoki040 what is the status of this please?

@stale stale bot removed the stale All issues that are marked as stale due to inactivity label Feb 9, 2022
@alexkunde
Copy link
Author

I was not yet able to make any changes on this. This improvement is nonetheless still dear to my heart. Please dont close.

@fira42073
Copy link
Contributor

Hey. I created PR for this issue. Please review - #2739

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request All issues for new features that have not been committed to help wanted Looking for support from community
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants