SOLR-14566: Add request-ID to all distrib-search requests (#1574)

solr/CHANGES.txt

@@ -215,6 +215,10 @@ Improvements
 
 * SOLR-14539: Introducing {!bool excludeTags=...} for Query DSL. (Mikhail Khludnev)
 
+* SOLR-14566: Request ID's ('rid') are now added by default to distributed search requests, and can be used to correlate 
+  logs from the receiving coordinator node with those from downstream shard requests.  This can be disabled by providing a
+  disableRequestId=true request parameter. (Jason Gerlowski)
+
 Optimizations
 ---------------------
 * SOLR-8306: Do not collect expand documents when expand.rows=0 (Marshall Sanders, Amelia Henderson)

solr/core/src/java/org/apache/solr/handler/component/DebugComponent.java

@@ -25,15 +25,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
-import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.lucene.search.Query;
 import org.apache.solr.common.SolrDocumentList;
 import org.apache.solr.common.params.CommonParams;
-import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.search.DocList;
 import org.apache.solr.search.QueryParsing;
@@ -55,11 +52,6 @@ public class DebugComponent extends SearchComponent
 {
   public static final String COMPONENT_NAME = "debug";
   
-  /**
-   * A counter to ensure that no RID is equal, even if they fall in the same millisecond
-   */
-  private static final AtomicLong ridCounter = new AtomicLong();
-  
   /**
    * Map containing all the possible stages as key and
    * the corresponding readable purpose as value
@@ -148,29 +140,9 @@ public class DebugComponent extends SearchComponent
     }
   }
 
-
   private void doDebugTrack(ResponseBuilder rb) {
-    String rid = getRequestId(rb.req);
+    final String rid = rb.req.getParams().get(CommonParams.REQUEST_ID);
     rb.addDebug(rid, "track", CommonParams.REQUEST_ID);//to see it in the response
-    rb.rsp.addToLog(CommonParams.REQUEST_ID, rid); //to see it in the logs of the landing core
-    
-  }
-
-  public static String getRequestId(SolrQueryRequest req) {
-    String rid = req.getParams().get(CommonParams.REQUEST_ID);
-    if(rid == null || "".equals(rid)) {
-      rid = generateRid(req);
-      ModifiableSolrParams params = new ModifiableSolrParams(req.getParams());
-      params.add(CommonParams.REQUEST_ID, rid);//add rid to the request so that shards see it
-      req.setParams(params);
-    }
-    return rid;
-  }
-
-  @SuppressForbidden(reason = "Need currentTimeMillis, only used for naming")
-  private static String generateRid(SolrQueryRequest req) {
-    String hostName = req.getCore().getCoreContainer().getHostName();
-    return hostName + "-" + req.getCore().getName() + "-" + System.currentTimeMillis() + "-" + ridCounter.getAndIncrement();
   }
 
   @Override

solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java

@@ -24,7 +24,9 @@ import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicLong;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.lucene.index.ExitableDirectoryReader;
 import org.apache.lucene.search.TotalHits;
 import org.apache.solr.client.solrj.SolrServerException;
@@ -74,6 +76,11 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware,
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
+  /**
+   * A counter to ensure that no RID is equal, even if they fall in the same millisecond
+   */
+  private static final AtomicLong ridCounter = new AtomicLong();
+
   protected volatile List<SearchComponent> components;
   private ShardHandlerFactory shardHandlerFactory;
   private PluginInfo shfInfo;
@@ -326,7 +333,9 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware,
     }
 
     final ShardHandler shardHandler1 = getAndPrepShardHandler(req, rb); // creates a ShardHandler object only if it's needed
-    
+
+    tagRequestWithRequestId(rb);
+
     if (timer == null) {
       // non-debugging prepare phase
       for( SearchComponent c : components ) {
@@ -528,6 +537,42 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware,
     }
   }
 
+  private void tagRequestWithRequestId(ResponseBuilder rb) {
+    final boolean ridTaggingDisabled = rb.req.getParams().getBool(CommonParams.DISABLE_REQUEST_ID, false);
+    if (! ridTaggingDisabled) {
+      String rid = getOrGenerateRequestId(rb.req);
+      if (StringUtils.isBlank(rb.req.getParams().get(CommonParams.REQUEST_ID))) {
+        ModifiableSolrParams params = new ModifiableSolrParams(rb.req.getParams());
+        params.add(CommonParams.REQUEST_ID, rid);//add rid to the request so that shards see it
+        rb.req.setParams(params);
+      }
+      if (rb.isDistrib) {
+        rb.rsp.addToLog(CommonParams.REQUEST_ID, rid); //to see it in the logs of the landing core
+      }
+    }
+  }
+
+  /**
+   * Returns a String to use as an identifier for this request.
+   *
+   * If the provided {@link SolrQueryRequest} contains a non-blank {@link CommonParams#REQUEST_ID} param value this is
+   * used.  This is especially useful for users who deploy Solr as one component in a larger ecosystem, and want to use
+   * an external ID utilized by other components as well.  If no {@link CommonParams#REQUEST_ID} value is present, one
+   * is generated from scratch for the request.
+   * <p>
+   * Callers are responsible for storing the returned value in the {@link SolrQueryRequest} object if they want to
+   * ensure that ID generation is not redone on subsequent calls.
+   */
+  public static String getOrGenerateRequestId(SolrQueryRequest req) {
+    String rid = req.getParams().get(CommonParams.REQUEST_ID);
+    return StringUtils.isNotBlank(rid) ? rid : generateRid(req);
+  }
+
+  private static String generateRid(SolrQueryRequest req) {
+    String hostName = req.getCore().getCoreContainer().getHostName();
+    return hostName + "-" + ridCounter.getAndIncrement();
+  }
+
   //////////////////////// SolrInfoMBeans methods //////////////////////
 
   @Override

solr/core/src/test/org/apache/solr/handler/component/DebugComponentTest.java

@@ -15,11 +15,11 @@
  * limitations under the License.
  */
 package org.apache.solr.handler.component;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.regex.Pattern;
 
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.params.CommonParams;
@@ -36,6 +36,9 @@ import org.junit.Test;
  *
  **/
 public class DebugComponentTest extends SolrTestCaseJ4 {
+
+  private static final String ANY_RID = "ANY_RID";
+
   @BeforeClass
   public static void beforeClass() throws Exception {
     initCore("solrconfig.xml", "schema.xml");
@@ -205,6 +208,8 @@ public class DebugComponentTest extends SolrTestCaseJ4 {
       req = req("q", "test query", "distrib", "true");
       rb = new ResponseBuilder(req, new SolrQueryResponse(), components);
       rb.isDistrib = true;
+      addRequestId(rb, ANY_RID);
+
       //expecting the same results with debugQuery=true or debug=track
       if(random().nextBoolean()) {
         rb.setDebug(true);
@@ -217,7 +222,7 @@ public class DebugComponentTest extends SolrTestCaseJ4 {
         rb.setDebugResults(random().nextBoolean());
       }
       component.prepare(rb);
-      ensureRidPresent(rb, null);
+      ensureTrackRecordsRid(rb, ANY_RID);
     }
    
     req = req("q", "test query", "distrib", "true", CommonParams.REQUEST_ID, "123");
@@ -225,24 +230,7 @@ public class DebugComponentTest extends SolrTestCaseJ4 {
     rb.isDistrib = true;
     rb.setDebug(true);
     component.prepare(rb);
-    ensureRidPresent(rb, "123");
-  }
-  
-  @SuppressWarnings("unchecked")
-  private void ensureRidPresent(ResponseBuilder rb, String expectedRid) {
-    SolrQueryRequest req = rb.req;
-    SolrQueryResponse resp = rb.rsp;
-    //a generated request ID should be added to the request
-    String rid = req.getParams().get(CommonParams.REQUEST_ID);
-    if(expectedRid == null) {
-      assertTrue(rid + " Doesn't match expected pattern.", Pattern.matches(".*-collection1-[0-9]*-[0-9]+", rid));
-    } else {
-      assertEquals("Expecting " + expectedRid + " but found " + rid, expectedRid, rid);
-    }
-    //The request ID is added to the debug/track section
-    assertEquals(rid, ((NamedList<Object>) rb.getDebugInfo().get("track")).get(CommonParams.REQUEST_ID));
-    //RID must be added to the toLog, so that it's included in the main request log
-    assertEquals(rid, resp.getToLog().get(CommonParams.REQUEST_ID));
+    ensureTrackRecordsRid(rb, "123");
   }
 
   //
@@ -279,4 +267,16 @@ public class DebugComponentTest extends SolrTestCaseJ4 {
     );
 
   }
+
+  @SuppressWarnings("unchecked")
+  private void ensureTrackRecordsRid(ResponseBuilder rb, String expectedRid) {
+    final String rid = (String) ((NamedList<Object>) rb.getDebugInfo().get("track")).get(CommonParams.REQUEST_ID);
+    assertEquals("Expecting " + expectedRid + " but found " + rid, expectedRid, rid);
+  }
+
+  private void addRequestId(ResponseBuilder rb, String requestId) {
+    ModifiableSolrParams params = new ModifiableSolrParams(rb.req.getParams());
+    params.add(CommonParams.REQUEST_ID, requestId);
+    rb.req.setParams(params);
+  }
 }

solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java

@@ -266,10 +266,21 @@ public interface CommonParams {
   String COST = "cost";
 
   /**
-   * Request ID parameter added to the request when using debug=track
+   * Request ID parameter added to all distributed queries (that do not opt out)
+   *
+   * @see #DISABLE_REQUEST_ID
    */
   String REQUEST_ID = "rid";
 
+  /**
+   * An opt-out flag to prevent the addition of {@link #REQUEST_ID} tracing on distributed queries
+   *
+   * Defaults to 'false' if not specified.
+   *
+   * @see #REQUEST_ID
+   */
+  String DISABLE_REQUEST_ID = "disableRequestId";
+
   /**
    * Request Purpose parameter added to each internal shard request when using debug=track
    */