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.

Sign up for GitHub

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

[CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) in tests #2587

Closed
wants to merge 2 commits into from
Closed

[CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) in tests #2587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af85889
[CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) in tests
zabetak Oct 15, 2021
a82ed88
Add TODO reference to CALCITE-4862
zabetak Oct 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion babel/build.gradle.kts
View file
Open in desktop
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
View file
Open in desktop
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")
Copy link
Contributor

Choose a reason for hiding this comment

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

do you have follow up jira on this? maybe add a TODO to reference that ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just created https://issues.apache.org/jira/browse/CALCITE-4862

apiv("org.testcontainers:testcontainers")
apiv("redis.clients:jedis")
apiv("sqlline:sqlline")
Expand Down
12 changes: 11 additions & 1 deletion cassandra/build.gradle.kts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

41 changes: 41 additions & 0 deletions core/src/test/resources/log4j2-test.xml
View file
Open in desktop
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>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering log4j2 doesn't support properties file now? why change to xml though

Copy link
Member Author

Choose a reason for hiding this comment

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

As of version 2.4, Log4j supports configuration via properties files (we are on 2.13.3). The syntax is however different so in any case it was necessary to update the files. Moreover, I vaguely remember that I had some problems in the past with some Log4j2 features and property files, which I didn't have with XML. Although I couldn't find a proper reference I have the impression that XML syntax is better supported.

<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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

32 changes: 32 additions & 0 deletions druid/src/test/resources/log4j2-test.xml
View file
Open in desktop
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>
12 changes: 6 additions & 6 deletions elasticsearch/src/test/resources/log4j2.xml → ...search/src/test/resources/log4j2-test.xml
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

36 changes: 36 additions & 0 deletions geode/src/test/resources/log4j2-test.xml
View file
Open in desktop
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
View file
Open in desktop
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