This is an automated email from the ASF dual-hosted git repository.
hossman 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 2e5f89a SOLR-15783: Prevent Logging MDC values from leaking between
request threads, and set 'trace_id' in MDC as soon as it's available
2e5f89a is described below
commit 2e5f89a87296269cdf05ac30d178089832cfa5a7
Author: Chris Hostetter <[email protected]>
AuthorDate: Thu Nov 11 14:57:07 2021 -0700
SOLR-15783: Prevent Logging MDC values from leaking between request
threads, and set 'trace_id' in MDC as soon as it's available
---
solr/CHANGES.txt | 2 +
.../java/org/apache/solr/logging/MDCSnapshot.java | 58 ++++++++++++++++++++++
.../java/org/apache/solr/servlet/HttpSolrCall.java | 4 --
.../apache/solr/servlet/SolrDispatchFilter.java | 11 +++-
.../src/java/org/apache/solr/util/TestHarness.java | 12 +++--
5 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index fd82e10..710bd8a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -408,6 +408,8 @@ Bug Fixes
* SOLR-10529: Solr UI Health Check enable/disable ping Button doesn't work
(Oscar Wang via janhoy)
+* SOLR-15783: Prevent Logging MDC values from leaking between request threads,
and set 'trace_id' in MDC as soon as it's available (hossman)
+
================== 8.11.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this
release.
diff --git a/solr/core/src/java/org/apache/solr/logging/MDCSnapshot.java
b/solr/core/src/java/org/apache/solr/logging/MDCSnapshot.java
new file mode 100644
index 0000000..040316d
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/logging/MDCSnapshot.java
@@ -0,0 +1,58 @@
+/*
+ * 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.logging;
+
+import java.io.Closeable;
+import java.util.Map;
+
+import org.slf4j.MDC;
+
+
+/**
+ * Takes a 'snapshot' of the current MDC context map which is restored on
(auto) close.
+ * This can be used to ensure that no MDC values set (or overridden) inside of
call stack
+ * will "leak" beyond that call stack.
+ *
+ * <pre>
+ * try (var mdcSnapshot = MDCSnapshot.create()) {
+ * assert null != mdcSnapshot; // prevent compiler warning
+ *
+ * // ... arbitrary calls to MDC methods
+ * }
+ * </pre>
+ *
+ * @see org.slf4j.MDC#putCloseable
+ */
+public final class MDCSnapshot implements Closeable {
+
+ public static MDCSnapshot create() {
+ return new MDCSnapshot();
+ }
+
+ private final Map<String,String> snapshot;
+ private MDCSnapshot() {
+ this.snapshot = MDC.getCopyOfContextMap();
+ assert null != snapshot;
+ }
+
+ public void close() {
+ MDC.clear();
+ if (! snapshot.isEmpty()) { // common case optimization
+ MDC.setContextMap(snapshot);
+ }
+ }
+}
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index 460e8d6..a414102 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -90,7 +90,6 @@ import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrConfig;
import org.apache.solr.core.SolrCore;
import org.apache.solr.handler.ContentStreamHandlerBase;
-import org.apache.solr.logging.MDCLoggingContext;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrQueryRequestBase;
@@ -504,9 +503,6 @@ public class HttpSolrCall {
* This method processes the request.
*/
public Action call() throws IOException {
- MDCLoggingContext.reset();
- MDCLoggingContext.setTracerId(getSpan().context().toTraceId()); // handles
empty string
- MDCLoggingContext.setNode(cores);
if (cores == null) {
sendError(503, "Server is shutting down or failed to initialize");
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 2816727..93a5897 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -80,6 +80,8 @@ import org.apache.solr.core.NodeConfig;
import org.apache.solr.core.SolrCore;
import org.apache.solr.core.SolrInfoBean;
import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.logging.MDCLoggingContext;
+import org.apache.solr.logging.MDCSnapshot;
import org.apache.solr.metrics.AltBufferPoolMetricSet;
import org.apache.solr.metrics.MetricsMap;
import org.apache.solr.metrics.OperatingSystemMetricSet;
@@ -430,7 +432,13 @@ public class SolrDispatchFilter extends BaseSolrFilter {
@Override
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException {
- doFilter(request, response, chain, false);
+ try (var mdcSnapshot = MDCSnapshot.create()) {
+ assert null != mdcSnapshot; // prevent compiler warning
+ MDCLoggingContext.reset();
+ MDCLoggingContext.setNode(cores);
+
+ doFilter(request, response, chain, false);
+ }
}
public void doFilter(ServletRequest _request, ServletResponse _response,
FilterChain chain, boolean retry) throws IOException, ServletException {
@@ -457,6 +465,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
boolean accepted = false;
try (var scope = tracer.scopeManager().activate(span)) {
assert scope != null; // prevent javac warning about scope being unused
+ MDCLoggingContext.setTracerId(span.context().toTraceId()); // handles
empty string
if (cores == null || cores.isShutDown()) {
try {
diff --git a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
index b4c4a31..67f7aeb 100644
--- a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
+++ b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
@@ -45,6 +45,7 @@ import org.apache.solr.core.SolrConfig;
import org.apache.solr.core.SolrCore;
import org.apache.solr.core.SolrXmlConfig;
import org.apache.solr.handler.UpdateRequestHandler;
+import org.apache.solr.logging.MDCSnapshot;
import org.apache.solr.metrics.reporters.SolrJmxReporter;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
@@ -278,7 +279,9 @@ public class TestHarness extends BaseTestHarness {
* @return The XML response to the update
*/
public String update(String xml) {
- try (SolrCore core = getCoreInc()) {
+ try (var mdcSnap = MDCSnapshot.create();
+ SolrCore core = getCoreInc()) {
+ assert null != mdcSnap; // prevent compiler warning of unused var
DirectSolrConnection connection = new DirectSolrConnection(core);
SolrRequestHandler handler = core.getRequestHandler("/update");
// prefer the handler mapped to /update, but use our generic backup
handler
@@ -335,7 +338,8 @@ public class TestHarness extends BaseTestHarness {
* @see LocalSolrQueryRequest
*/
public String query(String handler, SolrQueryRequest req) throws Exception {
- try {
+ try (var mdcSnap = MDCSnapshot.create()) {
+ assert null != mdcSnap; // prevent compiler warning of unused var
SolrCore core = req.getCore();
SolrQueryResponse rsp = new SolrQueryResponse();
SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp));
@@ -364,7 +368,9 @@ public class TestHarness extends BaseTestHarness {
/** It is the users responsibility to close the request object when done
with it.
* This method does not set/clear SolrRequestInfo */
public SolrQueryResponse queryAndResponse(String handler, SolrQueryRequest
req) throws Exception {
- try (SolrCore core = getCoreInc()) {
+ try (var mdcSnap = MDCSnapshot.create();
+ SolrCore core = getCoreInc()) {
+ assert null != mdcSnap; // prevent compiler warning of unused var
SolrQueryResponse rsp = new SolrQueryResponse();
core.execute(core.getRequestHandler(handler), req, rsp);
if (rsp.getException() != null) {
