From 80eeb2573ecd545683e0e69dd5550eeab1c79c2b Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Sat, 27 Dec 2025 21:43:22 -0500 Subject: [PATCH 1/2] SOLR-18042 Manage MDC Logging Context setup/teardown in a servlet filter. --- .../servlet/CoreContainerAwareHttpFilter.java | 59 +++++++++++++++++++ .../solr/servlet/CoreContainerProvider.java | 20 +++---- .../apache/solr/servlet/MdcLoggingFilter.java | 57 ++++++++++++++++++ .../solr/servlet/SolrDispatchFilter.java | 53 ++++------------- .../apache/solr/embedded/JettySolrRunner.java | 7 +++ solr/webapp/web/WEB-INF/web.xml | 10 ++++ 6 files changed, 154 insertions(+), 52 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java create mode 100644 solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java b/solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java new file mode 100644 index 000000000000..bfddf0357b72 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java @@ -0,0 +1,59 @@ +/* + * 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 jakarta.servlet.FilterConfig; +import jakarta.servlet.ServletException; +import jakarta.servlet.UnavailableException; +import jakarta.servlet.http.HttpFilter; +import java.lang.invoke.MethodHandles; +import org.apache.solr.core.CoreContainer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** A superclass for filters that will need to interact with the CoreContainer. */ +public abstract class CoreContainerAwareHttpFilter extends HttpFilter { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private CoreContainerProvider containerProvider; + + @Override + public void init(FilterConfig config) throws ServletException { + super.init(config); + containerProvider = CoreContainerProvider.serviceForContext(config.getServletContext()); + if (log.isTraceEnabled()) { + log.trace("{}.init(): {}", this.getClass().getName(), this.getClass().getClassLoader()); + } + } + + /** + * The CoreContainer. It's guaranteed to be constructed before this filter is initialized, but + * could have been shut down. Never null. + */ + public CoreContainer getCores() throws UnavailableException { + return containerProvider.getCoreContainer(); + } + + // TODO: not fond of having these here, but RateLimiter initialization can be sorted out later + RateLimitManager getRateLimitManager() { + return containerProvider.getRateLimitManager(); + } + + void replaceRateLimitManager(RateLimitManager rlm) { + containerProvider.setRateLimitManager(rlm); + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java index 1dcf7fd4ae34..d7e6adad9422 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java @@ -54,7 +54,6 @@ import org.apache.solr.core.NodeConfig; import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrXmlConfig; -import org.apache.solr.metrics.SolrMetricProducer; import org.apache.solr.servlet.RateLimitManager.Builder; import org.apache.solr.util.StartupLoggingUtils; import org.slf4j.Logger; @@ -67,9 +66,8 @@ */ public class CoreContainerProvider implements ServletContextListener { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null); private CoreContainer cores; - private Properties extraProperties; + // TODO: this probably should not live here... private RateLimitManager rateLimitManager; /** @@ -151,7 +149,7 @@ private void init(ServletContext servletContext) { // "extra" properties must be initialized first, so we know things like "do we have a zkHost" // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our // caller - this.extraProperties = + Properties extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded( (Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE)); @@ -160,7 +158,7 @@ private void init(ServletContext servletContext) { log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr()); } - logWelcomeBanner(); + logWelcomeBanner(extraProperties); String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE); if (muteConsole != null @@ -219,7 +217,7 @@ private void init(ServletContext servletContext) { } } - private void logWelcomeBanner() { + private void logWelcomeBanner(Properties props) { // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is // the only one that's so ugly so far. if (log.isInfoEnabled()) { @@ -228,7 +226,7 @@ private void logWelcomeBanner() { if (log.isInfoEnabled()) { log.info( "/ __| ___| |_ _ Starting in {} mode on port {}", - isCloudMode() ? "cloud" : "standalone", + isCloudMode(props) ? "cloud" : "standalone", getSolrPort()); } if (log.isInfoEnabled()) { @@ -290,12 +288,12 @@ private String getSolrPort() { * is non-empty * * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded - * @see #extraProperties * @see #init + * @param props the "extra properties" which will indicate cloud mode before startup. */ - private boolean isCloudMode() { - assert null != extraProperties; // we should never be called w/o this being initialized - return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) + private boolean isCloudMode(Properties props) { + assert null != props; // we should never be called w/o this being initialized + return (null != props.getProperty(SolrXmlConfig.ZK_HOST)) || EnvUtils.getPropertyAsBool("solr.zookeeper.server.enabled", false); } diff --git a/solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java b/solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java new file mode 100644 index 000000000000..30dda3272959 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java @@ -0,0 +1,57 @@ +/* + * 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 jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import org.apache.solr.common.util.SuppressForbidden; +import org.apache.solr.logging.MDCLoggingContext; +import org.apache.solr.logging.MDCSnapshot; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Servlet Filter to set up and tear down MDC Logging context for each reqeust. */ +public class MdcLoggingFilter extends CoreContainerAwareHttpFilter { + + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + @Override + @SuppressForbidden( + reason = + "Set the thread contextClassLoader for all 3rd party dependencies that we cannot control") + protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) + throws IOException, ServletException { + ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + // this autocloseable is here to invoke MDCSnapshot.close() which restores captured state + try (var mdcSnapshot = MDCSnapshot.create()) { + log.trace("MDC snapshot recorded {}", mdcSnapshot); // avoid both compiler and ide warning. + MDCLoggingContext.reset(); + MDCLoggingContext.setNode(getCores()); + // This doesn't belong here, but for the moment it is here to preserve it's relative + // timing of execution for now. Probably I will break this out in a subsequent change + Thread.currentThread().setContextClassLoader(getCores().getResourceLoader().getClassLoader()); + chain.doFilter(req, res); + } finally { + MDCLoggingContext.reset(); + Thread.currentThread().setContextClassLoader(contextClassLoader); + } + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index aad2d3530520..b952eb319c6a 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -20,31 +20,23 @@ import static org.apache.solr.util.tracing.TraceUtils.getSpan; import static org.apache.solr.util.tracing.TraceUtils.setTracer; -import com.google.common.annotations.VisibleForTesting; import jakarta.servlet.FilterChain; import jakarta.servlet.FilterConfig; import jakarta.servlet.ServletException; import jakarta.servlet.UnavailableException; -import jakarta.servlet.http.HttpFilter; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import java.lang.invoke.MethodHandles; -import java.util.List; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -import java.util.regex.Pattern; import org.apache.solr.api.V2HttpCall; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.util.ExecutorUtil; -import org.apache.solr.common.util.SuppressForbidden; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.NodeRoles; import org.apache.solr.handler.api.V2ApiUtils; -import org.apache.solr.logging.MDCLoggingContext; -import org.apache.solr.logging.MDCSnapshot; import org.apache.solr.request.SolrRequestInfo; import org.apache.solr.security.AuditEvent; import org.apache.solr.security.AuditEvent.EventType; @@ -65,19 +57,13 @@ // servlets that are more focused in scope. This should become possible now that we have a // ServletContextListener for startup/shutdown of CoreContainer that sets up a service from which // things like CoreContainer can be requested. (or better yet injected) -public class SolrDispatchFilter extends HttpFilter { +public class SolrDispatchFilter extends CoreContainerAwareHttpFilter { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private CoreContainerProvider containerProvider; - - protected final CountDownLatch init = new CountDownLatch(1); - protected String abortErrorMessage = null; private HttpSolrCallFactory solrCallFactory; - private List excludePatterns; - public final boolean isV2Enabled = V2ApiUtils.isEnabled(); /** @@ -115,9 +101,8 @@ public SolrDispatchFilter() {} @Override public void init(FilterConfig config) throws ServletException { - super.init(config); try { - containerProvider = CoreContainerProvider.serviceForContext(config.getServletContext()); + super.init(config); boolean isCoordinator = NodeRoles.MODE_ON.equals(getCores().nodeRoles.getRoleMode(NodeRoles.Role.COORDINATOR)); solrCallFactory = @@ -134,37 +119,28 @@ public void init(FilterConfig config) throws ServletException { } } finally { log.trace("SolrDispatchFilter.init() done"); - init.countDown(); } } - /** The CoreContainer. It's ready for use, albeit could shut down whenever. Never null. */ - public CoreContainer getCores() throws UnavailableException { - return containerProvider.getCoreContainer(); - } - @Override - @SuppressForbidden( - reason = - "Set the thread contextClassLoader for all 3rd party dependencies that we cannot control") public void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + // internal version of doFilter that tracks if we are in a retry + doFilterRetry(closeShield(request), closeShield(response), chain, false); + } - try (var mdcSnapshot = MDCSnapshot.create()) { - assert null != mdcSnapshot; // prevent compiler warning - MDCLoggingContext.reset(); - MDCLoggingContext.setNode(getCores()); - Thread.currentThread().setContextClassLoader(getCores().getResourceLoader().getClassLoader()); + /* - doFilterRetry(closeShield(request), closeShield(response), chain, false); - } - } + Wait? Where did X go??? (I hear you ask). First: look for a servlet filter to which request wrapping + operations may have been moved (see web.xml for list) + + */ - protected void doFilterRetry( + private void doFilterRetry( HttpServletRequest request, HttpServletResponse response, FilterChain chain, boolean retry) throws IOException, ServletException { setTracer(request, getCores().getTracer()); - RateLimitManager rateLimitManager = containerProvider.getRateLimitManager(); + RateLimitManager rateLimitManager = getRateLimitManager(); try { ServletUtils.rateLimitRequest( rateLimitManager, @@ -369,11 +345,6 @@ private boolean shouldAudit(AuditEvent.EventType eventType) { && cores.getAuditLoggerPlugin().shouldLog(eventType); } - @VisibleForTesting - void replaceRateLimitManager(RateLimitManager rateLimitManager) { - containerProvider.setRateLimitManager(rateLimitManager); - } - /** internal API */ public interface HttpSolrCallFactory { default HttpSolrCall createInstance( 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 982fc98c135b..2825b1f4a720 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 @@ -61,6 +61,7 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.servlet.CoreContainerProvider; +import org.apache.solr.servlet.MdcLoggingFilter; import org.apache.solr.servlet.PathExclusionFilter; import org.apache.solr.servlet.SolrDispatchFilter; import org.apache.solr.util.SocketProxy; @@ -112,6 +113,7 @@ public class JettySolrRunner { volatile FilterHolder debugFilter; volatile FilterHolder pathExcludeFilter; + volatile FilterHolder mdcLoggingFilter; volatile FilterHolder dispatchFilter; private int jettyPort = -1; @@ -409,12 +411,17 @@ public void contextInitialized(ServletContextEvent event) { pathExcludeFilter.setHeldClass(PathExclusionFilter.class); pathExcludeFilter.setInitParameter("excludePatterns", excludePatterns); + // logging context setup + mdcLoggingFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); + mdcLoggingFilter.setHeldClass(MdcLoggingFilter.class); + // This is our main workhorse dispatchFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); dispatchFilter.setHeldClass(SolrDispatchFilter.class); // Map dispatchFilter in same path as in web.xml root.addFilter(pathExcludeFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); + root.addFilter(mdcLoggingFilter, "/*", 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 aa3a192862de..a7df6a321131 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -44,6 +44,16 @@ /* + + MDCLoggingFilter + org.apache.solr.servlet.MdcLoggingFilter + + + + MDCLoggingFilter + /* + + SolrRequestFilter org.apache.solr.servlet.SolrDispatchFilter From 94c6995aa120f21262de5f442a7dcf97cb898b39 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Sat, 27 Dec 2025 22:41:36 -0500 Subject: [PATCH 2/2] SOLR-18042 changelog --- changelog/unreleased/SOLR-18042.yml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/SOLR-18042.yml diff --git a/changelog/unreleased/SOLR-18042.yml b/changelog/unreleased/SOLR-18042.yml new file mode 100644 index 000000000000..73ea60967dff --- /dev/null +++ b/changelog/unreleased/SOLR-18042.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: SOLR 18042 - MDC Logging context lifecycle is now managed by a servlet filter +type: other # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Gus Heck +links: + - name: SOLR-18042 + url: https://issues.apache.org/jira/browse/SOLR-18042