SOLR-18047 Move tracing initialization to TracingFilter

changelog/unreleased/SOLR-18047.yml

@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Initialization of tracing spans has moved to TracingFilter
+type: other # added, changed, fixed, deprecated, removed, dependency_update, security, other
+authors:
+  - name: Gus Heck
+links:
+  - name: SOLR-18047
+    url: https://issues.apache.org/jira/browse/SOLR-18047

solr/core/src/java/org/apache/solr/servlet/RateLimitFilter.java

@@ -47,18 +47,7 @@ protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterC
             SolrException.ErrorCode.TOO_MANY_REQUESTS.code, RateLimitManager.ERROR_MESSAGE);
         return;
       }
-      // todo: this shouldn't be required, tracing and rate limiting should be independently
-      //  composable
-      ServletUtils.traceHttpRequestExecution2(
-          req,
-          res,
-          () -> {
-            try {
-              chain.doFilter(req, res);
-            } catch (Exception e) {
-              throw new ExceptionWhileTracing(e);
-            }
-          });
+      chain.doFilter(req, res);
     } catch (InterruptedException e1) {
       Thread.currentThread().interrupt();
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e1.getMessage());

solr/core/src/java/org/apache/solr/servlet/ServletUtils.java

@@ -17,10 +17,7 @@
 
 package org.apache.solr.servlet;
 
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.context.Context;
 import jakarta.servlet.ReadListener;
-import jakarta.servlet.ServletException;
 import jakarta.servlet.ServletInputStream;
 import jakarta.servlet.ServletOutputStream;
 import jakarta.servlet.WriteListener;
@@ -33,8 +30,6 @@
 import java.io.OutputStream;
 import java.lang.invoke.MethodHandles;
 import org.apache.solr.common.util.Utils;
-import org.apache.solr.logging.MDCLoggingContext;
-import org.apache.solr.util.tracing.TraceUtils;
 import org.eclipse.jetty.http.HttpHeader;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -124,51 +119,6 @@ public void close() {
     };
   }
 
-  /**
-   * Sets up tracing for an HTTP request. Perhaps should be converted to a servlet filter at some
-   * point.
-   *
-   * @param request The request to limit
-   * @param response The associated response
-   * @param tracedExecution the executed code
-   */
-  static void traceHttpRequestExecution2(
-      HttpServletRequest request, HttpServletResponse response, Runnable tracedExecution)
-      throws ServletException, IOException {
-    Context context = TraceUtils.extractContext(request);
-    Span span = TraceUtils.startHttpRequestSpan(request, context);
-
-    final Thread currentThread = Thread.currentThread();
-    final String oldThreadName = currentThread.getName();
-    try (var scope = context.with(span).makeCurrent()) {
-      assert scope != null; // prevent javac warning about scope being unused
-      TraceUtils.setSpan(request, span);
-      TraceUtils.ifValidTraceId(
-          span, s -> MDCLoggingContext.setTracerId(s.getSpanContext().getTraceId()));
-      String traceId = MDCLoggingContext.getTraceId();
-      if (traceId != null) {
-        currentThread.setName(oldThreadName + "-" + traceId);
-      }
-      tracedExecution.run();
-    } catch (ExceptionWhileTracing e) {
-      if (e.e instanceof ServletException) {
-        throw (ServletException) e.e;
-      }
-      if (e.e instanceof IOException) {
-        throw (IOException) e.e;
-      }
-      if (e.e instanceof RuntimeException) {
-        throw (RuntimeException) e.e;
-      } else {
-        throw new RuntimeException(e.e);
-      }
-    } finally {
-      currentThread.setName(oldThreadName);
-      TraceUtils.setHttpStatus(span, response.getStatus());
-      span.end();
-    }
-  }
-
   // we make sure we read the full client request so that the client does
   // not hit a connection reset and we can reuse the
   // connection - see SOLR-8453 and SOLR-8683

solr/core/src/java/org/apache/solr/servlet/TracingFilter.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.servlet;
+
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.context.Context;
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpFilter;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import org.apache.solr.logging.MDCLoggingContext;
+import org.apache.solr.util.tracing.TraceUtils;
+
+/**
+ * Filter for distributed tracing. This filter creates a span for this request. While this filter
+ * could be replaced, the replacement must supply an instance of io.opentelemetry.api.trace.Span for
+ * use in the rest of solr.
+ */
+public class TracingFilter extends HttpFilter {
+
+  @Override
+  protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
+      throws IOException, ServletException {
+    Context context = TraceUtils.extractContext(req);
+    Span span = TraceUtils.startHttpRequestSpan(req, context);
+
+    final Thread currentThread = Thread.currentThread();
+    final String oldThreadName = currentThread.getName();
+    try (var scope = context.with(span).makeCurrent()) {
+      assert scope != null; // prevent javac warning about scope being unused
+      TraceUtils.setSpan(req, span);
+      TraceUtils.ifValidTraceId(
+          span, s -> MDCLoggingContext.setTracerId(s.getSpanContext().getTraceId()));
+      String traceId = MDCLoggingContext.getTraceId();
+      if (traceId != null) {
+        currentThread.setName(oldThreadName + "-" + traceId);
+      }
+      chain.doFilter(req, res);
+    } finally {
+      currentThread.setName(oldThreadName);
+      TraceUtils.setHttpStatus(span, res.getStatus());
+      span.end();
+    }
+  }
+}

solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java

@@ -65,6 +65,7 @@
 import org.apache.solr.servlet.RateLimitFilter;
 import org.apache.solr.servlet.RequiredSolrRequestFilter;
 import org.apache.solr.servlet.SolrDispatchFilter;
+import org.apache.solr.servlet.TracingFilter;
 import org.apache.solr.util.SocketProxy;
 import org.apache.solr.util.TimeOut;
 import org.apache.solr.util.configuration.SSLConfigurationsFactory;
@@ -117,6 +118,7 @@ public class JettySolrRunner {
   volatile FilterHolder requiredFilter;
   volatile FilterHolder rateLimitFilter;
   volatile FilterHolder dispatchFilter;
+  private FilterHolder tracingFilter;
 
   private int jettyPort = -1;
 
@@ -421,6 +423,10 @@ public void contextInitialized(ServletContextEvent event) {
       rateLimitFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED);
       rateLimitFilter.setHeldClass(RateLimitFilter.class);
 
+      // Ratelimit Requests
+      tracingFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED);
+      tracingFilter.setHeldClass(TracingFilter.class);
+
       // This is our main workhorse
       dispatchFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED);
       dispatchFilter.setHeldClass(SolrDispatchFilter.class);
@@ -429,6 +435,7 @@ public void contextInitialized(ServletContextEvent event) {
       root.addFilter(pathExcludeFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
       root.addFilter(requiredFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
       root.addFilter(rateLimitFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
+      root.addFilter(tracingFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
       root.addFilter(dispatchFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
 
       // Default servlet as a fall-through

solr/webapp/web/WEB-INF/web.xml

@@ -64,6 +64,16 @@
     <url-pattern>/*</url-pattern>
   </filter-mapping>
 
+  <filter>
+    <filter-name>TracingFilter</filter-name>
+    <filter-class>org.apache.solr.servlet.TracingFilter</filter-class>
+  </filter>
+
+  <filter-mapping>
+    <filter-name>TracingFilter</filter-name>
+    <url-pattern>/*</url-pattern>
+  </filter-mapping>
+
   <filter>
     <filter-name>SolrRequestFilter</filter-name>
     <filter-class>org.apache.solr.servlet.SolrDispatchFilter</filter-class>