SOLR-17934: Remove Request ID based tracing (#3709)

Author: epugh

changelog/unreleased/deprecation_removal_char_SOLR-17934.yml

@@ -0,0 +1,10 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Remove Request ID based tracing.
+type: removed
+authors:
+  - name: Eric Pugh
+links:
+  - name: SOLR-17934
+    url: https://issues.apache.org/jira/browse/SOLR-17934
+issues:
+  - 17934

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

@@ -73,7 +73,7 @@ public class DebugComponent extends SearchComponent {
   public void prepare(ResponseBuilder rb) throws IOException {
     if (rb.isDebugTrack() && rb.isDistrib) {
       rb.setNeedDocList(true);
-      doDebugTrack(rb);
+      rb.addDebug(new SimpleOrderedMap<>(), "track");
     }
   }
 
@@ -140,11 +140,6 @@ public void process(ResponseBuilder rb) throws IOException {
     }
   }
 
-  private void doDebugTrack(ResponseBuilder rb) {
-    final String rid = rb.req.getParams().get(CommonParams.REQUEST_ID);
-    rb.addDebug(rid, "track", CommonParams.REQUEST_ID); // to see it in the response
-  }
-
   @Override
   public void modifyRequest(ResponseBuilder rb, SearchComponent who, ShardRequest sreq) {
     if (!rb.isDebug()) return;
@@ -176,7 +171,6 @@ public void modifyRequest(ResponseBuilder rb, SearchComponent who, ShardRequest
     }
     if (rb.isDebugTrack()) {
       sreq.params.add(CommonParams.DEBUG, CommonParams.TRACK);
-      sreq.params.set(CommonParams.REQUEST_ID, rb.req.getParams().get(CommonParams.REQUEST_ID));
       sreq.params.set(
           CommonParams.REQUEST_PURPOSE, SolrPluginUtils.getRequestPurpose(sreq.purpose));
     }

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

@@ -344,6 +344,9 @@ public void setDebugResults(boolean debugResults) {
   }
 
   public NamedList<Object> getDebugInfo() {
+    if (debugInfo == null) {
+      debugInfo = new SimpleOrderedMap<>();
+    }
     return debugInfo;
   }
 

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

@@ -41,7 +41,6 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 import org.apache.lucene.index.ExitableDirectoryReader;
@@ -57,13 +56,11 @@
 import org.apache.solr.common.params.ShardParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.core.CloseHook;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.PluginInfo;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.handler.RequestHandlerBase;
-import org.apache.solr.logging.MDCLoggingContext;
 import org.apache.solr.metrics.SolrMetricsContext;
 import org.apache.solr.pkg.PackageAPI;
 import org.apache.solr.pkg.PackageListeners;
@@ -86,7 +83,6 @@
 import org.apache.solr.util.plugin.SolrCoreAware;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.slf4j.MDC;
 
 /** Refer SOLR-281 */
 public class SearchHandler extends RequestHandlerBase
@@ -99,27 +95,6 @@ public class SearchHandler extends RequestHandlerBase
 
   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
-   *
-   * @deprecated this was replaced by the auto-generated trace ids
-   */
-  @Deprecated(since = "9.4")
-  private static final AtomicLong ridCounter = new AtomicLong();
-
-  /**
-   * An opt-out flag to prevent the addition of {@link CommonParams#REQUEST_ID} tracing on
-   * distributed queries
-   *
-   * <p>Defaults to 'false' if not specified.
-   *
-   * @see CommonParams#DISABLE_REQUEST_ID
-   * @deprecated this was replaced by the auto-generated trace ids
-   */
-  @Deprecated(since = "9.4")
-  private static final boolean DISABLE_REQUEST_ID_DEFAULT =
-      Boolean.getBoolean("solr.disableRequestId");
-
   private HandlerMetrics metricsShard = HandlerMetrics.NO_OP;
 
   protected volatile List<SearchComponent> components;
@@ -250,8 +225,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
     }
 
     rb.isDistrib = isDistrib(req, rb); // can change later nonetheless
-    tagRequestWithRequestId(rb);
-
+    
     boolean dbg = req.getParams().getBool(CommonParams.DEBUG_QUERY, false);
     rb.setDebug(dbg);
     if (dbg == false) { // if it's true, we are doing everything anyway.
@@ -868,62 +842,6 @@ private static void throwSolrException(Throwable shardResponseException) throws
     }
   }
 
