Skip to content

Commit

Permalink
[CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) in tests
Browse files Browse the repository at this point in the history
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
  • Loading branch information
zabetak committed Oct 27, 2021
1 parent 87f60d2 commit 2280879
Show file tree
Hide file tree
Showing 37 changed files with 448 additions and 206 deletions.
3 changes: 2 additions & 1 deletion babel/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ dependencies {
testImplementation("net.hydromatic:scott-data-hsqldb")
testImplementation("org.hsqldb:hsqldb")
testImplementation("org.incava:java-diff")
testImplementation("org.slf4j:slf4j-log4j12")
testImplementation(project(":testkit"))

testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
}

val fmppMain by tasks.registering(org.apache.calcite.buildtools.fmpp.FmppTask::class) {
Expand Down
5 changes: 4 additions & 1 deletion bom/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,10 @@ dependencies {
apiv("org.postgresql:postgresql")
apiv("org.scala-lang:scala-library")
apiv("org.slf4j:slf4j-api", "slf4j")
apiv("org.slf4j:slf4j-log4j12", "slf4j")
// TODO: https://issues.apache.org/jira/browse/CALCITE-4862
// Eventually we should get rid of slf4j-log4j12 dependency but currently it is not possible
// since certain modules (Pig, Piglet) have dependencies using directly Log4j 1.x APIs
runtimev("org.slf4j:slf4j-log4j12", "slf4j")
apiv("org.testcontainers:testcontainers")
apiv("redis.clients:jedis")
apiv("sqlline:sqlline")
Expand Down
12 changes: 11 additions & 1 deletion cassandra/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,23 @@ dependencies {
testImplementation("org.apache.cassandra:cassandra-all") {
exclude("org.slf4j", "log4j-over-slf4j")
.because("log4j is already present in the classpath")
exclude("ch.qos.logback", "logback-core")
.because("conflicts with log4j-slf4j-impl")
exclude("ch.qos.logback", "logback-classic")
.because("conflicts with log4j-slf4j-impl")
}
testImplementation("org.cassandraunit:cassandra-unit") {
exclude("ch.qos.logback", "logback-core")
.because("conflicts with log4j-slf4j-impl")
exclude("ch.qos.logback", "logback-classic")
.because("conflicts with log4j-slf4j-impl")
}
testImplementation("org.cassandraunit:cassandra-unit")
testRuntimeOnly("net.java.dev.jna:jna")

annotationProcessor("org.immutables:value")
compileOnly("org.immutables:value-annotations")
compileOnly("com.google.code.findbugs:jsr305")
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
}

fun JavaCompile.configureAnnotationSet(sourceSet: SourceSet) {
Expand Down
35 changes: 35 additions & 0 deletions cassandra/src/test/resources/log4j2-test.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Licensed to the Apache Software Foundation (ASF) under one or more
~ contributor license agreements. See the NOTICE file distributed with
~ this work for additional information regarding copyright ownership.
~ The ASF licenses this file to you under the Apache License, Version 2.0
~ (the "License"); you may not use this file except in compliance with
~ the License. You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<Configuration>
<Appenders>
<Console name="A1" target="SYSTEM_OUT">
<PatternLayout
pattern="%d [%t] %-5p - %m%n"/>
</Console>
</Appenders>

<Loggers>
<Root level="INFO">
<AppenderRef ref="A1"/>
</Root>
<logger name="org.apache.cassandra" level="off"/>
<logger name="com.datastax" level="WARN"/>
<!-- Avoid showing connection errors on cassandra shutdown -->
<logger name="com.datastax.driver.core.ControlConnection" level="off"/>
</Loggers>
</Configuration>
7 changes: 5 additions & 2 deletions core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ dependencies {
implementation("com.fasterxml.jackson.core:jackson-core")
implementation("com.fasterxml.jackson.core:jackson-databind")
implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml")
implementation("com.google.uzaygezen:uzaygezen-core")
implementation("com.google.uzaygezen:uzaygezen-core") {
exclude("log4j", "log4j").because("conflicts with log4j-slf4j-impl which uses log4j2 and" +
" also leaks transitively to projects depending on calcite-core")
}
implementation("com.jayway.jsonpath:json-path")
implementation("com.yahoo.datasketches:sketches-core")
implementation("commons-codec:commons-codec")
Expand Down Expand Up @@ -86,7 +89,7 @@ dependencies {
testImplementation(kotlin("stdlib-jdk8"))
testImplementation(kotlin("test"))
testImplementation(kotlin("test-junit5"))
testRuntimeOnly("org.slf4j:slf4j-log4j12")
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
}

tasks.jar {
Expand Down
31 changes: 0 additions & 31 deletions core/src/test/resources/log4j.properties

This file was deleted.

41 changes: 41 additions & 0 deletions core/src/test/resources/log4j2-test.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Licensed to the Apache Software Foundation (ASF) under one or more
~ contributor license agreements. See the NOTICE file distributed with
~ this work for additional information regarding copyright ownership.
~ The ASF licenses this file to you under the Apache License, Version 2.0
~ (the "License"); you may not use this file except in compliance with
~ the License. You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<Configuration>
<Appenders>
<Console name="A1" target="SYSTEM_OUT">
<PatternLayout
pattern="%d [%t] %-5p - %m%n"/>
</Console>
</Appenders>

<Loggers>
<Root level="INFO">
<AppenderRef ref="A1"/>
</Root>
<!-- [CALCITE-2519] Silence ERROR logs from CalciteException, SqlValidatorException during tests -->
<logger name="org.apache.calcite.runtime.CalciteException" level="FATAL"/>
<logger name="org.apache.calcite.sql.validate.SqlValidatorException" level="ERROR"/>
<logger name="org.apache.calcite.plan.RexImplicationChecker" level="FATAL"/>
<logger name="org.apache.calcite.sql.test.SqlOperatorBaseTest" level="FATAL"/>
<!-- [CALCITE-4858] Silence anything but errors -->
<logger name="org.apache.calcite.plan.VisitorDataContext" level="ERROR"/>
<logger name="org.apache.calcite.avatica" level="ERROR"/>
<logger name="org.eclipse.jetty" level="ERROR"/>
<logger name="org.apache.calcite.sql.parser" level="ERROR"/>
</Loggers>
</Configuration>
2 changes: 1 addition & 1 deletion druid/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ dependencies {

testImplementation(project(":testkit"))
testImplementation("org.mockito:mockito-core")
testRuntimeOnly("org.slf4j:slf4j-log4j12")
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
annotationProcessor("org.immutables:value")
compileOnly("org.immutables:value-annotations")
compileOnly("com.google.code.findbugs:jsr305")
Expand Down
30 changes: 0 additions & 30 deletions druid/src/test/resources/log4j.properties

This file was deleted.

32 changes: 32 additions & 0 deletions druid/src/test/resources/log4j2-test.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Licensed to the Apache Software Foundation (ASF) under one or more
~ contributor license agreements. See the NOTICE file distributed with
~ this work for additional information regarding copyright ownership.
~ The ASF licenses this file to you under the Apache License, Version 2.0
~ (the "License"); you may not use this file except in compliance with
~ the License. You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<Configuration>
<Appenders>
<Console name="A1" target="SYSTEM_OUT">
<PatternLayout
pattern="%d [%t] %-5p - %m%n"/>
</Console>
</Appenders>

<Loggers>
<Root level="INFO">
<AppenderRef ref="A1"/>
</Root>
<logger name="org.apache.calcite.adapter.druid" level="ERROR"/>
</Loggers>
</Configuration>
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<Configuration status="WARN">
<Configuration>
<Appenders>
<Console name="console" target="SYSTEM_OUT">
<Console name="A1" target="SYSTEM_OUT">
<PatternLayout
pattern="[%-5level] %d{HH:mm:ss.SSS} %logger{36}.%M() - %msg%n" />
pattern="%d [%t] %-5p - %m%n"/>
</Console>
</Appenders>

<Loggers>
<Logger name="org.elasticsearch" level="WARN"/>
<Root level="INFO" additivity="false">
<AppenderRef ref="console" />
<Root level="INFO">
<AppenderRef ref="A1"/>
</Root>
<Logger name="org.elasticsearch" level="WARN"/>
</Loggers>
</Configuration>
2 changes: 2 additions & 0 deletions example/csv/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ dependencies {
annotationProcessor("org.immutables:value")
compileOnly("org.immutables:value-annotations")
compileOnly("com.google.code.findbugs:jsr305")

testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
}

fun JavaCompile.configureAnnotationSet(sourceSet: SourceSet) {
Expand Down
1 change: 1 addition & 0 deletions example/function/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ dependencies {
api("org.checkerframework:checker-qual")

testImplementation("sqlline:sqlline")
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
}
1 change: 1 addition & 0 deletions file/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ dependencies {
annotationProcessor("org.immutables:value")
compileOnly("org.immutables:value-annotations")
compileOnly("com.google.code.findbugs:jsr305")
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
}

fun JavaCompile.configureAnnotationSet(sourceSet: SourceSet) {
Expand Down
2 changes: 1 addition & 1 deletion geode/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ dependencies {

testImplementation(project(":testkit"))
testImplementation("com.fasterxml.jackson.core:jackson-databind")
testRuntimeOnly("org.slf4j:slf4j-log4j12")
testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl")
annotationProcessor("org.immutables:value")
compileOnly("org.immutables:value-annotations")
compileOnly("com.google.code.findbugs:jsr305")
Expand Down
28 changes: 0 additions & 28 deletions geode/src/test/resources/log4j.properties

This file was deleted.

36 changes: 36 additions & 0 deletions geode/src/test/resources/log4j2-test.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Licensed to the Apache Software Foundation (ASF) under one or more
~ contributor license agreements. See the NOTICE file distributed with
~ this work for additional information regarding copyright ownership.
~ The ASF licenses this file to you under the Apache License, Version 2.0
~ (the "License"); you may not use this file except in compliance with
~ the License. You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<Configuration>
<Appenders>
<Console name="A1" target="SYSTEM_OUT">
<PatternLayout
pattern="%d [%t] %-5p - %m%n"/>
</Console>
</Appenders>

<Loggers>
<Root level="INFO">
<AppenderRef ref="A1"/>
</Root>
<!-- Only report errors coming from Geode or its dependencies-->
<logger name="org.apache.geode" level="ERROR"/>
<logger name="org.eclipse.jetty" level="ERROR"/>
<!-- Change level to INFO to see OQL queries -->
<logger name="org.apache.calcite.adapter.geode" level="WARN"/>
</Loggers>
</Configuration>
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ aggdesigner-algorithm.version=6.0
apiguardian-api.version=1.1.0
asm.version=7.2
bouncycastle.version=1.60
cassandra-all.version=3.11.2
cassandra-all.version=3.11.3
cassandra-driver-core.version=3.6.0
cassandra-unit.version=3.5.0.1
chinook-data-hsqldb.version=0.1
Expand Down
Loading

0 comments on commit 2280879

Please sign in to comment.