Skip to content

Commit 2280879

Browse files
committed
[CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) in tests
1. Replace slf4j-log4j12 dependency (using Log4j 1.x) with log4j-slf4j-impl (using Log4j 2.x) when possible. Cannot remove Log4j 1.x from Pig/Piglet modules due to compilation dependency of the latter directly to log4j. 2. Use XML syntax instead of property syntax for Log4j configuration. A Log4j configuration is hierarchical by nature so formats with natural support for nesting (such as XML) are easier to use. 3. Exclude slf4j-log4j12 when it comes transitively from other dependencies. It gets in conflict with log4j-slf4j-impl and it may also appear transitively in projects using Calcite. 4. Add log4j-slf4j-impl in modules (kafka, mongodb, plus, example) to silence error messages due to missing logger implementation. 5. Always use testRuntimeOnly annotation for dependencies on SLF4J logger implementation. Calcite shouldn't enforce a specific logger implementation so they should never appear as compile or runtime dependencies. A concrete implementation is only needed when we want to log messages for testing purposes. 6. Bump Cassandra version to 3.11.3 to use Log4j2 in tests. Previous versions did not allow the use of any custom logger implementation (only logback). This was resolved CASSANDRA-13396 so now we can use Log4j2 as the rest of the tests. 7. Unify display pattern in elasticsearch log4j2.xml with the rest of tests. 8. Add/Modify Log4j2 configuration (log4j2-test.xml) to silence errors and not display logging messages in tests. Instead of globally turning off logs, specific logger entries were set in the configuration files: (i) to avoid hiding by accident important problems; (ii) to faciliate switching log levels during debugging in the future. Close apache#2587
1 parent 87f60d2 commit 2280879

File tree

37 files changed

+448
-206
lines changed

37 files changed

+448
-206
lines changed

babel/build.gradle.kts

+2-1
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ dependencies {
3131
testImplementation("net.hydromatic:scott-data-hsqldb")
3232
testImplementation("org.hsqldb:hsqldb")
3333
testImplementation("org.incava:java-diff")
34-
testImplementation("org.slf4j:slf4j-log4j12")
3534
testImplementation(project(":testkit"))
35+
36+
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
3637
}
3738

