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

Support to modify the log level key name in the cadence #1293

Merged
merged 2 commits into from
Nov 10, 2021

Conversation

git-hulk
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
API breaks? no
Deprecations? no
Related tickets no
License Apache 2.0

What's in this PR?

Support to modify the log level key name in the cadence

Why?

The log level key was used to identify which field in the log JSON was represented the log level and regard the log as ERROR level when the log level key was not found, so we want to have a way to change log level key(default was "level").

Checklist

  • Code meets the Developer Guide
  • User guide and development docs updated (if needed)
  • Related Helm chart(s) updated (if needed)

Copy link
Member

@pregnor pregnor left a comment

Choose a reason for hiding this comment

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

LGTM

Do you require an immediate release of this, or is it enough if we add it to the next "scheduled" release? (expected in the coming weeks)

@git-hulk
Copy link
Contributor Author

Many thanks to @pregnor, it's enough in the next release since we can workaround with local charts and use the remote repo agnain after releasing.

Copy link
Member

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

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

LGTM

@pregnor
Copy link
Member

pregnor commented Nov 10, 2021

@git-hulk I hope it is okay if we are going to rebase the PR to the latest master state and also push a chart patch version bump commit before merging. It is easier and faster for us to release a patch version this way.

@akijakya is going to do these changes and the release in the coming hours.

@git-hulk
Copy link
Contributor Author

Many thanks to @pregnor, we have patched the change on our local version, and works fine now. Would switch to the community version again after release.

@akijakya akijakya merged commit bc6a0cb into banzaicloud:master Nov 10, 2021
@akijakya
Copy link
Contributor

Hi @git-hulk!

The new chart version 0.22.1 has been released containing this change. You can pull the new chart after updating your local Helm repository index.

Note that the server version has also been bumped in the meantime 0.21->0.22.

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.

4 participants