This is an automated email from the ASF dual-hosted git repository.
adutra pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 0c790e0ad Standardize logging libraries in tests (#2268)
0c790e0ad is described below
commit 0c790e0ad48d69d97033bdde08d3bf4bb415210f
Author: Alexandre Dutra <[email protected]>
AuthorDate: Wed Aug 6 14:39:47 2025 +0200
Standardize logging libraries in tests (#2268)
This change enforces the following test logging patterns:
- Non-Quarkus modules use Logback Classic, configured via logback-test.xml
- Quarkus modules use JBoss Logging Manager, configured in Quarkus
configuration files.
This change also introduces a workaround for the "duplicate log messages"
issues with Gradle + JBoss Logging Manager. See this issue for context:
https://github.com/quarkusio/quarkus/issues/22844
The workaround implemented in this PR is very similar to the one proposed
in this comment:
https://github.com/quarkusio/quarkus/issues/22844#issuecomment-1824720923
Note: it's not entirely possible imho to suppress the following message on
the console:
```
The Agroal dependency is present but no JDBC datasources have been defined.
```
This is because:
1. The message happens during augmentation phase, not during tests
2. And it suffers from the "duplicate message" issue with (it's actually
Gradle that prints those messages).
---
.../src/main/kotlin/polaris-java.gradle.kts | 12 +--
.../src/main/kotlin/polaris-runtime.gradle.kts | 25 ++++++
.../src/test/resources/logback-test.xml | 11 +--
plugins/spark/v3.5/integration/build.gradle.kts | 5 +-
.../src/test/resources/logback-test.xml} | 22 ++---
runtime/admin/build.gradle.kts | 2 -
runtime/common/build.gradle.kts | 3 +-
runtime/defaults/build.gradle.kts | 3 +-
.../src/main/resources/application-it.properties | 13 ++-
.../src/main/resources/application-test.properties | 24 +++++-
.../src/main/resources/application.properties | 9 ++-
runtime/service/build.gradle.kts | 6 +-
.../service/exception/IcebergExceptionMapper.java | 15 ++--
.../IcebergJsonProcessingExceptionMapper.java | 15 +++-
.../service/exception/PolarisExceptionMapper.java | 8 +-
.../service/exception/ExceptionMapperTest.java | 94 ++++++++++++++--------
runtime/spark-tests/build.gradle.kts | 2 +-
.../junit/GradleDuplicateLoggingWorkaround.java | 84 +++++++++++++++++++
.../org.junit.jupiter.api.extension.Extension | 20 +++++
19 files changed, 277 insertions(+), 96 deletions(-)
diff --git a/build-logic/src/main/kotlin/polaris-java.gradle.kts
b/build-logic/src/main/kotlin/polaris-java.gradle.kts
index 980a144b7..799234506 100644
--- a/build-logic/src/main/kotlin/polaris-java.gradle.kts
+++ b/build-logic/src/main/kotlin/polaris-java.gradle.kts
@@ -107,11 +107,13 @@ testing {
dependencies {
implementation(project())
implementation(testFixtures(project()))
- runtimeOnly(
- libs.findLibrary("logback-classic").orElseThrow {
- GradleException("logback-classic not declared in
libs.versions.toml")
- }
- )
+ if (!plugins.hasPlugin("io.quarkus")) {
+ implementation(
+ libs.findLibrary("logback-classic").orElseThrow {
+ GradleException("logback-classic not declared in
libs.versions.toml")
+ }
+ )
+ }
implementation(
libs.findLibrary("assertj-core").orElseThrow {
GradleException("assertj-core not declared in libs.versions.toml")
diff --git a/build-logic/src/main/kotlin/polaris-runtime.gradle.kts
b/build-logic/src/main/kotlin/polaris-runtime.gradle.kts
index ee3e547a6..f52b2f16f 100644
--- a/build-logic/src/main/kotlin/polaris-runtime.gradle.kts
+++ b/build-logic/src/main/kotlin/polaris-runtime.gradle.kts
@@ -24,6 +24,14 @@ testing {
suites {
withType<JvmTestSuite> {
targets.all {
+ testTask.configure {
+ systemProperty("java.util.logging.manager",
"org.jboss.logmanager.LogManager")
+ // Enable automatic extension detection to execute
GradleDuplicateLoggingWorkaround
+ // automatically.
+ // See https://github.com/quarkusio/quarkus/issues/22844
+ systemProperty("junit.jupiter.extensions.autodetection.enabled",
"true")
+ }
+
if (testTask.name != "test") {
testTask.configure {
// For Quarkus...
@@ -47,6 +55,23 @@ testing {
}
}
+dependencies {
+ // All Quarkus projects should use JBoss LogManager with SLF4J, instead of
Logback
+ implementation("org.jboss.slf4j:slf4j-jboss-logmanager")
+}
+
+configurations.all {
+ // Validate that Logback dependencies are not used in Quarkus modules.
+ dependencies.configureEach {
+ if (group == "ch.qos.logback") {
+ throw GradleException(
+ "Logback dependencies are not allowed in Quarkus modules. " +
+ "Found $group:$name in ${project.name}."
+ )
+ }
+ }
+}
+
// Let the test's implementation config extend testImplementation, so it also
inherits the
// project's "main" implementation dependencies (not just the "api"
configuration)
configurations.named("intTestImplementation").configure {
diff --git a/runtime/service/src/test/resources/logback-test.xml
b/persistence/relational-jdbc/src/test/resources/logback-test.xml
similarity index 70%
rename from runtime/service/src/test/resources/logback-test.xml
rename to persistence/relational-jdbc/src/test/resources/logback-test.xml
index ddad1db40..b7e97bb82 100644
--- a/runtime/service/src/test/resources/logback-test.xml
+++ b/persistence/relational-jdbc/src/test/resources/logback-test.xml
@@ -19,21 +19,14 @@
under the License.
-->
-<configuration>
+<configuration debug="false">
<contextListener class="ch.qos.logback.classic.jul.LevelChangePropagator"/>
<appender name="console" class="ch.qos.logback.core.ConsoleAppender">
<encoder>
<pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
</encoder>
</appender>
- <root level="${log.level.console:-INFO}">
+ <root level="${test.log.level:-ERROR}">
<appender-ref ref="console"/>
</root>
-
- <!--
- Prevent the 'The Agroal dependency is present but no JDBC datasources have
been defined.' build-time warning.
- See
https://github.com/quarkusio/quarkus/blob/2fbc20f445fad43aaf4f3f984b9ac8319c7c7f0a/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java#L111
- THIS IS A TECHNICAL NECESSITY, because of the
- -->
- <logger name="io.quarkus.agroal.deployment" level="ERROR"/>
</configuration>
diff --git a/plugins/spark/v3.5/integration/build.gradle.kts
b/plugins/spark/v3.5/integration/build.gradle.kts
index 462523afe..bcfd7e15f 100644
--- a/plugins/spark/v3.5/integration/build.gradle.kts
+++ b/plugins/spark/v3.5/integration/build.gradle.kts
@@ -43,6 +43,7 @@ dependencies {
exclude(group = "org.scala-lang", module = "scala-reflect")
}
+ implementation(project(":polaris-runtime-defaults"))
implementation(project(":polaris-runtime-service"))
testImplementation(
@@ -52,6 +53,8 @@ dependencies {
testImplementation(project(":polaris-api-management-model"))
+ testImplementation(project(":polaris-runtime-test-common"))
+
testImplementation("org.apache.spark:spark-sql_${scalaVersion}:${spark35Version}")
{
// exclude log4j dependencies. Explicit dependencies for the log4j
libraries are
// enforced below to ensure the version compatibility
@@ -63,7 +66,6 @@ dependencies {
// enforce the usage of log4j 2.24.3. This is for the log4j-api compatibility
// of spark-sql dependency
testRuntimeOnly("org.apache.logging.log4j:log4j-core:2.25.1")
- testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j2-impl:2.25.1")
testImplementation("io.delta:delta-spark_${scalaVersion}:3.3.1")
@@ -95,7 +97,6 @@ dependencies {
tasks.named<Test>("intTest").configure {
maxParallelForks = 1
- systemProperty("java.util.logging.manager",
"org.jboss.logmanager.LogManager")
if (System.getenv("AWS_REGION") == null) {
environment("AWS_REGION", "us-west-2")
}
diff --git a/plugins/spark/v3.5/integration/src/intTest/resources/logback.xml
b/plugins/spark/v3.5/spark/src/test/resources/logback-test.xml
similarity index 55%
rename from plugins/spark/v3.5/integration/src/intTest/resources/logback.xml
rename to plugins/spark/v3.5/spark/src/test/resources/logback-test.xml
index 5ec1efd5f..b7e97bb82 100644
--- a/plugins/spark/v3.5/integration/src/intTest/resources/logback.xml
+++ b/plugins/spark/v3.5/spark/src/test/resources/logback-test.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="UTF-8"?>
+<?xml version="1.0" encoding="UTF-8" ?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
@@ -19,20 +19,14 @@
under the License.
-->
-<!--
-Spark by default set the logging at DEBUG level, which dumps a lot of code
details in
-the Intelij console, which impacts the IDE performance significantly when
running the
-tests. This configuration allows only log back the ERROR logs for the IDE, you
can comment
-out the configuration if you would like ot see all spark debug log during the
run.
--->
-<configuration>
- <appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
- <encoder class="ch.qos.logback.classic.encoder.PatternLayoutEncoder">
- <pattern>%d{yyyy-MM-dd HH:mm:ss} [%thread] %-5level - %msg%n</pattern>
+<configuration debug="false">
+ <contextListener class="ch.qos.logback.classic.jul.LevelChangePropagator"/>
+ <appender name="console" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
</encoder>
</appender>
-
- <root level="ERROR">
- <appender-ref ref="CONSOLE"/>
+ <root level="${test.log.level:-ERROR}">
+ <appender-ref ref="console"/>
</root>
</configuration>
diff --git a/runtime/admin/build.gradle.kts b/runtime/admin/build.gradle.kts
index f06895ba6..29ac2c856 100644
--- a/runtime/admin/build.gradle.kts
+++ b/runtime/admin/build.gradle.kts
@@ -43,7 +43,6 @@ dependencies {
implementation("io.quarkus:quarkus-container-image-docker")
implementation(project(":polaris-runtime-common"))
- implementation("org.jboss.slf4j:slf4j-jboss-logmanager")
testImplementation(project(":polaris-runtime-test-common"))
testFixturesApi(project(":polaris-core"))
@@ -92,7 +91,6 @@ artifacts {
tasks.withType(Test::class.java).configureEach {
maxParallelForks = 4
forkEvery = 1
- systemProperty("java.util.logging.manager",
"org.jboss.logmanager.LogManager")
}
tasks.named<Test>("test").configure {
diff --git a/runtime/common/build.gradle.kts b/runtime/common/build.gradle.kts
index 79452e80b..f1753882f 100644
--- a/runtime/common/build.gradle.kts
+++ b/runtime/common/build.gradle.kts
@@ -18,9 +18,8 @@
*/
plugins {
- alias(libs.plugins.quarkus)
alias(libs.plugins.jandex)
- id("polaris-runtime")
+ id("polaris-server")
}
dependencies {
diff --git a/runtime/defaults/build.gradle.kts
b/runtime/defaults/build.gradle.kts
index 88134be6e..b61c2458b 100644
--- a/runtime/defaults/build.gradle.kts
+++ b/runtime/defaults/build.gradle.kts
@@ -18,9 +18,8 @@
*/
plugins {
- alias(libs.plugins.quarkus)
alias(libs.plugins.jandex)
- id("polaris-runtime")
+ id("polaris-server")
}
dependencies {
diff --git a/runtime/defaults/src/main/resources/application-it.properties
b/runtime/defaults/src/main/resources/application-it.properties
index dc660f54a..7c3c70f3d 100644
--- a/runtime/defaults/src/main/resources/application-it.properties
+++ b/runtime/defaults/src/main/resources/application-it.properties
@@ -17,15 +17,22 @@
# under the License.
#
-# Configuration common to ALL integration tests (executed with "it" profile).
-# Note: Quarkus integration tests cannot use QuarkusTestProfile
+# Configuration common to ALL integration tests (executed with "it" profile –
Gradle "intTest" tasks).
+# Note: Quarkus integration tests cannot use QuarkusTestProfile.
+
+# Note about logging during integration tests:
+# - Logging for test code is configured in the `application-test.properties`
file (!!).
+# By default, it is set to `ERROR` level.
+# - Logging for application code is configured in the main
`application.properties` file.
+# By default, it is set to `INFO` level.
+# Configuring logging levels or appenders in this file IS INEFFECTIVE!
quarkus.http.limits.max-body-size=1000000
quarkus.http.port=0
quarkus.management.port=0
-# polaris.persistence.type=eclipse-link
+# polaris.persistence.type=jdbc
# polaris.persistence.type=in-memory-atomic
polaris.persistence.type=in-memory
diff --git a/runtime/defaults/src/main/resources/application-test.properties
b/runtime/defaults/src/main/resources/application-test.properties
index ff0f576ca..3ba01dfb0 100644
--- a/runtime/defaults/src/main/resources/application-test.properties
+++ b/runtime/defaults/src/main/resources/application-test.properties
@@ -17,21 +17,37 @@
# under the License.
#
-# Configuration common to ALL unit tests (executed with "test" profile).
+# Configuration common to ALL unit tests (executed with "test" profile –
Gradle "test" tasks).
# Per-test specific configuration should use QuarkusTestProfile
-quarkus.log.file.enable=false
quarkus.datasource.devservices.enabled=false
+quarkus.log.level=ERROR
+quarkus.log.file.enable=false
+quarkus.console.color=true
+
+# Useful loggers for debugging purposes.
+# quarkus.log.category."org.apache.polaris".level=INFO
+# quarkus.log.category."org.apache.iceberg".level=INFO
+# quarkus.log.category."io.quarkus.http.access-log".level=INFO
+# quarkus.log.category."org.apache.hc.client5.http".level=INFO
+
+# Silence a few verbose loggers that are not useful for unit tests.
+quarkus.log.category."org.apache.polaris.core.persistence.transactional.TransactionalMetaStoreManagerImpl".level=ERROR
+quarkus.log.category."org.apache.iceberg.rest.auth.AuthManagers".level=ERROR
+quarkus.log.category."org.apache.iceberg.rest.auth.OAuth2Manager".level=ERROR
+
# Prevent the 'Hibernate Validator does not support constraints on static
methods yet.' warning log messages on
# code generated by the OpenAPI generator. The warning can only be silenced
via a log level setting.
# See
https://github.com/quarkusio/quarkus/blob/15f6b4426512c04a54e7a0bd87f894fabe068c07/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java#L51-L53
-# and how the 'LOGGER' is setup.
+# and how the 'LOGGER' is set up.
quarkus.log.category."io.quarkus.hibernate.validator.deployment".level=ERROR
+
# Silence the FJP warning, nothing we can do about it.
# See
https://github.com/quarkusio/quarkus/blob/15f6b4426512c04a54e7a0bd87f894fabe068c07/core/deployment/src/main/java/io/quarkus/runner/bootstrap/ForkJoinClassLoading.java#L41-L42
quarkus.log.category."io.quarkus.runner.bootstrap.ForkJoinClassLoading".level=FATAL
+
# Prevent the 'The Agroal dependency is present but no JDBC datasources have
been defined.' build-time warning.
# See
https://github.com/quarkusio/quarkus/blob/2fbc20f445fad43aaf4f3f984b9ac8319c7c7f0a/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java#L111
-# THIS IS ALSO CONFIGURED IN
polaris-runtime-service/src/test/resources/logback-test.xml
quarkus.log.category."io.quarkus.agroal.deployment".level=ERROR
+
diff --git a/runtime/defaults/src/main/resources/application.properties
b/runtime/defaults/src/main/resources/application.properties
index a48c4bcbd..794ea1337 100644
--- a/runtime/defaults/src/main/resources/application.properties
+++ b/runtime/defaults/src/main/resources/application.properties
@@ -68,9 +68,11 @@ quarkus.log.file.path=./logs/polaris.log
quarkus.log.file.rotation.file-suffix=.yyyy-MM-dd.gz
quarkus.log.file.rotation.max-file-size=10M
quarkus.log.file.rotation.max-backup-index=14
-quarkus.log.category."org.apache.polaris".level=INFO
-quarkus.log.category."org.apache.iceberg.rest".level=INFO
-quarkus.log.category."io.smallrye.config".level=INFO
+
+# Useful loggers. Set "io.smallrye.config" to DEBUG to print the resolved
configuration properties.
+# quarkus.log.category."io.smallrye.config".level=INFO
+# quarkus.log.category."org.apache.polaris".level=INFO
+# quarkus.log.category."org.apache.iceberg.rest".level=INFO
quarkus.management.port=8182
quarkus.management.test-port=0
@@ -97,6 +99,7 @@ quarkus.otel.sdk.disabled=true
# quarkus.otel.traces.sampler=parentbased_always_on
# quarkus.otel.traces.sampler.arg=1.0d
+# This setting is a build-time setting and MUST be specified in this file.
quarkus.test.integration-test-profile=it
quarkus.fault-tolerance.global.timeout.enabled=false
diff --git a/runtime/service/build.gradle.kts b/runtime/service/build.gradle.kts
index b7a31c753..037776c40 100644
--- a/runtime/service/build.gradle.kts
+++ b/runtime/service/build.gradle.kts
@@ -67,8 +67,6 @@ dependencies {
implementation(libs.guava)
implementation(libs.slf4j.api)
- implementation("org.jboss.slf4j:slf4j-jboss-logmanager")
-
implementation(libs.hadoop.client.api)
implementation(libs.hadoop.client.runtime)
@@ -148,9 +146,8 @@ dependencies {
testImplementation(libs.threeten.extra)
testImplementation(libs.hawkular.agent.prometheus.scraper)
- testImplementation(libs.logback.classic)
-
testImplementation(project(":polaris-runtime-test-common"))
+
testImplementation("io.quarkus:quarkus-junit5")
testImplementation(libs.awaitility)
testImplementation(platform(libs.testcontainers.bom))
@@ -194,7 +191,6 @@ tasks.named("javadoc") { dependsOn("jandex") }
tasks.withType(Test::class.java).configureEach {
forkEvery = 1
- systemProperty("java.util.logging.manager",
"org.jboss.logmanager.LogManager")
if (System.getenv("AWS_REGION") == null) {
environment("AWS_REGION", "us-west-2")
}
diff --git
a/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
b/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
index 048da3f79..fff99e1f2 100644
---
a/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
+++
b/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
@@ -87,19 +87,17 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
private static final Set<String> ACCESS_DENIED_HINTS =
Set.of("access denied", "not authorized", "forbidden");
- public IcebergExceptionMapper() {}
-
@Override
public Response toResponse(RuntimeException runtimeException) {
- LOGGER.info("Handling runtimeException {}", runtimeException.getMessage());
+ getLogger().info("Handling runtimeException {}",
runtimeException.getMessage());
int responseCode = mapExceptionToResponseCode(runtimeException);
- LOGGER
+ getLogger()
.atLevel(responseCode >= 500 ? Level.INFO : Level.DEBUG)
.log("Full RuntimeException", runtimeException);
if (responseCode == Response.Status.INTERNAL_SERVER_ERROR.getStatusCode())
{
- LOGGER.error("Unhandled exception returning INTERNAL_SERVER_ERROR",
runtimeException);
+ getLogger().error("Unhandled exception returning INTERNAL_SERVER_ERROR",
runtimeException);
}
ErrorResponse icebergErrorResponse =
@@ -113,7 +111,7 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
.entity(icebergErrorResponse)
.type(MediaType.APPLICATION_JSON_TYPE)
.build();
- LOGGER.debug("Mapped exception to errorResp: {}", errorResp);
+ getLogger().debug("Mapped exception to errorResp: {}", errorResp);
return errorResp;
}
@@ -262,4 +260,9 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
return Optional.of(Status.INTERNAL_SERVER_ERROR.getStatusCode());
}
+
+ @VisibleForTesting
+ Logger getLogger() {
+ return LOGGER;
+ }
}
diff --git
a/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
b/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
index 57dcba32f..10130700c 100644
---
a/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
+++
b/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
@@ -26,6 +26,7 @@ import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.exc.ValueInstantiationException;
+import com.google.common.annotations.VisibleForTesting;
import jakarta.annotation.Nullable;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
@@ -40,7 +41,7 @@ import org.slf4j.LoggerFactory;
/** See Dropwizard's {@code
io.dropwizard.jersey.jackson.JsonProcessingExceptionMapper} */
@Provider
-public final class IcebergJsonProcessingExceptionMapper
+public class IcebergJsonProcessingExceptionMapper
implements ExceptionMapper<JsonProcessingException> {
private static final Logger LOGGER =
@@ -60,7 +61,8 @@ public final class IcebergJsonProcessingExceptionMapper
if (exception instanceof JsonGenerationException
|| exception instanceof InvalidDefinitionException) {
long id = ThreadLocalRandom.current().nextLong();
- LOGGER.error(String.format(Locale.ROOT, "Error handling a request:
%016x", id), exception);
+ getLogger()
+ .error(String.format(Locale.ROOT, "Error handling a request: %016x",
id), exception);
String message =
String.format(
Locale.ROOT,
@@ -75,8 +77,8 @@ public final class IcebergJsonProcessingExceptionMapper
/*
* Otherwise, it's those pesky users.
*/
- LOGGER.info("Unable to process JSON: {}", exception.getMessage());
- LOGGER.debug("Full JsonProcessingException", exception);
+ getLogger().info("Unable to process JSON: {}", exception.getMessage());
+ getLogger().debug("Full JsonProcessingException", exception);
String messagePrefix =
switch (exception) {
@@ -96,4 +98,9 @@ public final class IcebergJsonProcessingExceptionMapper
.entity(icebergErrorResponse)
.build();
}
+
+ @VisibleForTesting
+ Logger getLogger() {
+ return LOGGER;
+ }
}
diff --git
a/runtime/service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
b/runtime/service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
index 8b68d3890..7d0387f1d 100644
---
a/runtime/service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
+++
b/runtime/service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
@@ -18,6 +18,7 @@
*/
package org.apache.polaris.service.exception;
+import com.google.common.annotations.VisibleForTesting;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
@@ -64,7 +65,7 @@ public class PolarisExceptionMapper implements
ExceptionMapper<PolarisException>
@Override
public Response toResponse(PolarisException exception) {
Response.Status status = getStatus(exception);
- LOGGER
+ getLogger()
.atLevel(
status.getFamily() == Response.Status.Family.SERVER_ERROR ?
Level.INFO : Level.DEBUG)
.log("Full PolarisException", exception);
@@ -80,4 +81,9 @@ public class PolarisExceptionMapper implements
ExceptionMapper<PolarisException>
.type(MediaType.APPLICATION_JSON_TYPE)
.build();
}
+
+ @VisibleForTesting
+ Logger getLogger() {
+ return LOGGER;
+ }
}
diff --git
a/runtime/service/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
b/runtime/service/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
index d0da97fc3..7ba2bd3e6 100644
---
a/runtime/service/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
+++
b/runtime/service/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
@@ -18,11 +18,12 @@
*/
package org.apache.polaris.service.exception;
-import ch.qos.logback.classic.Level;
-import ch.qos.logback.classic.Logger;
-import ch.qos.logback.classic.spi.ILoggingEvent;
-import ch.qos.logback.classic.spi.IThrowableProxy;
-import ch.qos.logback.core.read.ListAppender;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
import com.fasterxml.jackson.core.JsonLocation;
import com.fasterxml.jackson.core.JsonProcessingException;
import jakarta.ws.rs.core.Response;
@@ -32,50 +33,62 @@ import java.util.stream.Stream;
import org.apache.polaris.core.exceptions.AlreadyExistsException;
import org.apache.polaris.core.exceptions.CommitConflictException;
import org.assertj.core.api.Assertions;
+import org.jboss.logmanager.Level;
+import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
-import org.slf4j.LoggerFactory;
+import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.impl.Slf4jLogger;
/** Unit tests for exception mappers */
+@SuppressWarnings("resource")
public class ExceptionMapperTest {
private static final String MESSAGE = "this is the exception message";
private static final String CAUSE = "this is the exception cause";
+ private static final org.jboss.logmanager.Logger JBOSS_LOGGER =
+ Mockito.mock(org.jboss.logmanager.Logger.class);
+
+ @BeforeEach
+ void setUp() {
+ reset(JBOSS_LOGGER);
+ when(JBOSS_LOGGER.isLoggable(any())).thenReturn(true);
+ }
+
@ParameterizedTest
@MethodSource("testFullExceptionIsLogged")
public void testFullExceptionIsLogged(
ExceptionMapper<Exception> mapper, Exception exception, Level level) {
- Logger logger = (Logger) LoggerFactory.getLogger(mapper.getClass());
- logger.setLevel(level);
- ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
- listAppender.start();
- logger.addAppender(listAppender);
mapper.toResponse(exception);
- Assertions.assertThat(
- listAppender.list.stream()
- .anyMatch(
- log -> {
- if (log.getLevel() != level) {
- return false;
- }
+ verify(JBOSS_LOGGER)
+ .logRaw(
+ argThat(
+ record -> {
+ if (record.getLevel() != level) {
+ return false;
+ }
+
+ String message = record.getMessage();
+ if (message == null) {
+ return false;
+ }
- IThrowableProxy proxy = log.getThrowableProxy();
- if (proxy == null) {
- return false;
- }
+ Throwable error = record.getThrown();
+ if (error == null) {
+ return false;
+ }
- return proxy.getMessage().contains(CAUSE)
- || Optional.ofNullable(proxy.getCause())
- .map(IThrowableProxy::getMessage)
- .orElse("")
- .contains(CAUSE);
- }))
- .as("The exception cause should be logged")
- .isTrue();
+ return message.contains(CAUSE)
+ || Optional.ofNullable(error.getCause())
+ .map(Throwable::getMessage)
+ .orElse("")
+ .contains(CAUSE);
+ }));
}
@Test
@@ -90,15 +103,30 @@ public class ExceptionMapperTest {
// inherited Exception
return Stream.of(
Arguments.of(
- new IcebergExceptionMapper(),
+ new IcebergExceptionMapper() {
+ @Override
+ Logger getLogger() {
+ return new Slf4jLogger(JBOSS_LOGGER);
+ }
+ },
new RuntimeException(MESSAGE, new RuntimeException(CAUSE)),
Level.ERROR),
Arguments.of(
- new IcebergJsonProcessingExceptionMapper(),
+ new IcebergJsonProcessingExceptionMapper() {
+ @Override
+ Logger getLogger() {
+ return new Slf4jLogger(JBOSS_LOGGER);
+ }
+ },
new TestJsonProcessingException(MESSAGE, null, new
RuntimeException(CAUSE)),
Level.DEBUG),
Arguments.of(
- new PolarisExceptionMapper(),
+ new PolarisExceptionMapper() {
+ @Override
+ Logger getLogger() {
+ return new Slf4jLogger(JBOSS_LOGGER);
+ }
+ },
new AlreadyExistsException(MESSAGE, new RuntimeException(CAUSE)),
Level.DEBUG));
}
diff --git a/runtime/spark-tests/build.gradle.kts
b/runtime/spark-tests/build.gradle.kts
index fd130a6c5..57b778f53 100644
--- a/runtime/spark-tests/build.gradle.kts
+++ b/runtime/spark-tests/build.gradle.kts
@@ -36,6 +36,7 @@ dependencies {
testImplementation(project(":polaris-tests"))
testImplementation(testFixtures(project(":polaris-runtime-service")))
+ testImplementation(project(":polaris-runtime-test-common"))
testImplementation(platform(libs.quarkus.bom))
testImplementation("io.quarkus:quarkus-junit5")
@@ -60,7 +61,6 @@ dependencies {
tasks.named<Test>("intTest").configure {
maxParallelForks = 1
- systemProperty("java.util.logging.manager",
"org.jboss.logmanager.LogManager")
if (System.getenv("AWS_REGION") == null) {
environment("AWS_REGION", "us-west-2")
}
diff --git
a/runtime/test-common/src/main/java/org/apache/polaris/test/commons/junit/GradleDuplicateLoggingWorkaround.java
b/runtime/test-common/src/main/java/org/apache/polaris/test/commons/junit/GradleDuplicateLoggingWorkaround.java
new file mode 100644
index 000000000..97e9e0a09
--- /dev/null
+++
b/runtime/test-common/src/main/java/org/apache/polaris/test/commons/junit/GradleDuplicateLoggingWorkaround.java
@@ -0,0 +1,84 @@
+/*
+ * 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.
+ */
+
+package org.apache.polaris.test.commons.junit;
+
+import jakarta.annotation.Priority;
+import jakarta.enterprise.event.Observes;
+import jakarta.enterprise.event.Startup;
+import jakarta.inject.Singleton;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * JUnit extension + Singleton CDI bean that "fixes" test logging
configuration.
+ *
+ * <p>This component removes the SLF4JBridgeHandler installed by Gradle, to
prevent duplicate
+ * logging messages in tests. Without this fix, Gradle tests using JBoss
LogManager (which is the
+ * default in Quarkus) will log each message twice: once via the
SLF4JBridgeHandler, without any
+ * formatting, and once via the JBoss LogManager (with formatting). This is
annoying because the
+ * non-formatted messages appear on the console, regardless of the log level.
+ *
+ * <p>Note: this issue has been reported to Quarkus and appears fixed, but
only the formatting part
+ * is fixed, not the duplicate messages.
+ *
+ * @see <a href="https://github.com/quarkusio/quarkus/issues/22844";>Gradle
tests (with JBoss
+ * LogManager setup) output duplicate unformatted messages</a>
+ * @see <a href="https://github.com/quarkusio/quarkus/issues/48763";>Gradle
+ * testLogging.showStandardStreams = false ignored by Quarkus tests with
JBoss LogManager</a>
+ */
+@Singleton
+public class GradleDuplicateLoggingWorkaround implements BeforeAllCallback {
+
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(GradleDuplicateLoggingWorkaround.class);
+
+ private static final AtomicBoolean DONE = new AtomicBoolean(false);
+
+ public void onStartup(@Observes @Priority(1) Startup event) {
+ // Sometimes the application is started before the test extension is
invoked,
+ // so we need to ensure the SLF4JBridgeHandler is removed at startup as
well.
+ removeGradleHandlers();
+ }
+
+ @Override
+ public void beforeAll(ExtensionContext context) {
+ removeGradleHandlers();
+ }
+
+ private static void removeGradleHandlers() {
+ if (!DONE.getAndSet(true)) {
+ var logger =
org.jboss.logmanager.LogManager.getLogManager().getLogger("");
+ var found = false;
+ for (var handler : logger.getHandlers()) {
+ if (handler.getClass().getSimpleName().equals("SLF4JBridgeHandler")) {
+ logger.removeHandler(handler);
+ found = true;
+ }
+ }
+ if (found) {
+ LOGGER.warn(
+ "Removed SLF4JBridgeHandler to fix duplicate logging messages in
Gradle tests.");
+ }
+ }
+ }
+}
diff --git
a/runtime/test-common/src/main/resources/META-INF/services/org.junit.jupiter.api.extension.Extension
b/runtime/test-common/src/main/resources/META-INF/services/org.junit.jupiter.api.extension.Extension
new file mode 100644
index 000000000..0214d2dd3
--- /dev/null
+++
b/runtime/test-common/src/main/resources/META-INF/services/org.junit.jupiter.api.extension.Extension
@@ -0,0 +1,20 @@
+#
+# 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.
+#
+
+org.apache.polaris.test.commons.junit.GradleDuplicateLoggingWorkaround
\ No newline at end of file
