Add Log4j 1 to Log4j 2 configuration file conversion #202

ppkarwasz · 2024-12-18T14:41:38Z

What's changed?

This PR uses the Log4j Configuration Converter API to add a Log4j1ConfigurationToLog4jCore2 rule, which converts log4j.properties and log4j.xml configuration files into the equivalent log4j2.xml configuration files.

It also refactors the Log4j1toLog4j2 recipe into three parts:

Log4j1toLog4jAPI performs the migration from the "Log4j 1 API" to Log4j API 2, as described in Log4j 1 API migration. This recipe performs almost exclusively Java code changes and is usually not required in applications that use JCL or SLF4J as logging interface.
Log4j1ConfigurationToLog4jCore2 migrates configuration files, as described in the Log4j 1 Configuration file migration section.
Log4j1ToLog4jCore2 migrates the runtime dependencies to use Log4j Core 2 instead of Log4j 1, as described in Log4j 1 Backend migration and also calls the previous recipe. This is probably the most useful recipe for users that no longer user Log4j 1 directly, but use it as logging implementation.

What's your motivation?

This PR includes the last missing ingredient for a complete Log4j 1 to Log4j API/Core 2 migration: the conversion of configuration files.

Anything in particular you'd like reviewers to focus on?

This PR is a draft, since the "Log4j Configuration Converter API" it uses hasn't been published yet. We will publish log4j-converter-config as soon as we are reasonably sure that it can be used in OpenRewrite without modifications.

The configuration converter recipe is special, since not only it rewrites the content of a file, but it also can change its type (e.g. from Properties.File to XML.Document). Currently the recipe:

parses the bytes returned by the "Log4j Configuration Converter API" as UTF-8,
it writes them as SourceFile of type PlainText regardless of the actual type of the converted file and encodes the file using UTF-8.

Is there a way to skip the decoding/encoding of the converted configuration file? I tried Binary, but in that case printAllAsBytes() fails.

Anyone you would like to review specifically?

@timtebeek

Checklist

I've added unit tests to cover both positive and negative cases
I've read and applied the recipe conventions and best practices
I've used the IntelliJ IDEA auto-formatter on affected files

