From 6f0833e0fb52f0788dd54559ec51f3fa6cd5da75 Mon Sep 17 00:00:00 2001
From: Gian Merlino <gianmerlino@gmail.com>
Date: Tue, 11 Mar 2025 14:09:42 -0700
Subject: [PATCH] Log query stack traces for DEVELOPER and OPERATOR personas.

Currently, query stack traces are logged only when "debug: true" is set
in the query context. This patch additionally logs stack traces targeted
at the DEVELOPER or OPERATOR personas, because for these personas, stack
traces are useful more often than not.

We continue to omit stack traces by default for USER and ADMIN, because
these personas are meant to interact with the API, not with code or logs.
Skipping stack traces minimizes clutter in the logs.
---
 .../apache/druid/server/QueryLifecycle.java   | 23 ++++++++++++++++++-
 .../apache/druid/sql/http/SqlResource.java    |  3 ++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/server/src/main/java/org/apache/druid/server/QueryLifecycle.java b/server/src/main/java/org/apache/druid/server/QueryLifecycle.java
index 6ffc8aff0bba..f5c167360c6a 100644
--- a/server/src/main/java/org/apache/druid/server/QueryLifecycle.java
+++ b/server/src/main/java/org/apache/druid/server/QueryLifecycle.java
@@ -423,7 +423,7 @@ public void emitLogsAndMetrics(
 
       if (e != null) {
         statsMap.put("exception", e.toString());
-        if (baseQuery.context().isDebug() || e.getMessage() == null) {
+        if (shouldLogStackTrace(e, baseQuery.context())) {
           log.warn(e, "Exception while processing queryId [%s]", baseQuery.getId());
         } else {
           log.noStackTrace().warn(e, "Exception while processing queryId [%s]", baseQuery.getId());
@@ -518,4 +518,25 @@ enum State
     DONE
   }
 
+  /**
+   * Returns whether stack traces should be logged for a particular exception thrown with a particular query context.
+   * Stack traces are logged if {@link QueryContext#isDebug()}, or if the {@link DruidException.Persona} is
+   * {@link DruidException.Persona#DEVELOPER} or {@link DruidException.Persona#OPERATOR}. The idea is that other
+   * personas are meant to interact with the API, not with code or logs, so logging stack traces by default adds
+   * clutter that is not very helpful.
+   *
+   * @param e            exception
+   * @param queryContext query context
+   */
+  public static boolean shouldLogStackTrace(final Throwable e, final QueryContext queryContext)
+  {
+    if (queryContext.isDebug() || e.getMessage() == null) {
+      return true;
+    } else if (e instanceof DruidException) {
+      final DruidException.Persona persona = ((DruidException) e).getTargetPersona();
+      return persona == DruidException.Persona.OPERATOR || persona == DruidException.Persona.DEVELOPER;
+    } else {
+      return false;
+    }
+  }
 }
diff --git a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
index b53b49f0fcaa..c966b4ec8bd2 100644
--- a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
+++ b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
@@ -28,6 +28,7 @@
 import org.apache.druid.java.util.common.StringUtils;
 import org.apache.druid.java.util.common.logger.Logger;
 import org.apache.druid.server.DruidNode;
+import org.apache.druid.server.QueryLifecycle;
 import org.apache.druid.server.QueryResource;
 import org.apache.druid.server.QueryResponse;
 import org.apache.druid.server.QueryResultPusher;
@@ -304,7 +305,7 @@ public void recordSuccess(long numBytes)
         @Override
         public void recordFailure(Exception e)
         {
-          if (sqlQuery.queryContext().isDebug()) {
+          if (QueryLifecycle.shouldLogStackTrace(e, sqlQuery.queryContext())) {
             log.warn(e, "Exception while processing sqlQueryId[%s]", sqlQueryId);
           } else {
             log.noStackTrace().warn(e, "Exception while processing sqlQueryId[%s]", sqlQueryId);