Skip to content

Conversation

@jkyros
Copy link
Contributor

@jkyros jkyros commented Dec 12, 2025

If you'd rather not have the noise here, I can just carry this downstream but I thought I'd at least offer 😄

  • I'm trying to prevent any user surprise from the log migration

  • We migrated to zap logs in the metrics adapter over in Reintroduce PR #5974: Remove klogr dependency and move to zap #6578 , and as part of that we translated the old log args to zap args, which avoids hard breaking users, but which will potentially also result in users having their log output change.

  • This just emits explicit log messages when using the old log flags explaining that the old log flags were deprecated so folks who are concerned can see why at the top of their logs.

Over in the olm operator we have that kedacontroller object that governs KEDA deployment and allows overriding args, Kedacontrollers persist across upgrades, and rather than rewrite the user's override args to be proper on the kedacontroller (and upset gitops systems since that's a user-set field) we thought it more straightforward to just signal it here since this is where they would look -- in the log whose output was different.

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
  • 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
  • [ ] A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • [ ] A PR is opened to update the documentation on (repo) (if applicable)
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #

Relates to #

We migrated to zap logs, and as part of that we translated the old log
args to zap log args, which will potentially result in users having
their log output change.

This just emits explicit logs explaining that
the old args were deprecated so folks who are concerned can see why at
the top of their logs.

Over in the olm operator we have a kedacontroller object that allows
overriding args, and rather than rewrite the user's specified args to be
proper (and upset gitops systems) we thought it more straightforward to
signal here that they had changed since this is probably where they
would look (in the very log that had changed).

Signed-off-by: John Kyros <[email protected]>
@jkyros jkyros requested a review from a team as a code owner December 12, 2025 06:58
@github-actions
Copy link

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 December 12, 2025 06:58
@snyk-io
Copy link

snyk-io bot commented Dec 12, 2025

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

Status Scanner 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.

@jkyros
Copy link
Contributor Author

jkyros commented Dec 19, 2025

Hmm, nope, we can't do this yet. We'd need to fix the default args in the OLM operator first, and these messages would need to be at Error level otherwise anyone at Error log level wouldn't see the Info messages. 😞

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants