You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by st...@apache.org on 2023/09/21 20:55:19 UTC

[solr] branch main updated: SOLR-16960 Tests should sometimes run with a Tracer (not no-op) (#1943)

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

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


The following commit(s) were added to refs/heads/main by this push:
     new 85f5c351858 SOLR-16960 Tests should sometimes run with a Tracer (not no-op) (#1943)
85f5c351858 is described below

commit 85f5c351858d0540d93f2feaef446eb27f2a3883
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)
---
 solr/CHANGES.txt                                     |  2 ++
 .../org/apache/solr/util/tracing/TraceUtils.java     |  9 ++++++++-
 solr/test-framework/build.gradle                     |  1 +
 .../org/apache/solr/cloud/MiniSolrCloudCluster.java  | 20 ++++++++++++++++++++
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index ba8adb622de..24ec5b37e1e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -225,6 +225,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 4d305f373fd..80fc524556b 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
@@ -26,6 +26,7 @@ import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapPropagator;
 import java.util.List;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 import javax.servlet.http.HttpServletRequest;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.request.SolrQueryRequest;
@@ -64,6 +65,12 @@ public class TraceUtils {
 
   public static final String TAG_DB_TYPE_SOLR = "solr";
 
+  /**
+   * this should only be changed in the context of testing, otherwise it would risk not recording
+   * trace data.
+   */
+  public static Predicate<Span> IS_RECORDING = Span::isRecording;
+
   public static Tracer getGlobalTracer() {
     return GlobalOpenTelemetry.getTracer("solr");
   }
@@ -94,7 +101,7 @@ public class TraceUtils {
   }
 
   public static void ifNotNoop(Span span, Consumer<Span> consumer) {
-    if (span.isRecording()) {
+    if (IS_RECORDING.test(span)) {
       consumer.accept(span);
     }
   }
diff --git a/solr/test-framework/build.gradle b/solr/test-framework/build.gradle
index 6c772aac512..cb0ddabf7fa 100644
--- a/solr/test-framework/build.gradle
+++ b/solr/test-framework/build.gradle
@@ -58,6 +58,7 @@ dependencies {
   permitUnusedDeclared 'commons-cli:commons-cli'
   implementation 'org.apache.httpcomponents:httpclient'
   implementation 'org.apache.httpcomponents:httpcore'
+  implementation 'io.opentelemetry:opentelemetry-api'
 
   implementation 'org.eclipse.jetty.toolchain:jetty-servlet-api'
   implementation 'org.eclipse.jetty:jetty-server'
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 3da6dd7a23e..5d7a9e610ec 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;
@@ -1243,6 +1244,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();
@@ -1299,5 +1301,23 @@ public class MiniSolrCloudCluster {
       this.disableTraceIdGeneration = true;
       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 void injectRandomRecordingFlag() {
+      boolean isRecording = LuceneTestCase.rarely();
+      TraceUtils.IS_RECORDING = (ignored) -> isRecording;
+    }
   }
 }