Skip to content

MINOR: Update log statements in alterBrokerConfigs/alterTopicConfigs methods#9824

Closed
omkreddy wants to merge 3 commits into
apache:trunkfrom
omkreddy:admin-logs
Closed

MINOR: Update log statements in alterBrokerConfigs/alterTopicConfigs methods#9824
omkreddy wants to merge 3 commits into
apache:trunkfrom
omkreddy:admin-logs

Conversation

@omkreddy

@omkreddy omkreddy commented Jan 5, 2021

Copy link
Copy Markdown
Contributor

Current below log statements are not useful. This PR logs readable/masked configs during alterBrokerConfigs/alterTopicConfigs method call.

[Admin Manager on Broker 1]: Updating topic test with new configuration kafka.server.KafkaConfig@c9ba35e3

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@omkreddy omkreddy requested a review from rajinisivaram January 5, 2021 15:41

@rajinisivaram rajinisivaram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@omkreddy Thanks for the PR, LGTM

@chia7712 chia7712 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.

@omkreddy Nice patch. LGTM!

BTW, does alterLogLevelConfigs need such log as well?

@omkreddy

omkreddy commented Jan 6, 2021

Copy link
Copy Markdown
Contributor Author

@chia7712 Thanks for the review . addressed the review comment.

@chia7712 chia7712 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.

@omkreddy Thanks for your updating. one trivial comment is left. +1 to merge this PR

alterConfigOp.opType() match {
case OpType.SET => Log4jController.logLevel(loggerName, logLevel)
case OpType.DELETE => Log4jController.unsetLogLevel(loggerName)
case OpType.SET => {

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.

the braces is redundant.

@chia7712 chia7712 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.

@omkreddy Thanks for your patch. LGTM

@omkreddy omkreddy closed this in ea2423e Jan 6, 2021
YiDing-Duke pushed a commit to YiDing-Duke/kafka that referenced this pull request Jun 7, 2021
…methods

Current below log statements are not useful. This PR logs readable/masked configs during alterBrokerConfigs/alterTopicConfigs method call.

`[Admin Manager on Broker 1]: Updating topic test with new configuration kafka.server.KafkaConfigc9ba35e3`

Author: Manikumar Reddy <manikumar.reddy@gmail.com>

Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>, Chia-Ping Tsai <chia7712@gmail.com>

Closes apache#9824 from omkreddy/admin-logs
YiDing-Duke pushed a commit to confluentinc/kafka that referenced this pull request Jun 7, 2021
…methods

Current below log statements are not useful. This PR logs readable/masked configs during alterBrokerConfigs/alterTopicConfigs method call.

`[Admin Manager on Broker 1]: Updating topic test with new configuration kafka.server.KafkaConfigc9ba35e3`

Author: Manikumar Reddy <manikumar.reddy@gmail.com>

Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>, Chia-Ping Tsai <chia7712@gmail.com>

Closes apache#9824 from omkreddy/admin-logs
YiDing-Duke pushed a commit to YiDing-Duke/kafka that referenced this pull request Jun 7, 2021
…methods

Current below log statements are not useful. This PR logs readable/masked configs during alterBrokerConfigs/alterTopicConfigs method call.

`[Admin Manager on Broker 1]: Updating topic test with new configuration kafka.server.KafkaConfigc9ba35e3`

Author: Manikumar Reddy <manikumar.reddy@gmail.com>

Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>, Chia-Ping Tsai <chia7712@gmail.com>

Closes apache#9824 from omkreddy/admin-logs
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.

3 participants