This PR uses the [Log4j Configuration Converter API](https://logging.staged.apache.org/log4j/transform/log4j-converter-config.html) to add a `Log4j1ConfigurationToLog4jCore2` rule, which converts `log4j.properties` and `log4j.xml` configuration files into the equivalent `log4j2.xml` configuration files. It also refactors the `Log4j1toLog4j2` recipe into three parts: - `Log4j1toLog4jAPI` performs the migration from the "Log4j 1 API" to Log4j API 2, as described in [Log4j 1 API migration](https://logging.apache.org/log4j/2.x/migrate-from-log4j1.html#api-migration). This recipe performs almost exclusively Java code changes and is usually not required in applications that use JCL or SLF4J as logging interface. - `Log4j1ConfigurationToLog4jCore2` migrates configuration files, as described in the [Log4j 1 Configuration file migration](https://logging.apache.org/log4j/2.x/migrate-from-log4j1.html#configuration-file-migration) section. - `Log4j1ToLog4jCore2` migrates the runtime dependencies to use Log4j Core 2 instead of Log4j 1, as described in [Log4j 1 Backend migration](https://logging.apache.org/log4j/2.x/migrate-from-log4j1.html#backend-migration) and also calls the previous recipe. This is probably the most useful recipe for users that no longer user Log4j 1 directly, but use it as logging implementation. Closes openrewrite#154. Related to apache/logging-log4j2#3220

src/main/java/org/openrewrite/java/logging/ConvertConfiguration.java

src/test/java/org/openrewrite/java/logging/ConvertConfigurationTest.java

src/main/java/org/openrewrite/java/logging/ConvertConfiguration.java

src/test/java/org/openrewrite/java/logging/ConvertConfigurationTest.java

timtebeek · 2024-12-18T16:09:58Z

Thanks @ppkarwasz ! The bot suggestions can be a little confusing at times; it wants you to place the static imports last if that was unclear before. I'll try to review in the coming days! Right now caught up midway through a release and some meetings.

src/main/java/org/openrewrite/java/logging/ConvertConfiguration.java

src/test/java/org/openrewrite/java/logging/ConvertConfigurationTest.java

src/main/java/org/openrewrite/java/logging/ConvertConfiguration.java

src/test/java/org/openrewrite/java/logging/ConvertConfigurationTest.java

ppkarwasz · 2024-12-19T14:10:09Z

@timtebeek,

The bot suggestions can be a little confusing at times; it wants you to place the static imports last if that was unclear before.

They are indeed confusing, thanks for the explanation.

I'll try to review in the coming days! Right now caught up midway through a release and some meetings.

I am also working on the 0.3.0 release of Log4j Transform. Unless there are radical changes we need to do in Log4j Transform, this PR can wait until 0.3.0 is released.

timtebeek

Had a closer look already and applied some polish & suggestions. Thanks for working on these recipes ahead of the release! Should help adoption I imagine. Do let us know if you'd like to coordinate a merge and patch release of rewrite-logging-frameworks at some point.

timtebeek · 2025-01-01T16:19:48Z

build.gradle.kts

    implementation("org.openrewrite.recipe:rewrite-java-dependencies:${rewriteVersion}")
    implementation("org.openrewrite.recipe:rewrite-static-analysis:${rewriteVersion}")
    runtimeOnly("org.openrewrite:rewrite-java-17")

    implementation("org.apache.logging.log4j:log4j-core:2.+")
    implementation("org.slf4j:slf4j-api:2.+")
+    implementation("org.apache.logging.log4j:log4j-converter-config:0.3.0-SNAPSHOT")


Indeed likely best to merge this once released; thanks for creating the recipe before then!

Suggested change

implementation("org.apache.logging.log4j:log4j-converter-config:0.3.0-SNAPSHOT")

implementation("org.apache.logging.log4j:log4j-converter-config:latest.release")

timtebeek · 2025-01-01T16:20:05Z

build.gradle.kts

+repositories {
+    mavenCentral()
+    mavenLocal()
+    // Temporarily add Apache Snapshot repository for Log4j artifacts
+    maven {
+        setUrl("https://repository.apache.org/snapshots")
+        mavenContent {
+            // Excessive 404 result codes in `repository.apache.org` will ban the worker IP
+            // https://infra.apache.org/infra-ban.html
+            snapshotsOnly()
+            excludeGroupAndSubgroups("org.openrewrite")
+        }
+    }
+}
+


Similarly best removed before a merge.

Suggested change

repositories {

mavenCentral()

mavenLocal()

// Temporarily add Apache Snapshot repository for Log4j artifacts

maven {

setUrl("https://repository.apache.org/snapshots")

mavenContent {

// Excessive 404 result codes in `repository.apache.org` will ban the worker IP

// https://infra.apache.org/infra-ban.html

snapshotsOnly()

excludeGroupAndSubgroups("org.openrewrite")

}

}

}

timtebeek · 2025-01-01T16:20:51Z

build.gradle.kts

@@ -50,7 +67,7 @@ dependencies {
    testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:latest.release")

    testImplementation("org.openrewrite:rewrite-kotlin:${rewriteVersion}")
-    testImplementation("org.openrewrite:rewrite-maven")
+    testImplementation("org.openrewrite:rewrite-properties:${rewriteVersion}")


This rewrite-bom manages the version here, so we can leave that out already.

Suggested change

testImplementation("org.openrewrite:rewrite-properties:${rewriteVersion}")

testImplementation("org.openrewrite:rewrite-properties")

timtebeek · 2025-01-01T16:21:53Z

src/main/java/org/openrewrite/java/logging/ConvertConfiguration.java

+                    converter.convert(inputStream, inputFormat, outputStream, outputFormat);
+                    String utf8 = new String(outputStream.toByteArray(), StandardCharsets.UTF_8);
+
+                    return PlainTextParser.convert(sourceFile)


It looks like the output files are actually XML; should we use the XML parser here to read the converted contents such that subsequent recipes can optionally target their contents as part of the same recipe run?

This is exactly the main question I had for you. The log4j-converter-config artifact can convert to multiple formats (right now log4j2.xml, log4j2.json, log4j2.yaml and log4j2.properties are supported).

In the "Log4j 1 to Log4j Core 2" recipe I only use the XML output, but the ConvertConfiguration recipe could be used by users for other conversions. Should I make a switch here on outputFormat and use PlainText only if the format is not one of the recognized ones?

github-actions bot reviewed Dec 18, 2024

View reviewed changes

Fix licenses

a33dea0

github-actions bot reviewed Dec 18, 2024

View reviewed changes

timtebeek assigned ppkarwasz Dec 18, 2024

timtebeek self-requested a review December 18, 2024 16:10

ppkarwasz added 2 commits December 19, 2024 14:15

Fix imports

0bd6cf9

Use repository.apache.org only for snapshots

81ce466

github-actions bot reviewed Dec 19, 2024

View reviewed changes

Limit requests to repository.apache.org

9cc4a8b

github-actions bot reviewed Dec 19, 2024

View reviewed changes

Try guessing bot suggestions

e1a4b99

ppkarwasz and others added 4 commits December 19, 2024 15:12

Reverts changes to Slf4jToLog4jTest

599234e

Merge branch 'main' into feature/log4j1-to-log4j-core

608f1c9

Run ./gradlew downloadRecipeDependencies to update jars

4d9a3bc

Light polish

5339812

timtebeek reviewed Jan 1, 2025

View reviewed changes

Update classes to expect a newer version of the log4j jars

6cb469a

timtebeek added the recipe Recipe Requested label Jan 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Log4j 1 to Log4j 2 configuration file conversion #202

Add Log4j 1 to Log4j 2 configuration file conversion #202

ppkarwasz commented Dec 18, 2024 •

edited by timtebeek

Loading

timtebeek commented Dec 18, 2024

ppkarwasz commented Dec 19, 2024

timtebeek left a comment

timtebeek Jan 1, 2025

timtebeek Jan 1, 2025

timtebeek Jan 1, 2025

timtebeek Jan 1, 2025

ppkarwasz Jan 1, 2025

	implementation("org.apache.logging.log4j:log4j-converter-config:0.3.0-SNAPSHOT")
	implementation("org.apache.logging.log4j:log4j-converter-config:latest.release")

	testImplementation("org.openrewrite:rewrite-properties:${rewriteVersion}")
	testImplementation("org.openrewrite:rewrite-properties")

Add Log4j 1 to Log4j 2 configuration file conversion #202

Are you sure you want to change the base?

Add Log4j 1 to Log4j 2 configuration file conversion #202

Conversation

ppkarwasz commented Dec 18, 2024 • edited by timtebeek Loading

What's changed?

What's your motivation?

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Checklist

timtebeek commented Dec 18, 2024

ppkarwasz commented Dec 19, 2024

timtebeek left a comment

Choose a reason for hiding this comment

timtebeek Jan 1, 2025

Choose a reason for hiding this comment

timtebeek Jan 1, 2025

Choose a reason for hiding this comment

timtebeek Jan 1, 2025

Choose a reason for hiding this comment

timtebeek Jan 1, 2025

Choose a reason for hiding this comment

ppkarwasz Jan 1, 2025

Choose a reason for hiding this comment

ppkarwasz commented Dec 18, 2024 •

edited by timtebeek

Loading