Tidy up lifecycle, query, and ingestion logging.

core/src/main/java/org/apache/druid/common/config/ConfigManager.java

@@ -124,7 +124,7 @@ public <T> AtomicReference<T> watchConfig(final String key, final ConfigSerde<T>
     ConfigHolder<T> holder = watchedConfigs.get(key);
     if (holder == null) {
       try {
-        log.info("Creating watch for key[%s]", key);
+        log.debug("Creating watch for key[%s]", key);
 
         holder = exec.submit(
             new Callable<ConfigHolder<T>>()

core/src/main/java/org/apache/druid/common/guava/ThreadRenamingCallable.java

@@ -21,8 +21,6 @@
 
 import java.util.concurrent.Callable;
 
-/**
- */
 public abstract class ThreadRenamingCallable<T> implements Callable<T>
 {
   private final String name;
@@ -48,5 +46,5 @@ public final T call() throws Exception
     }
   }
 
-  public abstract T doCall() throws Exception;
+  public abstract T doCall();
 }

core/src/main/java/org/apache/druid/guice/Jerseys.java

@@ -26,17 +26,15 @@
 import org.apache.druid.guice.annotations.PublicApi;
 import org.apache.druid.java.util.common.logger.Logger;
 
-/**
- */
 @PublicApi
 public class Jerseys
 {
   private static final Logger LOG = new Logger(Jerseys.class);
 
   public static void addResource(Binder binder, Class<?> resourceClazz)
   {
-    LOG.info("Adding Jersey resource: " + resourceClazz.getName());
-    Multibinder.newSetBinder(binder, new TypeLiteral<Class<?>>(){}, JSR311Resource.class)
+    LOG.debug("Adding Jersey resource: " + resourceClazz.getName());
+    Multibinder.newSetBinder(binder, new TypeLiteral<Class<?>>() {}, JSR311Resource.class)
                .addBinding()
                .toInstance(resourceClazz);
   }

core/src/main/java/org/apache/druid/guice/JsonConfigurator.java

@@ -187,7 +187,7 @@ public Message apply(String input)
       );
     }
 
-    log.info("Loaded class[%s] from props[%s] as [%s]", clazz, propertyBase, config);
+    log.debug("Loaded class[%s] from props[%s] as [%s]", clazz, propertyBase, config);
 
     return config;
   }

core/src/main/java/org/apache/druid/java/util/common/io/smoosh/FileSmoosher.java

@@ -369,7 +369,8 @@ public void close() throws IOException
 
     File metaFile = metaFile(baseDir);
 
-    try (Writer out = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(metaFile), StandardCharsets.UTF_8))) {
+    try (Writer out =
+             new BufferedWriter(new OutputStreamWriter(new FileOutputStream(metaFile), StandardCharsets.UTF_8))) {
       out.write(StringUtils.format("v1,%d,%d", maxChunkSize, outFiles.size()));
       out.write("\n");
 
@@ -469,7 +470,11 @@ public boolean isOpen()
     public void close() throws IOException
     {
       closer.close();
-      FileSmoosher.LOG.info("Created smoosh file [%s] of size [%s] bytes.", outFile.getAbsolutePath(), outFile.length());
+      FileSmoosher.LOG.debug(
+          "Created smoosh file [%s] of size [%s] bytes.",
+          outFile.getAbsolutePath(),
+          outFile.length()
+      );
     }
   }
 }

core/src/main/java/org/apache/druid/java/util/common/lifecycle/Lifecycle.java

@@ -438,7 +438,7 @@ public void start() throws Exception
           }
         }
         if (doStart) {
-          log.info("Invoking start method[%s] on object[%s].", method, o);
+          log.debug("Invoking start method[%s] on object[%s].", method, o);
           method.invoke(o);
         }
       }
