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

remove use log of log4j v1 #15984

Merged
merged 1 commit into from
Mar 15, 2024
Merged

Conversation

AlbericByte
Copy link
Contributor

Fixes #12425.

Description

log4j v1 is end of life and full of security issues
indexing-hadoop/pom.xml
extensions-core/hdfs-storage/pom.xml

remove log4j v1 dependences

Fixed the bug to remove log4j v1

Release note

remove log4j v1 dependences


Key changed/added classes in this PR
  • pom.xml

This PR has:

  • been self-reviewed.
  • using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@AlbericByte
Copy link
Contributor Author

AlbericByte commented Mar 1, 2024

@FrankChen021 when you have time, could you help to review the code

There is one integration test failed(integration-index-tests-indexer (kafka-index)), which is not related to the modification from the log

ERROR: listing workers for Build: failed to list workers: Unavailable: connection error: desc = "error reading server preface: read unix @->/run/docker.sock: use of closed network connection"
Error:  Command execution failed.
org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
    at org.apache.commons.exec.DefaultExecutor.executeInternal (DefaultExecutor.java:404)
    at org.apache.commons.exec.DefaultExecutor.execute (DefaultExecutor.java:166)
    at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:1000)
    at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:947)
    at org.codehaus.mojo.exec.ExecMojo.execute (ExecMojo.java:471)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:370)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:351)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:215)

@cryptoe
Copy link
Contributor

cryptoe commented Mar 1, 2024

@AlbericByte Can you please test a druid build with HDFS storage and see if this patch does not break logging.

@AlbericByte
Copy link
Contributor Author

@cryptoe Thanks for your time and suggestion.
I have tested using following command, and it can build successfully
Does it meet your suggestion. if not, could you help provide more detail.

mvn clean install \
     -Pintegration-tests,skip-static-checks,skip-tests \
     -Ddocker.run.skip=true \
     -Ddocker.build.hadoop=true

@abhishekagarwal87
Copy link
Contributor

I think @cryptoe meant to ask if the change is manually verified with HDFS as deep storage.

@AlbericByte
Copy link
Contributor Author

AlbericByte commented Mar 14, 2024

@abhishekagarwal87 and @cryptoe
Sorry for late, this is my first time to setup druid-hdfs-storage, i have launched a HADOOP docker environment, and test it work as expected, for example: the index log can be write to the HDFS as following:

druid_indexer_logs_type=hdfs
druid_indexer_logs_directory=hdfs://namenode:8020/drui/indexing-logs
Screenshot 2024-03-14 at 1 55 49 PM

@abhishekagarwal87 abhishekagarwal87 merged commit 33bb99c into apache:master Mar 15, 2024
84 checks passed
@adarshsanjeev adarshsanjeev added this to the 30.0.0 milestone May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security: remove use log of log4j v1
4 participants