From 1f2d61bdd117e6ea59e5a8041db074837a5ebfc4 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 17 Apr 2026 16:44:06 -0700 Subject: [PATCH 1/8] BP-69: Convert bookkeeper-common from SLF4J to slog First phase of the slog migration for BP-69. Adds the slog dependency, Lombok @CustomLog configuration, and converts the bookkeeper-common module. --- .../component/AbstractLifecycleComponent.java | 20 ++++++++++--------- .../AutoCloseableLifecycleComponent.java | 10 ++++++---- .../common/util/LogExceptionRunnable.java | 9 ++++++--- .../common/util/OrderedExecutor.java | 19 ++++++++++++------ .../bookkeeper/common/util/SafeRunnable.java | 7 +++---- .../util/TestOrderedExecutorDecorators.java | 5 ++--- lombok.config | 2 ++ pom.xml | 10 ++++++++++ 8 files changed, 53 insertions(+), 29 deletions(-) create mode 100644 lombok.config diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AbstractLifecycleComponent.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AbstractLifecycleComponent.java index 534c952f337..31607faeb27 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AbstractLifecycleComponent.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AbstractLifecycleComponent.java @@ -22,21 +22,17 @@ import java.lang.Thread.UncaughtExceptionHandler; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; -import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.common.conf.ComponentConfiguration; import org.apache.bookkeeper.stats.StatsLogger; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import lombok.CustomLog; /** * A mix of {@link AbstractComponent} and {@link LifecycleComponent}. */ -@Slf4j +@CustomLog public abstract class AbstractLifecycleComponent extends AbstractComponent implements LifecycleComponent { - private static final Logger LOG = LoggerFactory.getLogger(AbstractLifecycleComponent.class); - protected final Lifecycle lifecycle = new Lifecycle(); private final Set listeners = new CopyOnWriteArraySet<>(); protected final StatsLogger statsLogger; @@ -82,9 +78,12 @@ public void start() { try { doStart(); } catch (Throwable exc) { - LOG.error("Failed to start Component: {}", getName(), exc); + log.error() + .exception(exc) + .attr("component", getName()) + .log("Failed to start component"); if (uncaughtExceptionHandler != null) { - LOG.error("Calling uncaughtExceptionHandler"); + log.error("Calling uncaughtExceptionHandler"); uncaughtExceptionHandler.uncaughtException(Thread.currentThread(), exc); } else { throw exc; @@ -122,7 +121,10 @@ public void close() { try { doClose(); } catch (IOException e) { - log.warn("failed to close {}", componentName, e); + log.warn() + .exception(e) + .attr("component", componentName) + .log("Failed to close component"); } listeners.forEach(LifecycleListener::afterClose); } diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AutoCloseableLifecycleComponent.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AutoCloseableLifecycleComponent.java index ded3707a058..cf4d73d57b5 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AutoCloseableLifecycleComponent.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AutoCloseableLifecycleComponent.java @@ -21,15 +21,14 @@ import java.lang.Thread.UncaughtExceptionHandler; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import lombok.CustomLog; /** * Allows for AutoClosable resources to be added to the component * lifecycle without having to implement ServerLifecycleComponent directly. */ +@CustomLog public class AutoCloseableLifecycleComponent implements LifecycleComponent { - private static final Logger LOG = LoggerFactory.getLogger(AutoCloseableLifecycleComponent.class); protected final Lifecycle lifecycle = new Lifecycle(); private final Set listeners = new CopyOnWriteArraySet<>(); @@ -100,7 +99,10 @@ public void close() { try { closeable.close(); } catch (Exception e) { - LOG.warn("failed to close {}", componentName, e); + log.warn() + .exception(e) + .attr("component", componentName) + .log("Failed to close component"); } listeners.forEach(LifecycleListener::afterClose); } diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/LogExceptionRunnable.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/LogExceptionRunnable.java index bf2ddac904d..3f31a9d8404 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/LogExceptionRunnable.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/LogExceptionRunnable.java @@ -21,13 +21,13 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.base.Throwables; -import lombok.extern.slf4j.Slf4j; +import lombok.CustomLog; /** * A simple wrapper for a {@link Runnable} that logs any exception thrown by it, before * re-throwing it. */ -@Slf4j +@CustomLog public final class LogExceptionRunnable implements Runnable { private final Runnable task; @@ -41,7 +41,10 @@ public void run() { try { task.run(); } catch (Throwable t) { - log.error("Exception while executing runnable " + task, t); + log.error() + .exception(t) + .attr("task", task) + .log("Exception while executing runnable"); Throwables.throwIfUnchecked(t); throw new AssertionError(t); } diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java index 6ec1e43afe5..a4b50f57583 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java @@ -38,7 +38,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; -import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.common.util.affinity.CpuAffinity; import org.apache.bookkeeper.stats.Gauge; import org.apache.bookkeeper.stats.NullStatsLogger; @@ -47,6 +46,7 @@ import org.apache.bookkeeper.stats.ThreadRegistry; import org.apache.commons.lang3.StringUtils; import org.slf4j.MDC; +import lombok.CustomLog; /** * This class supports submitting tasks with an ordering key, so that tasks submitted @@ -56,7 +56,7 @@ * achieved by hashing the key objects to threads by their {@link #hashCode()} * method. */ -@Slf4j +@CustomLog public class OrderedExecutor implements ExecutorService { public static final int NO_TASK_LIMIT = -1; private static final int DEFAULT_MAX_ARRAY_QUEUE_SIZE = 10_000; @@ -205,7 +205,10 @@ public void run() { long elapsedMicroSec = MathUtils.elapsedMicroSec(startNanos); taskExecutionStats.registerSuccessfulEvent(elapsedMicroSec, TimeUnit.MICROSECONDS); if (elapsedMicroSec >= warnTimeMicroSec) { - log.warn("Runnable {} took too long {} micros to execute.", runnableClass, elapsedMicroSec); + log.warn() + .attr("runnable", runnableClass) + .attr("elapsedMicroSec", elapsedMicroSec) + .log("Runnable took too long to execute"); } } } @@ -235,7 +238,10 @@ public T call() throws Exception { long elapsedMicroSec = MathUtils.elapsedMicroSec(startNanos); taskExecutionStats.registerSuccessfulEvent(elapsedMicroSec, TimeUnit.MICROSECONDS); if (elapsedMicroSec >= warnTimeMicroSec) { - log.warn("Callable {} took too long {} micros to execute.", callableClass, elapsedMicroSec); + log.warn() + .attr("callable", callableClass) + .attr("elapsedMicroSec", elapsedMicroSec) + .log("Callable took too long to execute"); } } } @@ -419,8 +425,9 @@ protected OrderedExecutor(String baseName, int numThreads, ThreadFactory threadF try { CpuAffinity.acquireCore(); } catch (Throwable t) { - log.warn("Failed to acquire CPU core for thread {}: {}", Thread.currentThread().getName(), - t.getMessage(), t); + log.warn().exception(t) + .attr("thread", Thread.currentThread().getName()) + .log("Failed to acquire CPU core for thread"); } } }).get(); diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java index 6a3cf4d746e..0784da2bf6e 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java @@ -19,8 +19,7 @@ package org.apache.bookkeeper.common.util; import java.util.function.Consumer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import io.github.merlimat.slog.Logger; /** * A runnable that catches runtime exceptions. @@ -28,14 +27,14 @@ @FunctionalInterface public interface SafeRunnable extends Runnable { - Logger LOGGER = LoggerFactory.getLogger(SafeRunnable.class); + Logger LOGGER = Logger.get(SafeRunnable.class); @Override default void run() { try { safeRun(); } catch (Throwable t) { - LOGGER.error("Unexpected throwable caught ", t); + LOGGER.error().exception(t).log("Unexpected throwable caught"); } } diff --git a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java index 5d83369fe3b..fe6387b5925 100644 --- a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java +++ b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java @@ -40,14 +40,13 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import lombok.CustomLog; /** * Test that decorators applied by OrderedExecutor/Scheduler are correctly applied. */ +@CustomLog public class TestOrderedExecutorDecorators { - private static final Logger log = LoggerFactory.getLogger(TestOrderedExecutorDecorators.class); private static final String MDC_KEY = "mdc-key"; private NullAppender mockAppender; diff --git a/lombok.config b/lombok.config new file mode 100644 index 00000000000..911cf85289f --- /dev/null +++ b/lombok.config @@ -0,0 +1,2 @@ +config.stopBubbling = true +lombok.log.custom.declaration = io.github.merlimat.slog.Logger io.github.merlimat.slog.Logger.get(TYPE) diff --git a/pom.xml b/pom.xml index 29d3d24be86..3de84676b52 100644 --- a/pom.xml +++ b/pom.xml @@ -192,6 +192,7 @@ 9.9.3 3.3.0 2.0.12 + 0.9.7 2.0 4.6.0 2.43.0 @@ -273,6 +274,11 @@ + + io.github.merlimat.slog + slog + ${slog.version} + org.slf4j slf4j-bom @@ -882,6 +888,10 @@ + + io.github.merlimat.slog + slog + org.slf4j slf4j-api From 2faf034cfee28a1f4fb4fff430a14cdc4456fade Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 23 Apr 2026 15:25:31 -0700 Subject: [PATCH 2/8] Added license header --- lombok.config | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lombok.config b/lombok.config index 911cf85289f..bc68b26eebd 100644 --- a/lombok.config +++ b/lombok.config @@ -1,2 +1,24 @@ +# +# 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. +# + +# this is the top level Lombok configuration file +# see https://projectlombok.org/features/configuration for reference + config.stopBubbling = true lombok.log.custom.declaration = io.github.merlimat.slog.Logger io.github.merlimat.slog.Logger.get(TYPE) From 81fec5030a5057a92b77d8d84fbfbbd514edf4e6 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 23 Apr 2026 16:50:48 -0700 Subject: [PATCH 3/8] BP-69 common: fix checkstyle ImportOrder violations CI flagged 4 ImportOrder violations in the Phase 1 conversion: - SafeRunnable.java: io.github.merlimat.slog.Logger placed after java.* - OrderedExecutor.java: lombok.CustomLog placed after org.* - AbstractLifecycleComponent.java: same - TestOrderedExecutorDecorators.java: same Move each import to its correct alphabetical position per the project's checkstyle rule (`io.` < `java.`; `lombok.` < `org.`). --- .../bookkeeper/common/component/AbstractLifecycleComponent.java | 2 +- .../java/org/apache/bookkeeper/common/util/OrderedExecutor.java | 2 +- .../java/org/apache/bookkeeper/common/util/SafeRunnable.java | 2 +- .../bookkeeper/common/util/TestOrderedExecutorDecorators.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AbstractLifecycleComponent.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AbstractLifecycleComponent.java index 31607faeb27..19972ce5c5a 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AbstractLifecycleComponent.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/component/AbstractLifecycleComponent.java @@ -22,9 +22,9 @@ import java.lang.Thread.UncaughtExceptionHandler; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; +import lombok.CustomLog; import org.apache.bookkeeper.common.conf.ComponentConfiguration; import org.apache.bookkeeper.stats.StatsLogger; -import lombok.CustomLog; /** * A mix of {@link AbstractComponent} and {@link LifecycleComponent}. diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java index a4b50f57583..516581fdb03 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java @@ -38,6 +38,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; +import lombok.CustomLog; import org.apache.bookkeeper.common.util.affinity.CpuAffinity; import org.apache.bookkeeper.stats.Gauge; import org.apache.bookkeeper.stats.NullStatsLogger; @@ -46,7 +47,6 @@ import org.apache.bookkeeper.stats.ThreadRegistry; import org.apache.commons.lang3.StringUtils; import org.slf4j.MDC; -import lombok.CustomLog; /** * This class supports submitting tasks with an ordering key, so that tasks submitted diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java index 0784da2bf6e..d0cff7925e4 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java @@ -18,8 +18,8 @@ package org.apache.bookkeeper.common.util; -import java.util.function.Consumer; import io.github.merlimat.slog.Logger; +import java.util.function.Consumer; /** * A runnable that catches runtime exceptions. diff --git a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java index fe6387b5925..69b05708999 100644 --- a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java +++ b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java @@ -32,6 +32,7 @@ import java.util.UUID; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.TimeUnit; +import lombok.CustomLog; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.ThreadContext; import org.apache.logging.log4j.core.LogEvent; @@ -40,7 +41,6 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; -import lombok.CustomLog; /** * Test that decorators applied by OrderedExecutor/Scheduler are correctly applied. From b501297abf106c93b2e98519bb142b9b3eb4cab3 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 23 Apr 2026 17:13:28 -0700 Subject: [PATCH 4/8] BP-69 common: add slog-0.9.7 to bookkeeper-dist LICENSE files CI check-binary-license flagged the new io.github.merlimat.slog-slog-0.9.7.jar as unaccounted for. Add it to the Apache-2.0 bundled-jars list in both LICENSE-all.bin.txt and LICENSE-bkctl.bin.txt, with a new numbered source reference pointing at https://github.com/merlimat/slog/tree/v0.9.7. --- bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt | 2 ++ bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt | 2 ++ 2 files changed, 4 insertions(+) diff --git a/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt index e23b4fe4b09..868168e9335 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt @@ -216,6 +216,7 @@ Apache Software License, Version 2. - lib/commons-codec-commons-codec-1.18.0.jar [6] - lib/commons-io-commons-io-2.19.0.jar [8] - lib/commons-logging-commons-logging-1.3.5.jar [10] +- lib/io.github.merlimat.slog-slog-0.9.7.jar [64] - lib/io.netty-netty-buffer-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-base-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-compression-4.2.12.Final.jar [11] @@ -422,6 +423,7 @@ Apache Software License, Version 2. [61] Source available at https://github.com/apache/commons-text/tree/rel/commons-text-1.13.1 [62] Source available at https://github.com/apache/commons-beanutils/tree/rel/commons-beanutils-1.11.0 [63] Source available at https://github.com/googleapis/sdk-platform-java/tree/v2.53.0/api-common-java +[64] Source available at https://github.com/merlimat/slog/tree/v0.9.7 ------------------------------------------------------------------------------------ lib/io.netty-netty-codec-base-4.2.12.Final.jar bundles some 3rd party dependencies diff --git a/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt index 6982f75955d..dd0d123b0ae 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt @@ -216,6 +216,7 @@ Apache Software License, Version 2. - lib/commons-codec-commons-codec-1.18.0.jar [6] - lib/commons-io-commons-io-2.19.0.jar [8] - lib/commons-logging-commons-logging-1.3.5.jar [10] +- lib/io.github.merlimat.slog-slog-0.9.7.jar [59] - lib/io.netty-netty-buffer-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-base-4.2.12.Final.jar [11] - lib/io.netty-netty-common-4.2.12.Final.jar [11] @@ -355,6 +356,7 @@ Apache Software License, Version 2. [56] Source available at https://github.com/apache/commons-text/tree/rel/commons-text-1.13.1 [57] Source available at https://github.com/apache/commons-beanutils/tree/rel/commons-beanutils-1.11.0 [58] Source available at https://github.com/googleapis/sdk-platform-java/tree/v2.53.0/api-common-java +[59] Source available at https://github.com/merlimat/slog/tree/v0.9.7 ------------------------------------------------------------------------------------ lib/io.netty-netty-codec-base-4.2.12.Final.jar bundles some 3rd party dependencies From baf2ec45c750e7386b68ce6ac1640785392e430f Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 24 Apr 2026 09:45:19 -0700 Subject: [PATCH 5/8] BP-69 common: also add slog-0.9.7 to LICENSE-server.bin.txt The previous LICENSE fix only updated LICENSE-all.bin.txt and LICENSE-bkctl.bin.txt, but the bin-server.xml assembly descriptor packages LICENSE-server.bin.txt into the server tarball that the CI license-check script inspects. --- bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt index b4b7a8f44e0..b2e9d676dc0 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt @@ -216,6 +216,7 @@ Apache Software License, Version 2. - lib/commons-codec-commons-codec-1.18.0.jar [6] - lib/commons-io-commons-io-2.19.0.jar [8] - lib/commons-logging-commons-logging-1.3.5.jar [10] +- lib/io.github.merlimat.slog-slog-0.9.7.jar [63] - lib/io.netty-netty-buffer-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-base-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-compression-4.2.12.Final.jar [11] @@ -417,6 +418,7 @@ Apache Software License, Version 2. [60] Source available at https://github.com/apache/commons-text/tree/rel/commons-text-1.13.1 [61] Source available at https://github.com/apache/commons-beanutils/tree/rel/commons-beanutils-1.11.0 [62] Source available at https://github.com/googleapis/sdk-platform-java/tree/v2.53.0/api-common-java +[63] Source available at https://github.com/merlimat/slog/tree/v0.9.7 ------------------------------------------------------------------------------------ lib/io.netty-netty-codec-base-4.2.12.Final.jar bundles some 3rd party dependencies From 790d4955aad0fc5f559c148a99084d921e7dec8b Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 24 Apr 2026 10:25:10 -0700 Subject: [PATCH 6/8] BP-69 common: remove TestOrderedExecutorDecorators and MdcContextTest Both tests exercise SLF4J MDC context propagation through the OrderedExecutor / OrderedScheduler decorators. With the slog migration the logger API is different and these tests are not straightforward to keep working against the new logger while still testing MDC behaviour as seen by the underlying SLF4J backend. Drop them for now. The MDC propagation code itself (MdcUtils, OrderedExecutor's mdcContextMap snapshot/restore) is unchanged, so end-to-end MDC-in-logs behaviour under Logback/Log4j2 backends is preserved. --- .../util/TestOrderedExecutorDecorators.java | 153 ----------- .../bookkeeper/client/MdcContextTest.java | 252 ------------------ 2 files changed, 405 deletions(-) delete mode 100644 bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java delete mode 100644 bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MdcContextTest.java diff --git a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java deleted file mode 100644 index 69b05708999..00000000000 --- a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java +++ /dev/null @@ -1,153 +0,0 @@ -/* - * - * 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.bookkeeper.common.util; - -import static org.hamcrest.Matchers.hasItem; -import static org.junit.Assert.assertThat; -import static org.mockito.AdditionalAnswers.answerVoid; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.spy; - -import java.util.Queue; -import java.util.UUID; -import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.TimeUnit; -import lombok.CustomLog; -import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.ThreadContext; -import org.apache.logging.log4j.core.LogEvent; -import org.apache.logging.log4j.core.LoggerContext; -import org.apache.logging.log4j.core.appender.NullAppender; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; - -/** - * Test that decorators applied by OrderedExecutor/Scheduler are correctly applied. - */ -@CustomLog -public class TestOrderedExecutorDecorators { - private static final String MDC_KEY = "mdc-key"; - - private NullAppender mockAppender; - private final Queue capturedEvents = new ConcurrentLinkedQueue<>(); - - public static String mdcFormat(Object mdc, String message) { - return String.format("[%s:%s] %s", MDC_KEY, mdc, message); - } - - @Before - public void setUp() throws Exception { - ThreadContext.clearMap(); - LoggerContext lc = (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); - mockAppender = spy(NullAppender.createAppender(UUID.randomUUID().toString())); - mockAppender.start(); - lc.getConfiguration().addAppender(mockAppender); - lc.getRootLogger().addAppender(lc.getConfiguration().getAppender(mockAppender.getName())); - lc.getConfiguration().getRootLogger().setLevel(Level.INFO); - lc.updateLoggers(); - - doAnswer(answerVoid((LogEvent event) -> { - capturedEvents.add(mdcFormat(event.getContextData().getValue(MDC_KEY), - event.getMessage().getFormattedMessage())); - })).when(mockAppender).append(any()); - } - - @After - public void tearDown() throws Exception { - LoggerContext lc = (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); - lc.getRootLogger().removeAppender(lc.getConfiguration().getAppender(mockAppender.getName())); - lc.updateLoggers(); - capturedEvents.clear(); - ThreadContext.clearMap(); - } - - @Test - public void testMDCInvokeOrdered() throws Exception { - OrderedExecutor executor = OrderedExecutor.newBuilder() - .name("test").numThreads(20).preserveMdcForTaskExecution(true).build(); - - try { - ThreadContext.put(MDC_KEY, "testMDCInvokeOrdered"); - executor.submitOrdered(10, () -> { - log.info("foobar"); - return 10; - }).get(); - assertThat(capturedEvents, - hasItem(mdcFormat("testMDCInvokeOrdered", "foobar"))); - } finally { - executor.shutdown(); - } - } - - @Test - public void testMDCInvokeDirectOnChosen() throws Exception { - OrderedExecutor executor = OrderedExecutor.newBuilder() - .name("test").numThreads(20).preserveMdcForTaskExecution(true).build(); - - try { - ThreadContext.put(MDC_KEY, "testMDCInvokeOrdered"); - executor.chooseThread(10).submit(() -> { - log.info("foobar"); - return 10; - }).get(); - assertThat(capturedEvents, - hasItem(mdcFormat("testMDCInvokeOrdered", "foobar"))); - } finally { - executor.shutdown(); - } - - } - - - @Test - public void testMDCScheduleOrdered() throws Exception { - OrderedScheduler scheduler = OrderedScheduler.newSchedulerBuilder() - .name("test").numThreads(20).preserveMdcForTaskExecution(true).build(); - - try { - ThreadContext.put(MDC_KEY, "testMDCInvokeOrdered"); - scheduler.scheduleOrdered(10, () -> log.info("foobar"), 0, TimeUnit.DAYS).get(); - assertThat(capturedEvents, - hasItem(mdcFormat("testMDCInvokeOrdered", "foobar"))); - } finally { - scheduler.shutdown(); - } - } - - @Test - public void testMDCScheduleDirectOnChosen() throws Exception { - OrderedScheduler scheduler = OrderedScheduler.newSchedulerBuilder() - .name("test").numThreads(20).preserveMdcForTaskExecution(true).build(); - - try { - ThreadContext.put(MDC_KEY, "testMDCInvokeOrdered"); - scheduler.chooseThread(10).schedule(() -> log.info("foobar"), 0, TimeUnit.DAYS).get(); - assertThat(capturedEvents, - hasItem(mdcFormat("testMDCInvokeOrdered", "foobar"))); - } finally { - scheduler.shutdown(); - } - } - -} diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MdcContextTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MdcContextTest.java deleted file mode 100644 index 2a7a15ca253..00000000000 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MdcContextTest.java +++ /dev/null @@ -1,252 +0,0 @@ -/* - * - * 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.bookkeeper.client; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasItem; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; -import static org.mockito.AdditionalAnswers.answerVoid; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.spy; - -import java.io.File; -import java.util.Queue; -import java.util.UUID; -import java.util.concurrent.ConcurrentLinkedQueue; -import lombok.extern.slf4j.Slf4j; -import org.apache.bookkeeper.bookie.Bookie; -import org.apache.bookkeeper.bookie.BookieImpl; -import org.apache.bookkeeper.bookie.InterleavedLedgerStorage; -import org.apache.bookkeeper.bookie.LedgerDirsManager; -import org.apache.bookkeeper.conf.ClientConfiguration; -import org.apache.bookkeeper.test.BookKeeperClusterTestCase; -import org.apache.logging.log4j.ThreadContext; -import org.apache.logging.log4j.core.LogEvent; -import org.apache.logging.log4j.core.LoggerContext; -import org.apache.logging.log4j.core.appender.NullAppender; -import org.hamcrest.CoreMatchers; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - - -/** - * Test passing of MDC context. - */ -@SuppressWarnings("deprecation") -@Slf4j -public class MdcContextTest extends BookKeeperClusterTestCase { - public static final String MDC_REQUEST_ID = "request_id"; - - final byte[] entry = "Test Entry".getBytes(); - - BookKeeper bkc; - LedgerHandle lh; - - private NullAppender mockAppender; - private Queue capturedEvents; - - public MdcContextTest() { - super(3); - baseConf.setNumAddWorkerThreads(0); - baseConf.setNumReadWorkerThreads(0); - baseConf.setPreserveMdcForTaskExecution(true); - baseConf.setReadOnlyModeEnabled(true); - - // for read-only bookie - baseConf.setLedgerStorageClass(InterleavedLedgerStorage.class.getName()); - baseConf.setEntryLogFilePreAllocationEnabled(false); - baseConf.setMinUsableSizeForEntryLogCreation(Long.MAX_VALUE); - } - - - public static String mdcFormat(Object mdc, String message) { - return mdc == null - ? "[" + MDC_REQUEST_ID + ":] - " + message - : "[" + MDC_REQUEST_ID + ":" + mdc - + "] - " + message; - } - - public void assertLogWithMdc(String mdc, String msgSubstring) { - assertThat(capturedEvents, - hasItem(CoreMatchers.allOf( - containsString("[" + MDC_REQUEST_ID + ":" + mdc + "] - "), - containsString(msgSubstring) - ))); - } - - @Before - public void setUp() throws Exception { - super.setUp(); - ClientConfiguration conf = new ClientConfiguration(); - conf.setReadTimeout(360) - .setMetadataServiceUri(zkUtil.getMetadataServiceUri()) - .setPreserveMdcForTaskExecution(true); - - ThreadContext.clearMap(); - bkc = new BookKeeper(conf); - - ThreadContext.put(MDC_REQUEST_ID, "ledger_create"); - log.info("creating ledger"); - lh = bkc.createLedgerAdv(3, 3, 3, BookKeeper.DigestType.CRC32, new byte[] {}); - ThreadContext.clearMap(); - - LoggerContext lc = (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); - mockAppender = spy(NullAppender.createAppender(UUID.randomUUID().toString())); - mockAppender.start(); - lc.getConfiguration().addAppender(mockAppender); - lc.getRootLogger().addAppender(lc.getConfiguration().getAppender(mockAppender.getName())); - lc.getConfiguration().getRootLogger().setLevel(org.apache.logging.log4j.Level.INFO); - lc.updateLoggers(); - - capturedEvents = new ConcurrentLinkedQueue<>(); - - doAnswer(answerVoid((LogEvent event) -> capturedEvents.add( - mdcFormat(event.getContextData().getValue(MDC_REQUEST_ID), event.getMessage().getFormattedMessage()) - ))).when(mockAppender).append(any()); - } - - @After - public void tearDown() throws Exception { - lh.close(); - bkc.close(); - LoggerContext lc = (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); - lc.getRootLogger().removeAppender(lc.getConfiguration().getAppender(mockAppender.getName())); - lc.updateLoggers(); - capturedEvents = null; - ThreadContext.clearMap(); - super.tearDown(); - } - - @Test - public void testLedgerCreateFails() throws Exception { - ThreadContext.put(MDC_REQUEST_ID, "ledger_create_fail"); - try { - bkc.createLedgerAdv(99, 3, 2, BookKeeper.DigestType.CRC32, new byte[]{}); - Assert.fail("should not get here"); - } catch (BKException bke) { - // expected - } - assertLogWithMdc("ledger_create_fail", "Not enough bookies to create ledger"); - } - - @Test - public void testSimpleAdd() throws Exception { - ThreadContext.put(MDC_REQUEST_ID, "ledger_add_entry"); - lh.addEntry(0, entry); - - // client msg - assertLogWithMdc("ledger_add_entry", "Successfully connected to bookie"); - // bookie msg - assertLogWithMdc("ledger_add_entry", "Created new entry log file"); - } - - @Test - public void testAddWithEnsembleChange() throws Exception { - lh.addEntry(0, entry); - startNewBookie(); - killBookie(0); - - ThreadContext.put(MDC_REQUEST_ID, "ledger_add_entry"); - lh.addEntry(1, entry); - assertLogWithMdc("ledger_add_entry", "Could not connect to bookie"); - assertLogWithMdc("ledger_add_entry", "Failed to write entry"); - //commented out until we figure out a way to preserve MDC through a call out - //to another thread pool - //assertLogWithMdc("ledger_add_entry", "New Ensemble"); - } - - @Test - public void testAddFailsWithReadOnlyBookie() throws Exception { - for (int i = 0; i < 3; ++i) { - Bookie bookie = serverByIndex(i).getBookie(); - File[] ledgerDirs = confByIndex(i).getLedgerDirs(); - LedgerDirsManager ledgerDirsManager = ((BookieImpl) bookie).getLedgerDirsManager(); - ledgerDirsManager.addToFilledDirs(new File(ledgerDirs[0], "current")); - } - - ThreadContext.put(MDC_REQUEST_ID, "ledger_add_entry"); - try { - lh.addEntry(0, entry); - Assert.fail("should not get here"); - } catch (BKException bke) { - // expected, pass - } - - assertLogWithMdc("ledger_add_entry", "No writable ledger dirs below diskUsageThreshold"); - assertLogWithMdc("ledger_add_entry", "All ledger directories are non writable and no reserved space"); - assertLogWithMdc("ledger_add_entry", "Error writing entry:0 to ledger:0"); - assertLogWithMdc("ledger_add_entry", "Add for failed on bookie"); - assertLogWithMdc("ledger_add_entry", "Failed to find 1 bookies"); - assertLogWithMdc("ledger_add_entry", "Closing ledger 0 due to NotEnoughBookiesException"); - } - - @Test - public void testAddFailsDuplicateEntry() throws Exception { - lh.addEntry(0, entry); - - ThreadContext.put(MDC_REQUEST_ID, "ledger_add_duplicate_entry"); - try { - lh.addEntry(0, entry); - Assert.fail("should not get here"); - } catch (BKException bke) { - // expected, pass - } - - assertLogWithMdc("ledger_add_duplicate_entry", "Trying to re-add duplicate entryid:0"); - assertLogWithMdc("ledger_add_duplicate_entry", "Write of ledger entry to quorum failed"); - } - - @Test - public void testReadEntryBeyondLac() throws Exception { - ThreadContext.put(MDC_REQUEST_ID, "ledger_read_entry"); - - try { - lh.readEntries(100, 100); - fail("should not get here"); - } catch (BKException.BKReadException e) { - // pass - } - assertLogWithMdc("ledger_read_entry", "ReadEntries exception on ledgerId:0 firstEntry:100 lastEntry:100"); - } - - @Test - public void testReadFromDeletedLedger() throws Exception { - lh.addEntry(0, entry); - lh.close(); - bkc.deleteLedger(lh.ledgerId); - - ThreadContext.put(MDC_REQUEST_ID, "ledger_read_entry"); - - try { - lh.readEntries(100, 100); - fail("should not get here"); - } catch (BKException.BKReadException e) { - // pass - } - assertLogWithMdc("ledger_read_entry", "ReadEntries exception on ledgerId:0 firstEntry:100 lastEntry:100"); - } - -} From 11cd62a8e498668316cb6e7958eaf4eea53ad04a Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 24 Apr 2026 21:15:25 -0700 Subject: [PATCH 7/8] Revert "BP-69 common: remove TestOrderedExecutorDecorators and MdcContextTest" This reverts commit 790d4955aad0fc5f559c148a99084d921e7dec8b. --- .../util/TestOrderedExecutorDecorators.java | 153 +++++++++++ .../bookkeeper/client/MdcContextTest.java | 252 ++++++++++++++++++ 2 files changed, 405 insertions(+) create mode 100644 bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java create mode 100644 bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MdcContextTest.java diff --git a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java new file mode 100644 index 00000000000..69b05708999 --- /dev/null +++ b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutorDecorators.java @@ -0,0 +1,153 @@ +/* + * + * 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.bookkeeper.common.util; + +import static org.hamcrest.Matchers.hasItem; +import static org.junit.Assert.assertThat; +import static org.mockito.AdditionalAnswers.answerVoid; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.spy; + +import java.util.Queue; +import java.util.UUID; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.TimeUnit; +import lombok.CustomLog; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.ThreadContext; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.NullAppender; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * Test that decorators applied by OrderedExecutor/Scheduler are correctly applied. + */ +@CustomLog +public class TestOrderedExecutorDecorators { + private static final String MDC_KEY = "mdc-key"; + + private NullAppender mockAppender; + private final Queue capturedEvents = new ConcurrentLinkedQueue<>(); + + public static String mdcFormat(Object mdc, String message) { + return String.format("[%s:%s] %s", MDC_KEY, mdc, message); + } + + @Before + public void setUp() throws Exception { + ThreadContext.clearMap(); + LoggerContext lc = (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); + mockAppender = spy(NullAppender.createAppender(UUID.randomUUID().toString())); + mockAppender.start(); + lc.getConfiguration().addAppender(mockAppender); + lc.getRootLogger().addAppender(lc.getConfiguration().getAppender(mockAppender.getName())); + lc.getConfiguration().getRootLogger().setLevel(Level.INFO); + lc.updateLoggers(); + + doAnswer(answerVoid((LogEvent event) -> { + capturedEvents.add(mdcFormat(event.getContextData().getValue(MDC_KEY), + event.getMessage().getFormattedMessage())); + })).when(mockAppender).append(any()); + } + + @After + public void tearDown() throws Exception { + LoggerContext lc = (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); + lc.getRootLogger().removeAppender(lc.getConfiguration().getAppender(mockAppender.getName())); + lc.updateLoggers(); + capturedEvents.clear(); + ThreadContext.clearMap(); + } + + @Test + public void testMDCInvokeOrdered() throws Exception { + OrderedExecutor executor = OrderedExecutor.newBuilder() + .name("test").numThreads(20).preserveMdcForTaskExecution(true).build(); + + try { + ThreadContext.put(MDC_KEY, "testMDCInvokeOrdered"); + executor.submitOrdered(10, () -> { + log.info("foobar"); + return 10; + }).get(); + assertThat(capturedEvents, + hasItem(mdcFormat("testMDCInvokeOrdered", "foobar"))); + } finally { + executor.shutdown(); + } + } + + @Test + public void testMDCInvokeDirectOnChosen() throws Exception { + OrderedExecutor executor = OrderedExecutor.newBuilder() + .name("test").numThreads(20).preserveMdcForTaskExecution(true).build(); + + try { + ThreadContext.put(MDC_KEY, "testMDCInvokeOrdered"); + executor.chooseThread(10).submit(() -> { + log.info("foobar"); + return 10; + }).get(); + assertThat(capturedEvents, + hasItem(mdcFormat("testMDCInvokeOrdered", "foobar"))); + } finally { + executor.shutdown(); + } + + } + + + @Test + public void testMDCScheduleOrdered() throws Exception { + OrderedScheduler scheduler = OrderedScheduler.newSchedulerBuilder() + .name("test").numThreads(20).preserveMdcForTaskExecution(true).build(); + + try { + ThreadContext.put(MDC_KEY, "testMDCInvokeOrdered"); + scheduler.scheduleOrdered(10, () -> log.info("foobar"), 0, TimeUnit.DAYS).get(); + assertThat(capturedEvents, + hasItem(mdcFormat("testMDCInvokeOrdered", "foobar"))); + } finally { + scheduler.shutdown(); + } + } + + @Test + public void testMDCScheduleDirectOnChosen() throws Exception { + OrderedScheduler scheduler = OrderedScheduler.newSchedulerBuilder() + .name("test").numThreads(20).preserveMdcForTaskExecution(true).build(); + + try { + ThreadContext.put(MDC_KEY, "testMDCInvokeOrdered"); + scheduler.chooseThread(10).schedule(() -> log.info("foobar"), 0, TimeUnit.DAYS).get(); + assertThat(capturedEvents, + hasItem(mdcFormat("testMDCInvokeOrdered", "foobar"))); + } finally { + scheduler.shutdown(); + } + } + +} diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MdcContextTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MdcContextTest.java new file mode 100644 index 00000000000..2a7a15ca253 --- /dev/null +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MdcContextTest.java @@ -0,0 +1,252 @@ +/* + * + * 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.bookkeeper.client; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; +import static org.mockito.AdditionalAnswers.answerVoid; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.spy; + +import java.io.File; +import java.util.Queue; +import java.util.UUID; +import java.util.concurrent.ConcurrentLinkedQueue; +import lombok.extern.slf4j.Slf4j; +import org.apache.bookkeeper.bookie.Bookie; +import org.apache.bookkeeper.bookie.BookieImpl; +import org.apache.bookkeeper.bookie.InterleavedLedgerStorage; +import org.apache.bookkeeper.bookie.LedgerDirsManager; +import org.apache.bookkeeper.conf.ClientConfiguration; +import org.apache.bookkeeper.test.BookKeeperClusterTestCase; +import org.apache.logging.log4j.ThreadContext; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.NullAppender; +import org.hamcrest.CoreMatchers; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + + +/** + * Test passing of MDC context. + */ +@SuppressWarnings("deprecation") +@Slf4j +public class MdcContextTest extends BookKeeperClusterTestCase { + public static final String MDC_REQUEST_ID = "request_id"; + + final byte[] entry = "Test Entry".getBytes(); + + BookKeeper bkc; + LedgerHandle lh; + + private NullAppender mockAppender; + private Queue capturedEvents; + + public MdcContextTest() { + super(3); + baseConf.setNumAddWorkerThreads(0); + baseConf.setNumReadWorkerThreads(0); + baseConf.setPreserveMdcForTaskExecution(true); + baseConf.setReadOnlyModeEnabled(true); + + // for read-only bookie + baseConf.setLedgerStorageClass(InterleavedLedgerStorage.class.getName()); + baseConf.setEntryLogFilePreAllocationEnabled(false); + baseConf.setMinUsableSizeForEntryLogCreation(Long.MAX_VALUE); + } + + + public static String mdcFormat(Object mdc, String message) { + return mdc == null + ? "[" + MDC_REQUEST_ID + ":] - " + message + : "[" + MDC_REQUEST_ID + ":" + mdc + + "] - " + message; + } + + public void assertLogWithMdc(String mdc, String msgSubstring) { + assertThat(capturedEvents, + hasItem(CoreMatchers.allOf( + containsString("[" + MDC_REQUEST_ID + ":" + mdc + "] - "), + containsString(msgSubstring) + ))); + } + + @Before + public void setUp() throws Exception { + super.setUp(); + ClientConfiguration conf = new ClientConfiguration(); + conf.setReadTimeout(360) + .setMetadataServiceUri(zkUtil.getMetadataServiceUri()) + .setPreserveMdcForTaskExecution(true); + + ThreadContext.clearMap(); + bkc = new BookKeeper(conf); + + ThreadContext.put(MDC_REQUEST_ID, "ledger_create"); + log.info("creating ledger"); + lh = bkc.createLedgerAdv(3, 3, 3, BookKeeper.DigestType.CRC32, new byte[] {}); + ThreadContext.clearMap(); + + LoggerContext lc = (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); + mockAppender = spy(NullAppender.createAppender(UUID.randomUUID().toString())); + mockAppender.start(); + lc.getConfiguration().addAppender(mockAppender); + lc.getRootLogger().addAppender(lc.getConfiguration().getAppender(mockAppender.getName())); + lc.getConfiguration().getRootLogger().setLevel(org.apache.logging.log4j.Level.INFO); + lc.updateLoggers(); + + capturedEvents = new ConcurrentLinkedQueue<>(); + + doAnswer(answerVoid((LogEvent event) -> capturedEvents.add( + mdcFormat(event.getContextData().getValue(MDC_REQUEST_ID), event.getMessage().getFormattedMessage()) + ))).when(mockAppender).append(any()); + } + + @After + public void tearDown() throws Exception { + lh.close(); + bkc.close(); + LoggerContext lc = (LoggerContext) org.apache.logging.log4j.LogManager.getContext(false); + lc.getRootLogger().removeAppender(lc.getConfiguration().getAppender(mockAppender.getName())); + lc.updateLoggers(); + capturedEvents = null; + ThreadContext.clearMap(); + super.tearDown(); + } + + @Test + public void testLedgerCreateFails() throws Exception { + ThreadContext.put(MDC_REQUEST_ID, "ledger_create_fail"); + try { + bkc.createLedgerAdv(99, 3, 2, BookKeeper.DigestType.CRC32, new byte[]{}); + Assert.fail("should not get here"); + } catch (BKException bke) { + // expected + } + assertLogWithMdc("ledger_create_fail", "Not enough bookies to create ledger"); + } + + @Test + public void testSimpleAdd() throws Exception { + ThreadContext.put(MDC_REQUEST_ID, "ledger_add_entry"); + lh.addEntry(0, entry); + + // client msg + assertLogWithMdc("ledger_add_entry", "Successfully connected to bookie"); + // bookie msg + assertLogWithMdc("ledger_add_entry", "Created new entry log file"); + } + + @Test + public void testAddWithEnsembleChange() throws Exception { + lh.addEntry(0, entry); + startNewBookie(); + killBookie(0); + + ThreadContext.put(MDC_REQUEST_ID, "ledger_add_entry"); + lh.addEntry(1, entry); + assertLogWithMdc("ledger_add_entry", "Could not connect to bookie"); + assertLogWithMdc("ledger_add_entry", "Failed to write entry"); + //commented out until we figure out a way to preserve MDC through a call out + //to another thread pool + //assertLogWithMdc("ledger_add_entry", "New Ensemble"); + } + + @Test + public void testAddFailsWithReadOnlyBookie() throws Exception { + for (int i = 0; i < 3; ++i) { + Bookie bookie = serverByIndex(i).getBookie(); + File[] ledgerDirs = confByIndex(i).getLedgerDirs(); + LedgerDirsManager ledgerDirsManager = ((BookieImpl) bookie).getLedgerDirsManager(); + ledgerDirsManager.addToFilledDirs(new File(ledgerDirs[0], "current")); + } + + ThreadContext.put(MDC_REQUEST_ID, "ledger_add_entry"); + try { + lh.addEntry(0, entry); + Assert.fail("should not get here"); + } catch (BKException bke) { + // expected, pass + } + + assertLogWithMdc("ledger_add_entry", "No writable ledger dirs below diskUsageThreshold"); + assertLogWithMdc("ledger_add_entry", "All ledger directories are non writable and no reserved space"); + assertLogWithMdc("ledger_add_entry", "Error writing entry:0 to ledger:0"); + assertLogWithMdc("ledger_add_entry", "Add for failed on bookie"); + assertLogWithMdc("ledger_add_entry", "Failed to find 1 bookies"); + assertLogWithMdc("ledger_add_entry", "Closing ledger 0 due to NotEnoughBookiesException"); + } + + @Test + public void testAddFailsDuplicateEntry() throws Exception { + lh.addEntry(0, entry); + + ThreadContext.put(MDC_REQUEST_ID, "ledger_add_duplicate_entry"); + try { + lh.addEntry(0, entry); + Assert.fail("should not get here"); + } catch (BKException bke) { + // expected, pass + } + + assertLogWithMdc("ledger_add_duplicate_entry", "Trying to re-add duplicate entryid:0"); + assertLogWithMdc("ledger_add_duplicate_entry", "Write of ledger entry to quorum failed"); + } + + @Test + public void testReadEntryBeyondLac() throws Exception { + ThreadContext.put(MDC_REQUEST_ID, "ledger_read_entry"); + + try { + lh.readEntries(100, 100); + fail("should not get here"); + } catch (BKException.BKReadException e) { + // pass + } + assertLogWithMdc("ledger_read_entry", "ReadEntries exception on ledgerId:0 firstEntry:100 lastEntry:100"); + } + + @Test + public void testReadFromDeletedLedger() throws Exception { + lh.addEntry(0, entry); + lh.close(); + bkc.deleteLedger(lh.ledgerId); + + ThreadContext.put(MDC_REQUEST_ID, "ledger_read_entry"); + + try { + lh.readEntries(100, 100); + fail("should not get here"); + } catch (BKException.BKReadException e) { + // pass + } + assertLogWithMdc("ledger_read_entry", "ReadEntries exception on ledgerId:0 firstEntry:100 lastEntry:100"); + } + +} From 697df39b733141656ae11dc4df3ed33d19ab7904 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 24 Apr 2026 21:15:37 -0700 Subject: [PATCH 8/8] Bump slog dependency to 0.9.8 Pulls in the MDC propagation fix from merlimat/slog#6, which makes log4j2 ThreadContext entries visible on slog events emitted via the Log4j2Logger backend (so %X{key} layouts and appenders that read event.getContextData().getValue(key) see the caller's MDC). --- bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt | 4 ++-- bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt | 4 ++-- bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt | 4 ++-- pom.xml | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt index 868168e9335..f66fb0c91a7 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt @@ -216,7 +216,7 @@ Apache Software License, Version 2. - lib/commons-codec-commons-codec-1.18.0.jar [6] - lib/commons-io-commons-io-2.19.0.jar [8] - lib/commons-logging-commons-logging-1.3.5.jar [10] -- lib/io.github.merlimat.slog-slog-0.9.7.jar [64] +- lib/io.github.merlimat.slog-slog-0.9.8.jar [64] - lib/io.netty-netty-buffer-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-base-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-compression-4.2.12.Final.jar [11] @@ -423,7 +423,7 @@ Apache Software License, Version 2. [61] Source available at https://github.com/apache/commons-text/tree/rel/commons-text-1.13.1 [62] Source available at https://github.com/apache/commons-beanutils/tree/rel/commons-beanutils-1.11.0 [63] Source available at https://github.com/googleapis/sdk-platform-java/tree/v2.53.0/api-common-java -[64] Source available at https://github.com/merlimat/slog/tree/v0.9.7 +[64] Source available at https://github.com/merlimat/slog/tree/v0.9.8 ------------------------------------------------------------------------------------ lib/io.netty-netty-codec-base-4.2.12.Final.jar bundles some 3rd party dependencies diff --git a/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt index dd0d123b0ae..8b39fb9a5dd 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt @@ -216,7 +216,7 @@ Apache Software License, Version 2. - lib/commons-codec-commons-codec-1.18.0.jar [6] - lib/commons-io-commons-io-2.19.0.jar [8] - lib/commons-logging-commons-logging-1.3.5.jar [10] -- lib/io.github.merlimat.slog-slog-0.9.7.jar [59] +- lib/io.github.merlimat.slog-slog-0.9.8.jar [59] - lib/io.netty-netty-buffer-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-base-4.2.12.Final.jar [11] - lib/io.netty-netty-common-4.2.12.Final.jar [11] @@ -356,7 +356,7 @@ Apache Software License, Version 2. [56] Source available at https://github.com/apache/commons-text/tree/rel/commons-text-1.13.1 [57] Source available at https://github.com/apache/commons-beanutils/tree/rel/commons-beanutils-1.11.0 [58] Source available at https://github.com/googleapis/sdk-platform-java/tree/v2.53.0/api-common-java -[59] Source available at https://github.com/merlimat/slog/tree/v0.9.7 +[59] Source available at https://github.com/merlimat/slog/tree/v0.9.8 ------------------------------------------------------------------------------------ lib/io.netty-netty-codec-base-4.2.12.Final.jar bundles some 3rd party dependencies diff --git a/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt index b2e9d676dc0..362493a0515 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt @@ -216,7 +216,7 @@ Apache Software License, Version 2. - lib/commons-codec-commons-codec-1.18.0.jar [6] - lib/commons-io-commons-io-2.19.0.jar [8] - lib/commons-logging-commons-logging-1.3.5.jar [10] -- lib/io.github.merlimat.slog-slog-0.9.7.jar [63] +- lib/io.github.merlimat.slog-slog-0.9.8.jar [63] - lib/io.netty-netty-buffer-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-base-4.2.12.Final.jar [11] - lib/io.netty-netty-codec-compression-4.2.12.Final.jar [11] @@ -418,7 +418,7 @@ Apache Software License, Version 2. [60] Source available at https://github.com/apache/commons-text/tree/rel/commons-text-1.13.1 [61] Source available at https://github.com/apache/commons-beanutils/tree/rel/commons-beanutils-1.11.0 [62] Source available at https://github.com/googleapis/sdk-platform-java/tree/v2.53.0/api-common-java -[63] Source available at https://github.com/merlimat/slog/tree/v0.9.7 +[63] Source available at https://github.com/merlimat/slog/tree/v0.9.8 ------------------------------------------------------------------------------------ lib/io.netty-netty-codec-base-4.2.12.Final.jar bundles some 3rd party dependencies diff --git a/pom.xml b/pom.xml index 3de84676b52..d18f145ed76 100644 --- a/pom.xml +++ b/pom.xml @@ -192,7 +192,7 @@ 9.9.3 3.3.0 2.0.12 - 0.9.7 + 0.9.8 2.0 4.6.0 2.43.0