3839
val fmppMain by tasks.registering(org.apache.calcite.buildtools.fmpp.FmppTask::class) {

bom/build.gradle.kts

+4-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,10 @@ dependencies {
147147
apiv("org.postgresql:postgresql")
148148
apiv("org.scala-lang:scala-library")
149149
apiv("org.slf4j:slf4j-api", "slf4j")
150-
apiv("org.slf4j:slf4j-log4j12", "slf4j")
150+
// TODO: https://issues.apache.org/jira/browse/CALCITE-4862
151+
// Eventually we should get rid of slf4j-log4j12 dependency but currently it is not possible
152+
// since certain modules (Pig, Piglet) have dependencies using directly Log4j 1.x APIs
153+
runtimev("org.slf4j:slf4j-log4j12", "slf4j")
151154
apiv("org.testcontainers:testcontainers")
152155
apiv("redis.clients:jedis")
153156
apiv("sqlline:sqlline")

cassandra/build.gradle.kts

+11-1
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,23 @@ dependencies {
3535
testImplementation("org.apache.cassandra:cassandra-all") {
3636
exclude("org.slf4j", "log4j-over-slf4j")
3737
.because("log4j is already present in the classpath")
38+
exclude("ch.qos.logback", "logback-core")
39+
.because("conflicts with log4j-slf4j-impl")
40+
exclude("ch.qos.logback", "logback-classic")
41+
.because("conflicts with log4j-slf4j-impl")
42+
}
43+
testImplementation("org.cassandraunit:cassandra-unit") {
44+
exclude("ch.qos.logback", "logback-core")
45+
.because("conflicts with log4j-slf4j-impl")
46+
exclude("ch.qos.logback", "logback-classic")
47+
.because("conflicts with log4j-slf4j-impl")
3848
}
39-
testImplementation("org.cassandraunit:cassandra-unit")
4049
testRuntimeOnly("net.java.dev.jna:jna")
4150

4251
annotationProcessor("org.immutables:value")
4352
compileOnly("org.immutables:value-annotations")
4453
compileOnly("com.google.code.findbugs:jsr305")
54+
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
4555
}
4656

4757
fun JavaCompile.configureAnnotationSet(sourceSet: SourceSet) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to you under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
-->
18+
<Configuration>
19+
<Appenders>
20+
<Console name="A1" target="SYSTEM_OUT">
21+
<PatternLayout
22+
pattern="%d [%t] %-5p - %m%n"/>
23+
</Console>
24+
</Appenders>
25+
26+
<Loggers>
27+
<Root level="INFO">
28+
<AppenderRef ref="A1"/>
29+
</Root>
30+
<logger name="org.apache.cassandra" level="off"/>
31+
<logger name="com.datastax" level="WARN"/>
32+
<!-- Avoid showing connection errors on cassandra shutdown -->
33+
<logger name="com.datastax.driver.core.ControlConnection" level="off"/>
34+
</Loggers>
35+
</Configuration>

core/build.gradle.kts

+5-2
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@ dependencies {
5353
implementation("com.fasterxml.jackson.core:jackson-core")
5454
implementation("com.fasterxml.jackson.core:jackson-databind")
5555
implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml")
56-
implementation("com.google.uzaygezen:uzaygezen-core")
56+
implementation("com.google.uzaygezen:uzaygezen-core") {
57+
exclude("log4j", "log4j").because("conflicts with log4j-slf4j-impl which uses log4j2 and" +
58+
" also leaks transitively to projects depending on calcite-core")
59+
}
5760
implementation("com.jayway.jsonpath:json-path")
5861
implementation("com.yahoo.datasketches:sketches-core")
5962
implementation("commons-codec:commons-codec")
@@ -86,7 +89,7 @@ dependencies {
8689
testImplementation(kotlin("stdlib-jdk8"))
8790
testImplementation(kotlin("test"))
8891
testImplementation(kotlin("test-junit5"))
89-
testRuntimeOnly("org.slf4j:slf4j-log4j12")
92+
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
9093
}
9194

9295
tasks.jar {

core/src/test/resources/log4j.properties

-31
This file was deleted.
+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to you under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
-->
18+
<Configuration>
19+
<Appenders>
20+
<Console name="A1" target="SYSTEM_OUT">
21+
<PatternLayout
22+
pattern="%d [%t] %-5p - %m%n"/>
23+
</Console>
24+
</Appenders>
25+
26+
<Loggers>
27+
<Root level="INFO">
28+
<AppenderRef ref="A1"/>
29+
</Root>
30+
<!-- [CALCITE-2519] Silence ERROR logs from CalciteException, SqlValidatorException during tests -->
31+
<logger name="org.apache.calcite.runtime.CalciteException" level="FATAL"/>
32+
<logger name="org.apache.calcite.sql.validate.SqlValidatorException" level="ERROR"/>
33+
<logger name="org.apache.calcite.plan.RexImplicationChecker" level="FATAL"/>
34+
<logger name="org.apache.calcite.sql.test.SqlOperatorBaseTest" level="FATAL"/>
35+
<!-- [CALCITE-4858] Silence anything but errors -->
36+
<logger name="org.apache.calcite.plan.VisitorDataContext" level="ERROR"/>
37+
<logger name="org.apache.calcite.avatica" level="ERROR"/>
38+
<logger name="org.eclipse.jetty" level="ERROR"/>
39+
<logger name="org.apache.calcite.sql.parser" level="ERROR"/>
40+
</Loggers>
41+
</Configuration>

druid/build.gradle.kts

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ dependencies {
3636

3737
testImplementation(project(":testkit"))
3838
testImplementation("org.mockito:mockito-core")
39-
testRuntimeOnly("org.slf4j:slf4j-log4j12")
39+
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
4040
annotationProcessor("org.immutables:value")
4141
compileOnly("org.immutables:value-annotations")
4242
compileOnly("com.google.code.findbugs:jsr305")

druid/src/test/resources/log4j.properties

-30
This file was deleted.
+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to you under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
-->
18+
<Configuration>
19+
<Appenders>
20+
<Console name="A1" target="SYSTEM_OUT">
21+
<PatternLayout
22+
pattern="%d [%t] %-5p - %m%n"/>
23+
</Console>
24+
</Appenders>
25+
26+
<Loggers>
27+
<Root level="INFO">
28+
<AppenderRef ref="A1"/>
29+
</Root>
30+
<logger name="org.apache.calcite.adapter.druid" level="ERROR"/>
31+
</Loggers>
32+
</Configuration>

elasticsearch/src/test/resources/log4j2.xml elasticsearch/src/test/resources/log4j2-test.xml

+6-6
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,18 @@
1515
~ See the License for the specific language governing permissions and
1616
~ limitations under the License.
1717
-->
18-
<Configuration status="WARN">
18+
<Configuration>
1919
<Appenders>
20-
<Console name="console" target="SYSTEM_OUT">
20+
<Console name="A1" target="SYSTEM_OUT">
2121
<PatternLayout
22-
pattern="[%-5level] %d{HH:mm:ss.SSS} %logger{36}.%M() - %msg%n" />
22+
pattern="%d [%t] %-5p - %m%n"/>
2323
</Console>
2424
</Appenders>
2525

2626
<Loggers>
27-
<Logger name="org.elasticsearch" level="WARN"/>
28-
<Root level="INFO" additivity="false">
29-
<AppenderRef ref="console" />
27+
<Root level="INFO">
28+
<AppenderRef ref="A1"/>
3029
</Root>
30+
<Logger name="org.elasticsearch" level="WARN"/>
3131
</Loggers>
3232
</Configuration>

example/csv/build.gradle.kts

+2
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ dependencies {
4646
annotationProcessor("org.immutables:value")
4747
compileOnly("org.immutables:value-annotations")
4848
compileOnly("com.google.code.findbugs:jsr305")
49+
50+
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
4951
}
5052

5153
fun JavaCompile.configureAnnotationSet(sourceSet: SourceSet) {

example/function/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ dependencies {
2020
api("org.checkerframework:checker-qual")
2121

2222
testImplementation("sqlline:sqlline")
23+
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
2324
}

file/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ dependencies {
4040
annotationProcessor("org.immutables:value")
4141
compileOnly("org.immutables:value-annotations")
4242
compileOnly("com.google.code.findbugs:jsr305")
43+
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
4344
}
4445

4546
fun JavaCompile.configureAnnotationSet(sourceSet: SourceSet) {

geode/build.gradle.kts

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ dependencies {
3434

3535
testImplementation(project(":testkit"))
3636
testImplementation("com.fasterxml.jackson.core:jackson-databind")
37-
testRuntimeOnly("org.slf4j:slf4j-log4j12")
37+
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
3838
annotationProcessor("org.immutables:value")
3939
compileOnly("org.immutables:value-annotations")
4040
compileOnly("com.google.code.findbugs:jsr305")

geode/src/test/resources/log4j.properties

-28
This file was deleted.
+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to you under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
-->
18+
<Configuration>
19+
<Appenders>
20+
<Console name="A1" target="SYSTEM_OUT">
21+
<PatternLayout
22+
pattern="%d [%t] %-5p - %m%n"/>
23+
</Console>
24+
</Appenders>
25+
26+
<Loggers>
27+
<Root level="INFO">
28+
<AppenderRef ref="A1"/>
29+
</Root>
30+
<!-- Only report errors coming from Geode or its dependencies-->
31+
<logger name="org.apache.geode" level="ERROR"/>
32+
<logger name="org.eclipse.jetty" level="ERROR"/>
33+
<!-- Change level to INFO to see OQL queries -->
34+
<logger name="org.apache.calcite.adapter.geode" level="WARN"/>
35+
</Loggers>
36+
</Configuration>

gradle.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ aggdesigner-algorithm.version=6.0
7676
apiguardian-api.version=1.1.0
7777
asm.version=7.2
7878
bouncycastle.version=1.60
79-
cassandra-all.version=3.11.2
79+
cassandra-all.version=3.11.3
8080
cassandra-driver-core.version=3.6.0
8181
cassandra-unit.version=3.5.0.1
8282
chinook-data-hsqldb.version=0.1

0 commit comments

Comments
 (0)