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

MINOR: Note that slf4j-log4j in version 1.7.35+ should be used #12114

Merged
merged 2 commits into from
May 3, 2022

Conversation

cadonna
Copy link
Member

@cadonna cadonna commented May 2, 2022

Adds a note to the upgrade notes to use slf4j-log4j version
1.7.35+ [1] or slf4j-reload4j to avoid possible compatibility issues
originating from the logging framework [2].

[1] https://www.slf4j.org/manual.html#swapping
[2] https://www.slf4j.org/codes.html#no_tlm

Committer Checklist (excluded from commit message)

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

Adds a note to the upgrade notes to use slf4j-log4j version
1.7.35+ [1] or slf4j-reload4j to avoid possible compatibility issues
originating from the logging framework [2].

[1] https://www.slf4j.org/manual.html#swapping
[2] https://www.slf4j.org/codes.html#no_tlm
@cadonna
Copy link
Member Author

cadonna commented May 2, 2022

Call for review: @ijuma

@@ -73,7 +73,11 @@ <h5><a id="upgrade_320_notable" href="#upgrade_320_notable">Notable changes in 3
via Connect worker and/or connector configuration. Connect may enable idempotent producers
by default in a future major release.</li>
<li>Kafka has replaced log4j and slf4j-log4j12 with reload4j and slf4j-reload4j due to security concerns.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can say something like:

    <li>Kafka has replaced log4j and slf4j-log4j12 with reload4j and slf4j-reload4j due to security concerns.
         This only affects modules that specify a logging backend (`connect-runtime` and `kafka-tools` are two such
         examples). A number of modules, including `kafka-clients`, leave it to the application to specify the logging
         backend. More information can be found at <a href"https://reload4j.qos.ch">reload4j</a>.
        Projects that depend on the affected modules from the Kafka project should use
        <a href="https://www.slf4j.org/manual.html#swapping">slf4j-log4j12 version 1.7.35 or above</a> or
        slf4j-reload4j to avoid
        <a href="https://www.slf4j.org/codes.html#no_tlm">possible compatibility issues originating from the logging framework</a>.

Copy link
Member

Choose a reason for hiding this comment

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

We should also update the same text in the 3.1 branch once we merge this.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ijuma changed the text as you proposed.

I will cherry-pick the commit to 3.2 and 3.1.

@tombentley Since this is just a doc change, we do not need a new RC, right? We can port the change directly to the doc repo in case RC1 passes the votes.

Copy link
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@cadonna cadonna merged commit 3d08724 into apache:trunk May 3, 2022
cadonna added a commit that referenced this pull request May 3, 2022
Adds a note to the upgrade notes to use slf4j-log4j version
1.7.35+ [1] or slf4j-reload4j to avoid possible compatibility issues
originating from the logging framework [2].

[1] https://www.slf4j.org/manual.html#swapping
[2] https://www.slf4j.org/codes.html#no_tlm

Reviewer: Ismael Juma <ismael@juma.me.uk>
@cadonna
Copy link
Member Author

cadonna commented May 3, 2022

Cherry-picked to 3.2
Opened PR #12117 for 3.1 since I ran into merge conflicts.

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