@@ -456,7 +456,7 @@ public void stop()
           }
         }
         if (doStop) {
-          log.info("Invoking stop method[%s] on object[%s].", method, o);
+          log.debug("Invoking stop method[%s] on object[%s].", method, o);
           try {
             method.invoke(o);
           }

core/src/main/java/org/apache/druid/java/util/common/logger/Logger.java

@@ -22,20 +22,32 @@
 import org.apache.druid.java.util.common.StringUtils;
 import org.slf4j.LoggerFactory;
 
-/**
- */
+import java.util.function.BiConsumer;
+
 public class Logger
 {
   private final org.slf4j.Logger log;
+  private final boolean stackTraces;
 
   public Logger(String name)
   {
-    log = LoggerFactory.getLogger(name);
+    this(LoggerFactory.getLogger(name), true);
   }
 
   public Logger(Class clazz)
   {
-    log = LoggerFactory.getLogger(clazz);
+    this(LoggerFactory.getLogger(clazz), true);
+  }
+
+  protected Logger(org.slf4j.Logger log, boolean stackTraces)
+  {
+    this.log = log;
+    this.stackTraces = stackTraces;
+  }
+
+  protected org.slf4j.Logger getSlf4jLogger()
+  {
+    return log;
   }
 
   @Override
@@ -44,6 +56,15 @@ public String toString()
     return StringUtils.format("Logger{name=[%s], class[%s]}", log.getName(), log.getClass());
   }
 
+  /**
+   * Create a copy of this Logger that does not log exception stack traces, unless the log level is DEBUG or lower.
+   * Useful for writing code like: {@code log.noStackTrace().warn(e, "Something happened.");}
+   */
+  public Logger noStackTrace()
+  {
+    return new Logger(log, false);
+  }
+
   public void trace(String message, Object... formatArgs)
   {
     if (log.isTraceEnabled()) {
@@ -61,7 +82,7 @@ public void debug(String message, Object... formatArgs)
   public void debug(Throwable t, String message, Object... formatArgs)
   {
     if (log.isDebugEnabled()) {
-      log.debug(StringUtils.nonStrictFormat(message, formatArgs), t);
+      logException(log::debug, t, StringUtils.nonStrictFormat(message, formatArgs));
     }
   }
 
@@ -75,7 +96,7 @@ public void info(String message, Object... formatArgs)
   public void info(Throwable t, String message, Object... formatArgs)
   {
     if (log.isInfoEnabled()) {
-      log.info(StringUtils.nonStrictFormat(message, formatArgs), t);
+      logException(log::info, t, StringUtils.nonStrictFormat(message, formatArgs));
     }
   }
 
@@ -88,7 +109,7 @@ public void info(Throwable t, String message, Object... formatArgs)
   @Deprecated
   public void warn(String message, Throwable t)
   {
-    log.warn(message, t);
+    warn(t, message);
   }
 
   public void warn(String message, Object... formatArgs)
@@ -98,7 +119,7 @@ public void warn(String message, Object... formatArgs)
 
   public void warn(Throwable t, String message, Object... formatArgs)
   {
-    log.warn(StringUtils.nonStrictFormat(message, formatArgs), t);
+    logException(log::warn, t, StringUtils.nonStrictFormat(message, formatArgs));
   }
 
   public void error(String message, Object... formatArgs)
@@ -115,12 +136,12 @@ public void error(String message, Object... formatArgs)
   @Deprecated
   public void error(String message, Throwable t)
   {
-    log.error(message, t);
+    error(t, message);
   }
 
   public void error(Throwable t, String message, Object... formatArgs)
   {
-    log.error(StringUtils.nonStrictFormat(message, formatArgs), t);
+    logException(log::error, t, StringUtils.nonStrictFormat(message, formatArgs));
   }
 
   public void assertionError(String message, Object... formatArgs)
@@ -152,4 +173,17 @@ public boolean isInfoEnabled()
   {
     return log.isInfoEnabled();
   }
+
+  private void logException(BiConsumer<String, Throwable> fn, Throwable t, String message)
+  {
+    if (stackTraces || log.isDebugEnabled()) {
+      fn.accept(message, t);
+    } else {
+      if (message.isEmpty()) {
+        fn.accept(t.toString(), null);
+      } else {
+        fn.accept(StringUtils.nonStrictFormat("%s (%s)", message, t.toString()), null);
+      }
+    }
+  }
 }

core/src/main/java/org/apache/druid/java/util/emitter/EmittingLogger.java

@@ -53,10 +53,21 @@ public static void registerEmitter(ServiceEmitter emitter)
   public EmittingLogger(Class clazz)
   {
     super(clazz);
-
     this.className = clazz.getName();
   }
 
+  private EmittingLogger(org.slf4j.Logger log, boolean stackTraces)
+  {
+    super(log, stackTraces);
+    this.className = log.getName();
+  }
+
+  @Override
+  public EmittingLogger noStackTrace()
+  {
+    return new EmittingLogger(getSlf4jLogger(), false);
+  }
+
   public AlertBuilder makeAlert(String message, Object... objects)
   {
     return makeAlert(null, message, objects);

core/src/main/java/org/apache/druid/java/util/emitter/core/NoopEmitter.java

@@ -46,4 +46,10 @@ public void close()
   {
     // Do nothing
   }
+
+  @Override
+  public String toString()
+  {
+    return "NoopEmitter{}";
+  }
 }

core/src/main/java/org/apache/druid/java/util/http/client/pool/ResourcePool.java

@@ -248,22 +248,22 @@ void giveBack(V object)
         if (resourceHolderList.size() >= maxSize) {
           if (holderListContains(object)) {
             log.warn(
+                new Exception("Exception for stacktrace"),
                 StringUtils.format(
                     "Returning object[%s] at key[%s] that has already been returned!? Skipping",
                     object,
                     key
-                ),
-                new Exception("Exception for stacktrace")
+                )
             );
           } else {
             log.warn(
+                new Exception("Exception for stacktrace"),
                 StringUtils.format(
                     "Returning object[%s] at key[%s] even though we already have all that we can hold[%s]!? Skipping",
                     object,
                     key,
                     resourceHolderList
-                ),
-                new Exception("Exception for stacktrace")
+                )
             );
           }
           return;

core/src/main/java/org/apache/druid/segment/SegmentUtils.java

@@ -34,8 +34,10 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
 
 /**
  * Utility methods useful for implementing deep storage extensions.
@@ -75,6 +77,19 @@ public static int getVersionFromDir(File inDir) throws IOException
     throw new IOE("Invalid segment dir [%s]. Can't find either of version.bin or index.drd.", inDir);
   }
 
+  /**
+   * Returns a String with identifiers of "segments" comma-separated. Useful for log messages. Not useful for anything
+   * else, because this doesn't take special effort to escape commas that occur in identifiers (not common, but could
+   * potentially occur in a datasource name).
+   */
+  public static String commaSeparateIdentifiers(final Collection<DataSegment> segments)
+  {
+    return segments
+        .stream()
+        .map(segment -> segment.getId().toString())
+        .collect(Collectors.joining(", "));
+  }
+
   private SegmentUtils()
   {
   }

core/src/main/java/org/apache/druid/timeline/DataSegment.java

@@ -21,6 +21,7 @@
 
 import com.fasterxml.jackson.annotation.JacksonInject;
 import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
@@ -70,8 +71,13 @@ public static class PruneSpecsHolder
     @VisibleForTesting
     public static final PruneSpecsHolder DEFAULT = new PruneSpecsHolder();
 
-    @Inject(optional = true) @PruneLoadSpec boolean pruneLoadSpec = false;
-    @Inject(optional = true) @PruneLastCompactionState boolean pruneLastCompactionState = false;
+    @Inject(optional = true)
+    @PruneLoadSpec
+    boolean pruneLoadSpec = false;
+
+    @Inject(optional = true)
+    @PruneLastCompactionState
+    boolean pruneLastCompactionState = false;
   }
 
   private static final Interner<String> STRING_INTERNER = Interners.newWeakInterner();