-  private void tagRequestWithRequestId(ResponseBuilder rb) {
-    final boolean ridTaggingDisabled =
-        rb.req.getParams().getBool(CommonParams.DISABLE_REQUEST_ID, DISABLE_REQUEST_ID_DEFAULT);
-    if (!ridTaggingDisabled) {
-      String rid = getOrGenerateRequestId(rb.req);
-
-      // NOTE: SearchHandler explicitly never clears/removes this MDC value...
-      // We want it to live for the entire request, beyond the scope of SearchHandler's processing,
-      // and trust SolrDispatchFilter to clean it up at the end of the request.
-      //
-      // Examples:
-      // - ERROR logging of Exceptions propogated up to our base class
-      // - SolrCore.RequestLog
-      // - ERRORs that may be logged during response writing
-      MDC.put(CommonParams.REQUEST_ID, rid);
-
-      if (StrUtils.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.
-   *
-   * <p>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);
-    if (StrUtils.isNotBlank(rid)) {
-      return rid;
-    }
-    String traceId = MDCLoggingContext.getTraceId();
-    if (StrUtils.isNotBlank(traceId)) {
-      return traceId;
-    }
-    return generateRid(req);
-  }
-
-  private static String generateRid(SolrQueryRequest req) {
-    String hostName = req.getCoreContainer().getHostName();
-    return hostName + "-" + ridCounter.getAndIncrement();
-  }
-
   //////////////////////// SolrInfoMBeans methods //////////////////////
 
   @Override

solr/core/src/test/org/apache/solr/handler/TestRequestId.java

@@ -23,7 +23,6 @@
 import org.apache.solr.SolrTestCaseJ4;
 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.request.SolrQueryRequest;
 import org.apache.solr.response.SolrQueryResponse;
 import org.junit.BeforeClass;
@@ -32,8 +31,6 @@
 /** */
 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");
@@ -166,8 +163,7 @@ public void testModifyRequestTrack() {
     List<SearchComponent> components = new ArrayList<>(1);
     components.add(component);
     for (int i = 0; i < 10; i++) {
-      SolrQueryRequest req =
-          req("q", "test query", "distrib", "true", CommonParams.REQUEST_ID, "123456-my_rid");
+      SolrQueryRequest req = req("q", "test query", "distrib", "true");
       SolrQueryResponse resp = new SolrQueryResponse();
       ResponseBuilder rb = new ResponseBuilder(req, resp, components);
       ShardRequest sreq = new ShardRequest();
@@ -192,8 +188,6 @@ public void testModifyRequestTrack() {
       // the purpose must be added as readable param to be included in the shard logs
       assertEquals(
           "GET_FIELDS,GET_DEBUG,SET_TERM_STATS", sreq.params.get(CommonParams.REQUEST_PURPOSE));
-      // the rid must be added to be included in the shard logs
-      assertEquals("123456-my_rid", sreq.params.get(CommonParams.REQUEST_ID));
       // close requests - this method obtains a searcher in order to access its StatsCache
       req.close();
     }
@@ -210,7 +204,6 @@ public void testPrepare() throws IOException {
       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()) {
@@ -224,15 +217,13 @@ public void testPrepare() throws IOException {
         rb.setDebugResults(random().nextBoolean());
       }
       component.prepare(rb);
-      ensureTrackRecordsRid(rb, ANY_RID);
     }
 
-    req = req("q", "test query", "distrib", "true", CommonParams.REQUEST_ID, "123");
+    req = req("q", "test query", "distrib", "true");
     rb = new ResponseBuilder(req, new SolrQueryResponse(), components);
     rb.isDistrib = true;
     rb.setDebug(true);
     component.prepare(rb);
-    ensureTrackRecordsRid(rb, "123");
   }
 
   //
@@ -285,21 +276,8 @@ public void testQueryToString() {
         "//str[@name='parsedquery'][contains(.,'3.0')]");
   }
 
-  @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);
-  }
-
   @Test
-  public void testDistributedStageResolution() throws IOException {
+  public void testDistributedStageResolution() {
     final DebugComponent debugComponent = new DebugComponent();
     assertEquals(
         "PARSE_QUERY", debugComponent.getDistributedStageName(ResponseBuilder.STAGE_PARSE_QUERY));

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

@@ -121,7 +121,6 @@ public void testSimpleSearch() throws Exception {
     QueryResponse response = collection1.query(query);
     NamedList<Object> track = (NamedList<Object>) response.getDebugMap().get("track");
     assertNotNull(track);
-    assertNotNull(track.get("rid"));
     assertNotNull(track.get("EXECUTE_QUERY"));
     assertNotNull(((NamedList<Object>) track.get("EXECUTE_QUERY")).get(shard1));
     assertNotNull(((NamedList<Object>) track.get("EXECUTE_QUERY")).get(shard2));

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

@@ -277,26 +277,6 @@ public static EchoParamStyle get(String v) {
    */
   String COST = "cost";
 
-  /**
-   * Request ID parameter added to all distributed queries (that do not opt out)
-   *
-   * @see #DISABLE_REQUEST_ID
-   * @deprecated this was replaced by the auto-generated trace ids
-   */
-  @Deprecated(since = "9.4")
-  String REQUEST_ID = "rid";
-
-  /**
-   * An opt-out flag to prevent the addition of {@link #REQUEST_ID} tracing on distributed queries
-   *
-   * <p>Defaults to 'false' if not specified.
-   *
-   * @see #REQUEST_ID
-   * @deprecated this was replaced by the auto-generated trace ids
-   */
-  @Deprecated(since = "9.4")
-  String DISABLE_REQUEST_ID = "disableRequestId";
-
   /**
    * Parameter to control the distributed term statistics request for current query when distributed
    * IDF is enabled in solrconfig

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

@@ -7,7 +7,6 @@
 AWS_PROFILE=aws.profile
 SOLR_ALWAYS_ON_TRACE_ID=solr.alwaysOnTraceId
 SOLR_AUTH_JWT_ALLOW_OUTBOUND_HTTP=solr.auth.jwt.allowOutboundHttp
-SOLR_DISABLE_REQUEST_ID=solr.disableRequestId
 SOLR_HIDDEN_SYS_PROPS=solr.hiddenSysProps
 SOLR_HOME=solr.solr.home
 SOLR_HOST_ADVERTISE=host