This is an automated email from the ASF dual-hosted git repository.
merlimat pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 908e88f0e9 Log only the bookie settings that differ from the built-in
defaults (#4892)
908e88f0e9 is described below
commit 908e88f0e9f79a23f11d3a4d73a18148e59226e4
Author: Matteo Merli <[email protected]>
AuthorDate: Sat Oct 17 05:34:46 2026 -0700
Log only the bookie settings that differ from the built-in defaults (#4892)
---
.../org/apache/bookkeeper/proto/BookieServer.java | 108 +++++++++++++++++---
.../proto/BookieServerOverriddenConfigTest.java | 110 +++++++++++++++++++++
2 files changed, 204 insertions(+), 14 deletions(-)
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
index 2fa5e90e0b..b835798eef 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
@@ -27,11 +27,18 @@ import com.google.common.annotations.VisibleForTesting;
import io.netty.buffer.ByteBufAllocator;
import java.io.IOException;
import java.lang.Thread.UncaughtExceptionHandler;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
import java.net.UnknownHostException;
import java.util.Arrays;
-import java.util.Iterator;
+import java.util.HashSet;
+import java.util.List;
import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
import java.util.TreeMap;
+import java.util.regex.Pattern;
import lombok.CustomLog;
import org.apache.bookkeeper.bookie.Bookie;
import org.apache.bookkeeper.bookie.BookieCriticalThread;
@@ -39,6 +46,7 @@ import org.apache.bookkeeper.bookie.BookieException;
import org.apache.bookkeeper.bookie.BookieImpl;
import org.apache.bookkeeper.bookie.ExitCode;
import org.apache.bookkeeper.bookie.UncleanShutdownDetection;
+import org.apache.bookkeeper.conf.AbstractConfiguration;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.net.BookieId;
import org.apache.bookkeeper.net.BookieSocketAddress;
@@ -76,6 +84,9 @@ public class BookieServer {
// Exception handler
private volatile UncaughtExceptionHandler uncaughtExceptionHandler = null;
+ // configuration keys whose values must not be logged
+ private static final Pattern SENSITIVE_KEY =
Pattern.compile("password|secret|token", Pattern.CASE_INSENSITIVE);
+
public BookieServer(ServerConfiguration conf,
Bookie bookie,
StatsLogger statsLogger,
@@ -220,28 +231,97 @@ public class BookieServer {
}
/**
- * Returns the configuration entries that differ from the {@link
ServerConfiguration}
- * defaults — i.e. values explicitly set by the user (via config file or
programmatically)
- * whose value is not the same as the built-in default.
+ * Returns the entries of {@code conf} whose value differs from the
built-in default, keyed by
+ * configuration property name.
+ *
+ * <p>The defaults are not stored in the configuration but encoded in the
{@link ServerConfiguration}
+ * getters, so every getter is invoked on both {@code conf} and an empty
configuration, recording the
+ * keys it reads. When the two results differ, the recorded keys that are
explicitly set in {@code conf}
+ * are reported, with the value masked when the key looks like a secret.
Keys that no getter reads
+ * (settings of other components, or unrelated entries loaded from a
shared properties file) are not
+ * reported.
+ *
+ * <p>Since every public {@code get*}/{@code is*} getter of {@link
ServerConfiguration} is invoked
+ * reflectively here on each bookie start, those getters must remain
side-effect free and cheap.
*/
- private static Map<String, Object> overriddenConfig(ServerConfiguration
conf) {
- ServerConfiguration defaults = new ServerConfiguration();
+ @VisibleForTesting
+ static Map<String, Object> overriddenConfig(ServerConfiguration conf) {
+ KeyRecordingConfiguration actual = new KeyRecordingConfiguration(conf);
+ KeyRecordingConfiguration defaults = new KeyRecordingConfiguration(new
ServerConfiguration());
Map<String, Object> overrides = new TreeMap<>();
- Iterator<String> keys = conf.getInMemoryConfiguration().getKeys();
- while (keys.hasNext()) {
- String key = keys.next();
- Object value = conf.getProperty(key);
- if (value == null) {
+ for (Method getter : ServerConfiguration.class.getMethods()) {
+ if (!isGetter(getter)) {
continue;
}
- Object defaultValue = defaults.getProperty(key);
- if (defaultValue == null ||
!value.toString().equals(defaultValue.toString())) {
- overrides.put(key, value);
+ actual.keys.clear();
+ defaults.keys.clear();
+ if (Objects.deepEquals(invoke(getter, actual), invoke(getter,
defaults))) {
+ continue;
+ }
+ Set<String> keys = new HashSet<>(actual.keys);
+ keys.addAll(defaults.keys);
+ for (String key : keys) {
+ if (conf.containsKey(key)) {
+ overrides.put(key, SENSITIVE_KEY.matcher(key).find() ?
"******" : conf.getProperty(key));
+ }
}
}
return overrides;
}
+ private static boolean isGetter(Method method) {
+ return method.getParameterCount() == 0
+ && !Modifier.isStatic(method.getModifiers())
+ &&
AbstractConfiguration.class.isAssignableFrom(method.getDeclaringClass())
+ && (method.getName().startsWith("get") ||
method.getName().startsWith("is"));
+ }
+
+ private static Object invoke(Method getter, ServerConfiguration conf) {
+ try {
+ return getter.invoke(conf);
+ } catch (InvocationTargetException e) {
+ // a getter rejecting the configured value still means the value
differs from the default
+ return e.getCause().getClass();
+ } catch (IllegalAccessException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ /**
+ * A {@link ServerConfiguration} that reads its properties from another
configuration and records the
+ * keys that have been looked up.
+ *
+ * <p>Besides the two internal hooks, {@code getList} must be overridden
because
+ * {@code CompositeConfiguration} implements it (and {@code
getStringArray}) by reading its child
+ * configurations directly.
+ */
+ private static class KeyRecordingConfiguration extends ServerConfiguration
{
+ private final ServerConfiguration source;
+ private final Set<String> keys = new HashSet<>();
+
+ KeyRecordingConfiguration(ServerConfiguration source) {
+ this.source = source;
+ }
+
+ @Override
+ protected Object getPropertyInternal(String key) {
+ keys.add(key);
+ return source.getProperty(key);
+ }
+
+ @Override
+ protected boolean containsKeyInternal(String key) {
+ keys.add(key);
+ return source.containsKey(key);
+ }
+
+ @Override
+ public List<Object> getList(String key, List<?> defaultValue) {
+ keys.add(key);
+ return source.getList(key, defaultValue);
+ }
+ }
+
public boolean isRunning() {
return bookie.isRunning() && nettyServer.isRunning() && running;
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/BookieServerOverriddenConfigTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/BookieServerOverriddenConfigTest.java
new file mode 100644
index 0000000000..bb6864e1d6
--- /dev/null
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/BookieServerOverriddenConfigTest.java
@@ -0,0 +1,110 @@
+/*
+ *
+ * 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.proto;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.junit.Test;
+
+/**
+ * Unit test for {@link BookieServer#overriddenConfig(ServerConfiguration)}.
+ */
+public class BookieServerOverriddenConfigTest {
+
+ @Test
+ public void testEmptyConfiguration() {
+ assertTrue(BookieServer.overriddenConfig(new
ServerConfiguration()).isEmpty());
+ }
+
+ @Test
+ public void testValuesEqualToDefaultsAreNotReported() {
+ ServerConfiguration conf = new ServerConfiguration();
+ conf.setBookiePort(3181);
+ conf.setJournalSyncData(true);
+ conf.setGcWaitTime(600000);
+ conf.setJournalDirName("/tmp/bk-txn");
+
+ assertTrue(BookieServer.overriddenConfig(conf).isEmpty());
+ }
+
+ @Test
+ public void testOverriddenValuesAreReported() {
+ ServerConfiguration conf = new ServerConfiguration();
+ conf.setBookiePort(3181);
+ conf.setJournalSyncData(false);
+ conf.setGcWaitTime(300000);
+ conf.setLedgerDirNames(new String[] { "/data/ledgers1",
"/data/ledgers2" });
+
+ Map<String, Object> overrides = BookieServer.overriddenConfig(conf);
+
+ assertEquals(Set.of("journalSyncData", "gcWaitTime",
"ledgerDirectories"), overrides.keySet());
+ assertEquals("false",
String.valueOf(overrides.get("journalSyncData")));
+ assertEquals("300000", String.valueOf(overrides.get("gcWaitTime")));
+ assertEquals("[/data/ledgers1, /data/ledgers2]",
String.valueOf(overrides.get("ledgerDirectories")));
+ }
+
+ @Test
+ public void testOnlyKeysActuallySetAreReported() {
+ ServerConfiguration conf = new ServerConfiguration();
+ // getJournalDirNames() looks up journalDirectories first and falls
back to journalDirectory
+ conf.setProperty("journalDirectory", "/data/journal");
+
+ Map<String, Object> overrides = BookieServer.overriddenConfig(conf);
+
+ assertEquals(Set.of("journalDirectory"), overrides.keySet());
+ assertEquals("/data/journal", overrides.get("journalDirectory"));
+ }
+
+ @Test
+ public void testUnknownKeysAreNotReported() {
+ ServerConfiguration conf = new ServerConfiguration();
+ conf.setProperty("brokerServicePort", "6650");
+
+ assertTrue(BookieServer.overriddenConfig(conf).isEmpty());
+ }
+
+ @Test
+ public void testSensitiveValuesAreMasked() {
+ ServerConfiguration conf = new ServerConfiguration();
+ conf.setHttpServerKeyStorePassword("hunter2");
+
+ Map<String, Object> overrides = BookieServer.overriddenConfig(conf);
+
+ assertEquals("******", overrides.get("httpServerKeyStorePassword"));
+ assertFalse(overrides.toString().contains("hunter2"));
+ }
+
+ @Test
+ public void testValueRejectedByGetterIsReported() {
+ ServerConfiguration conf = new ServerConfiguration();
+ conf.setProperty("ledgerManagerFactoryClass", "does.not.Exist");
+
+ Map<String, Object> overrides = BookieServer.overriddenConfig(conf);
+
+ assertEquals("does.not.Exist",
overrides.get("ledgerManagerFactoryClass"));
+ }
+}
