[solr] branch branch_9x updated: SOLR-16960 Tests should sometimes run with a Tracer (not no-op) (#1943) (manual cherry pick from commit 85f5c35 and 1d0d75a)

[st...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

stillalex pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new 21373f89063 SOLR-16960 Tests should sometimes run with a Tracer (not no-op) (#1943) (manual cherry pick from commit 85f5c35 and 1d0d75a)
21373f89063 is described below

commit 21373f89063dab898ab8dfbcc6bf89faf547fff6
Author: Alex D <st...@apache.org>
AuthorDate: Thu Sep 21 13:55:10 2023 -0700

    SOLR-16960 Tests should sometimes run with a Tracer (not no-op) (#1943)
    (manual cherry pick from commit 85f5c35 and 1d0d75a)
---
 solr/CHANGES.txt                                   |  2 ++
 .../org/apache/solr/util/tracing/TraceUtils.java   | 12 ++++++++++-
 solr/test-framework/build.gradle                   |  1 +
 .../apache/solr/cloud/MiniSolrCloudCluster.java    | 25 ++++++++++++++++++++++
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 395d6a9f292..fc0068c7d4a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -167,6 +167,8 @@ Other Changes
 
 * SOLR-16978: Be case insensitive when parsing booleans from text (Tomás Fernández Löbbe)
 
+* SOLR-16960: Tests should sometimes run with a Tracer (not no-op) (Alex Deparvu)
+
 ==================  9.3.0 ==================
 
 Upgrade Notes
diff --git a/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java b/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
index 069da747a34..c64dcffa88a 100644
--- a/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
+++ b/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
@@ -21,11 +21,21 @@ import io.opentracing.noop.NoopSpan;
 import io.opentracing.tag.Tags;
 import java.util.List;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 import org.apache.solr.request.SolrQueryRequest;
 
 /** Utilities for distributed tracing. */
 public class TraceUtils {
 
+  public static final Predicate<Span> DEFAULT_IS_RECORDING =
+      (span) -> span != null && !(span instanceof NoopSpan);
+
+  /**
+   * this should only be changed in the context of testing, otherwise it would risk not recording
+   * trace data.
+   */
+  public static Predicate<Span> IS_RECORDING = DEFAULT_IS_RECORDING;
+
   public static void setDbInstance(SolrQueryRequest req, String coreOrColl) {
     if (req != null && coreOrColl != null) {
       ifNotNoop(req.getSpan(), (span) -> span.setTag(Tags.DB_INSTANCE, coreOrColl));
@@ -33,7 +43,7 @@ public class TraceUtils {
   }
 
   public static void ifNotNoop(Span span, Consumer<Span> consumer) {
-    if (span != null && !(span instanceof NoopSpan)) {
+    if (IS_RECORDING.test(span)) {
       consumer.accept(span);
     }
   }
diff --git a/solr/test-framework/build.gradle b/solr/test-framework/build.gradle
index c6589155072..7905d2be21a 100644
--- a/solr/test-framework/build.gradle
+++ b/solr/test-framework/build.gradle
@@ -54,6 +54,7 @@ dependencies {
   implementation 'org.apache.logging.log4j:log4j-core'
   implementation 'io.opentracing:opentracing-noop'
   implementation 'io.opentracing:opentracing-util'
+  implementation 'io.opentracing:opentracing-api'
   implementation 'io.dropwizard.metrics:metrics-core'
   implementation 'io.dropwizard.metrics:metrics-jetty10'
   implementation 'commons-cli:commons-cli'
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java
index 6ebd4400ecf..45d8b78174a 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java
@@ -83,6 +83,7 @@ import org.apache.solr.embedded.JettyConfig;
 import org.apache.solr.embedded.JettySolrRunner;
 import org.apache.solr.util.TimeOut;
 import org.apache.solr.util.tracing.SimplePropagator;
+import org.apache.solr.util.tracing.TraceUtils;
 import org.apache.zookeeper.KeeperException;
 import org.eclipse.jetty.server.handler.HandlerWrapper;
 import org.eclipse.jetty.servlet.ServletHolder;
@@ -746,6 +747,7 @@ public class MiniSolrCloudCluster {
       if (!externalZkServer) {
         zkServer.shutdown();
       }
+      resetRecordingFlag();
     }
   }
 
@@ -1305,6 +1307,7 @@ public class MiniSolrCloudCluster {
       // eager init to prevent OTEL init races caused by test setup
       if (!disableTraceIdGeneration && TracerConfigurator.TRACE_ID_GEN_ENABLED) {
         SimplePropagator.load();
+        injectRandomRecordingFlag();
       }
 
       JettyConfig jettyConfig = jettyConfigBuilder.build();
@@ -1362,4 +1365,26 @@ public class MiniSolrCloudCluster {
       return this;
     }
   }
+
+  /**
+   * Randomizes the tracing Span::isRecording check.
+   *
+   * <p>This will randomize the Span::isRecording check so we have better coverage of all methods
+   * that deal with span creation without having to enable otel module.
+   *
+   * <p>It only makes sense to call this if we are using the alwaysOn tracer, the OTEL tracer
+   * already has this flag turned on and randomizing it would risk not recording trace data.
+   *
+   * <p>Note. Tracing is not a SolrCloud only feature. this method is placed here for convenience
+   * only, any test can make use of this example for more complete coverage of the tracing
+   * mechanics.
+   */
+  private static void injectRandomRecordingFlag() {
+    boolean isRecording = LuceneTestCase.rarely();
+    TraceUtils.IS_RECORDING = (ignored) -> isRecording;
+  }
+
+  private static void resetRecordingFlag() {
+    TraceUtils.IS_RECORDING = TraceUtils.DEFAULT_IS_RECORDING;
+  }
 }