@@ -312,6 +318,7 @@ public ShardSpec getShardSpec()
 
   @Nullable
   @JsonProperty
+  @JsonInclude(JsonInclude.Include.NON_NULL)
   public CompactionState getLastCompactionState()
   {
     return lastCompactionState;

core/src/main/java/org/apache/druid/utils/CompressionUtils.java

@@ -139,7 +139,7 @@ public static long zip(File directory, OutputStream out) throws IOException
 
     long totalSize = 0;
     for (File file : directory.listFiles()) {
-      log.info("Adding file[%s] with size[%,d].  Total size so far[%,d]", file, file.length(), totalSize);
+      log.debug("Adding file[%s] with size[%,d].  Total size so far[%,d]", file, file.length(), totalSize);
       if (file.length() > Integer.MAX_VALUE) {
         zipOut.finish();
         throw new IOE("file[%s] too large [%,d]", file, file.length());

core/src/test/java/org/apache/druid/java/util/common/logger/LoggerTest.java

@@ -23,23 +23,31 @@
 
 public class LoggerTest
 {
+  private final Logger log = new Logger(LoggerTest.class);
+
   @SuppressWarnings("MalformedFormatString")
   @Test
   public void testLogWithCrazyMessages()
   {
     final String message = "this % might %d kill %*.s the %s parser";
-    final Logger log = new Logger(LoggerTest.class);
     log.warn(message);
   }
 
   @SuppressWarnings("MalformedFormatString")
   @Test
   public void testLegacyLogging()
   {
-    final Logger log = new Logger(LoggerTest.class);
     final Throwable throwable = new Throwable();
     // These should show up in an IDE as deprecated, but shouldn't actually fail.
     log.error("foo", throwable);
     log.warn("foo", throwable);
   }
+
+  @Test
+  public void testErrorExceptions()
+  {
+    log.noStackTrace().error(new RuntimeException("beep"), "Feel the hatred of %d years", 10000);
+    log.noStackTrace().error(new RuntimeException("beep"), "");
+    log.error(new RuntimeException("beep"), "An exception");
+  }
 }

core/src/test/java/org/apache/druid/timeline/SegmentWithOvershadowedStatusTest.java

@@ -81,7 +81,7 @@ public void testUnwrappedSegmentWithOvershadowedStatusDeserialization() throws E
         JacksonUtils.TYPE_REFERENCE_MAP_STRING_OBJECT
     );
 
-    Assert.assertEquals(12, objectMap.size());
+    Assert.assertEquals(11, objectMap.size());
     Assert.assertEquals("something", objectMap.get("dataSource"));
     Assert.assertEquals(interval.toString(), objectMap.get("interval"));
     Assert.assertEquals("1", objectMap.get("version"));