From cb4b6514f973ace4b762a0f445ef961955a5f6bb Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Thu, 5 Feb 2026 19:21:15 -0500 Subject: [PATCH 1/4] SOLR-18047 Move tracing initialization to TracingFilter --- .../solr/servlet/ExceptionWhileTracing.java | 28 --------- .../apache/solr/servlet/RateLimitFilter.java | 13 +--- .../org/apache/solr/servlet/ServletUtils.java | 50 --------------- .../apache/solr/servlet/TracingFilter.java | 63 +++++++++++++++++++ .../apache/solr/embedded/JettySolrRunner.java | 7 +++ solr/webapp/web/WEB-INF/web.xml | 10 +++ 6 files changed, 81 insertions(+), 90 deletions(-) delete mode 100644 solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java create mode 100644 solr/core/src/java/org/apache/solr/servlet/TracingFilter.java diff --git a/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java b/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java deleted file mode 100644 index c01d3d0c3f8b..000000000000 --- a/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * 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; - -/* - * This is not pretty, hope to remove it when tracing becomes a filter. - */ -public class ExceptionWhileTracing extends RuntimeException { - public Exception e; - - public ExceptionWhileTracing(Exception e) { - this.e = e; - } -} diff --git a/solr/core/src/java/org/apache/solr/servlet/RateLimitFilter.java b/solr/core/src/java/org/apache/solr/servlet/RateLimitFilter.java index 6f0d301e086e..2f8990fdea1e 100644 --- a/solr/core/src/java/org/apache/solr/servlet/RateLimitFilter.java +++ b/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()); diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index 97af38d8b274..d188e627ef7f 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/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 diff --git a/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java b/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java new file mode 100644 index 000000000000..cb5477f0513f --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java @@ -0,0 +1,63 @@ +/* + * 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.FilterConfig; +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; + +public class TracingFilter extends HttpFilter { + + @Override + public void init(FilterConfig config) throws ServletException { + super.init(config); + } + + @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(); + } + } +} diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index 2530ed7acd4a..e4224e44e6c8 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/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 diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml index e383133ea6dc..6732a6cb58a4 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -64,6 +64,16 @@ /* + + TracingFilter + org.apache.solr.servlet.TracingFilter + + + + TracingFilter + /* + + SolrRequestFilter org.apache.solr.servlet.SolrDispatchFilter From 6878362b25ad80c10d856d70381e0d5ed09eefe6 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Thu, 5 Feb 2026 19:25:53 -0500 Subject: [PATCH 2/4] SOLR-18047 changelog --- changelog/unreleased/SOLR-18047.yml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/SOLR-18047.yml diff --git a/changelog/unreleased/SOLR-18047.yml b/changelog/unreleased/SOLR-18047.yml new file mode 100644 index 000000000000..c8474de04963 --- /dev/null +++ b/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 From b0edfdba9c6cd5053f0c07c58b9d46ad849ee571 Mon Sep 17 00:00:00 2001 From: Gus Heck <46900717+gus-asf@users.noreply.github.com> Date: Thu, 5 Feb 2026 23:10:20 -0500 Subject: [PATCH 3/4] Add Javadoc Co-authored-by: David Smiley --- solr/core/src/java/org/apache/solr/servlet/TracingFilter.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java b/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java index cb5477f0513f..c3339776e49d 100644 --- a/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java @@ -29,6 +29,9 @@ import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.util.tracing.TraceUtils; +/** + * Filter for distributed tracing -- creating a span for this request. + */ public class TracingFilter extends HttpFilter { @Override From e57ec6b8b237d4b518bfc97cfd6d771ae1c2f3a6 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Fri, 6 Feb 2026 01:57:23 -0500 Subject: [PATCH 4/4] SOLR-18047 review feedback --- .../java/org/apache/solr/servlet/TracingFilter.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java b/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java index c3339776e49d..3371d26d98c9 100644 --- a/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/TracingFilter.java @@ -20,7 +20,6 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import jakarta.servlet.FilterChain; -import jakarta.servlet.FilterConfig; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpFilter; import jakarta.servlet.http.HttpServletRequest; @@ -30,15 +29,12 @@ import org.apache.solr.util.tracing.TraceUtils; /** - * Filter for distributed tracing -- creating a span for this request. + * 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 - public void init(FilterConfig config) throws ServletException { - super.init(config); - } - @Override protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) throws IOException, ServletException {