Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions changelog/unreleased/SOLR-18042.yml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand Down Expand Up @@ -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));

Expand All @@ -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
Expand Down Expand Up @@ -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()) {
Expand All @@ -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()) {
Expand Down Expand Up @@ -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);
}

Expand Down
57 changes: 57 additions & 0 deletions solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully not a future Filter for just one line.
I sympathize with your characterization of SolrDispatchFilter as a kitchen sink but a few lines for certain topics at the critical Solr entrypoint is understandable IMO.

Thread.currentThread().setContextClassLoader(getCores().getResourceLoader().getClassLoader());
chain.doFilter(req, res);
} finally {
MDCLoggingContext.reset();
Thread.currentThread().setContextClassLoader(contextClassLoader);
}
}
}
53 changes: 12 additions & 41 deletions solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Pattern> excludePatterns;

public final boolean isV2Enabled = V2ApiUtils.isEnabled();

/**
Expand Down Expand Up @@ -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 =
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the comment but I think it could be worded more directly to the point.
"NOTE: Remember Solr has a series of Servlet filters in advance doing miscellaneous things like MDC, excludePatterns, ..."
The above comment would be searchablel.

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,
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -112,6 +113,7 @@ public class JettySolrRunner {

volatile FilterHolder debugFilter;
volatile FilterHolder pathExcludeFilter;
volatile FilterHolder mdcLoggingFilter;
volatile FilterHolder dispatchFilter;

private int jettyPort = -1;
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions solr/webapp/web/WEB-INF/web.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@
<url-pattern>/*</url-pattern>
</filter-mapping>

<filter>
<filter-name>MDCLoggingFilter</filter-name>
<filter-class>org.apache.solr.servlet.MdcLoggingFilter</filter-class>
</filter>

<filter-mapping>
<filter-name>MDCLoggingFilter</filter-name>
<url-pattern>/*</url-pattern>
</filter-mapping>

<filter>
<filter-name>SolrRequestFilter</filter-name>
<filter-class>org.apache.solr.servlet.SolrDispatchFilter</filter-class>
Expand Down