Log only the bookie settings that differ from the built-in defaults

bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java

@@ -27,10 +27,16 @@
 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 lombok.CustomLog;
 import org.apache.bookkeeper.bookie.Bookie;
@@ -39,6 +45,7 @@
 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;
@@ -220,28 +227,93 @@ private void validateUser(ServerConfiguration conf) throws BookieException {
     }
 
     /**
-     * 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. Keys that no getter reads (settings of other components, or unrelated entries loaded
+     * from a shared properties file) are not reported.
      */
-    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, 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;

bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/BookieServerOverriddenConfigTest.java

@@ -0,0 +1,98 @@
+/*
+ *
+ * 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.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 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"));
+    }
+}