lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a.@apache.org
Subject lucene-solr:branch_7x: SOLR-12344: SolrSlf4jReporter doesn't set MDC context.
Date Thu, 02 Aug 2018 13:02:26 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/branch_7x 600c15d14 -> 32e417959


SOLR-12344: SolrSlf4jReporter doesn't set MDC context.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/32e41795
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/32e41795
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/32e41795

Branch: refs/heads/branch_7x
Commit: 32e417959a8098505812b87a275844f2cc98b1d1
Parents: 600c15d
Author: Andrzej Bialecki <ab@apache.org>
Authored: Thu Aug 2 14:29:19 2018 +0200
Committer: Andrzej Bialecki <ab@apache.org>
Committed: Thu Aug 2 14:32:03 2018 +0200

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 +
 .../apache/solr/metrics/SolrMetricManager.java  | 14 +++++
 .../metrics/reporters/SolrSlf4jReporter.java    | 64 +++++++++++++++++---
 .../src/test-files/solr/solr-slf4jreporter.xml  |  7 +++
 .../reporters/SolrSlf4jReporterTest.java        |  5 +-
 solr/solr-ref-guide/src/metrics-reporting.adoc  | 29 ++++++++-
 6 files changed, 109 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/32e41795/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0f4f60a..e04048d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -168,6 +168,8 @@ Bug Fixes
   even though they remain the transaction log. The second case is when synchronously forwarding
updates to sub-shard
   leader fails and the underlying errors are not propagated to the client. (Cao Manh Dat,
shalin)
 
+* SOLR-12344: SolrSlf4jReporter doesn't set MDC context. (ab)
+
 Optimizations
 ----------------------
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/32e41795/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
index 5fa3659..f1b7923 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -55,8 +55,10 @@ import org.apache.solr.core.PluginInfo;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrInfoBean;
 import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.logging.MDCLoggingContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.slf4j.MDC;
 
 /**
  * This class maintains a repository of named {@link MetricRegistry} instances, and provides
several
@@ -921,6 +923,15 @@ public class SolrMetricManager {
         new Class[]{SolrMetricManager.class, String.class},
         new Object[]{this, registry}
     );
+    // prepare MDC for plugins that want to use its properties
+    MDCLoggingContext.setNode(coreContainer);
+    if (solrCore != null) {
+      MDCLoggingContext.setCore(solrCore);
+    }
+    if (tag != null) {
+      // add instance tag to MDC
+      MDC.put("tag", "t:" + tag);
+    }
     try {
       if (reporter instanceof SolrCoreReporter) {
         ((SolrCoreReporter)reporter).init(pluginInfo, solrCore);
@@ -931,6 +942,9 @@ public class SolrMetricManager {
       }
     } catch (IllegalStateException e) {
       throw new IllegalArgumentException("reporter init failed: " + pluginInfo, e);
+    } finally {
+      MDCLoggingContext.clear();
+      MDC.remove("tag");
     }
     registerReporter(registry, pluginInfo.name, tag, reporter);
   }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/32e41795/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java
index 26338c1..ff00a00 100644
--- a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java
+++ b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java
@@ -18,15 +18,23 @@ package org.apache.solr.metrics.reporters;
 
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.util.Map;
+import java.util.SortedMap;
 import java.util.concurrent.TimeUnit;
 
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.Meter;
 import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.ScheduledReporter;
 import com.codahale.metrics.Slf4jReporter;
-
+import com.codahale.metrics.Timer;
 import org.apache.solr.metrics.FilteringSolrMetricReporter;
 import org.apache.solr.metrics.SolrMetricManager;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.slf4j.MDC;
 
 /**
  * Metrics reporter that wraps {@link com.codahale.metrics.Slf4jReporter}.
@@ -38,7 +46,7 @@ import org.slf4j.LoggerFactory;
  *   <li><code>filter</code>: (optional, str) if not empty only metric
names that start
  *   with this value will be reported, default is all metrics from a registry,</li>
  *   <li><code>logger</code>: (optional, str) logger name to use. Default
is the
- *   metrics group, eg. <code>solr.jvm</code></li>
+ *   metrics group, eg. <code>solr.jvm</code>, <code>solr.core</code>,
etc</li>
  * </ul>
  */
 public class SolrSlf4jReporter extends FilteringSolrMetricReporter {
@@ -48,9 +56,45 @@ public class SolrSlf4jReporter extends FilteringSolrMetricReporter {
 
   private String instancePrefix = null;
   private String logger = null;
-  private Slf4jReporter reporter;
+  private Map<String, String> mdcContext;
+  private Slf4jReporterWrapper reporter;
   private boolean active;
 
+  // this wrapper allows us to set MDC context - unfortunately it's not possible to
+  // simply override {@link Slf4jReporter#report()} because its constructor is private
+  private class Slf4jReporterWrapper extends ScheduledReporter {
+    final Slf4jReporter delegate;
+    final Map<String, String> mdcContext;
+
+    Slf4jReporterWrapper(String logger, Map<String, String> mdcContext, Slf4jReporter
delegate, TimeUnit rateUnit, TimeUnit durationUnit) {
+      super(null, logger, null, rateUnit, durationUnit);
+      this.delegate = delegate;
+      this.mdcContext = mdcContext;
+    }
+
+    @Override
+    public void report() {
+      // set up MDC
+      MDC.setContextMap(mdcContext);
+      try {
+        delegate.report();
+      } finally {
+        // clear MDC
+        MDC.clear();
+      }
+    }
+
+    @Override
+    public void report(SortedMap<String, Gauge> gauges, SortedMap<String, Counter>
counters, SortedMap<String, Histogram> histograms, SortedMap<String, Meter> meters,
SortedMap<String, Timer> timers) {
+      throw new UnsupportedOperationException("this method should never be called here!");
+    }
+
+    @Override
+    public void close() {
+      super.close();
+      delegate.close();
+    }
+  }
   /**
    * Create a SLF4J reporter for metrics managed in a named registry.
    *
@@ -72,11 +116,8 @@ public class SolrSlf4jReporter extends FilteringSolrMetricReporter {
 
   @Override
   protected void doInit() {
-    if (instancePrefix == null) {
-      instancePrefix = registryName;
-    } else {
-      instancePrefix = instancePrefix + "." + registryName;
-    }
+    mdcContext = MDC.getCopyOfContextMap();
+    mdcContext.put("registry", "m:" + registryName);
     Slf4jReporter.Builder builder = Slf4jReporter
         .forRegistry(metricManager.registry(registryName))
         .convertRatesTo(TimeUnit.SECONDS)
@@ -84,6 +125,9 @@ public class SolrSlf4jReporter extends FilteringSolrMetricReporter {
 
     final MetricFilter filter = newMetricFilter();
     builder = builder.filter(filter);
+    if (instancePrefix != null) {
+      builder = builder.prefixedWith(instancePrefix);
+    }
     if (logger == null || logger.isEmpty()) {
       // construct logger name from Group
       if (pluginInfo.attributes.containsKey("group")) {
@@ -99,7 +143,9 @@ public class SolrSlf4jReporter extends FilteringSolrMetricReporter {
       }
     }
     builder = builder.outputTo(LoggerFactory.getLogger(logger));
-    reporter = builder.build();
+    // build BUT don't start - scheduled execution is handled by the wrapper
+    Slf4jReporter delegate = builder.build();
+    reporter = new Slf4jReporterWrapper(logger, mdcContext, delegate, TimeUnit.SECONDS, TimeUnit.MILLISECONDS);
     reporter.start(period, TimeUnit.SECONDS);
     active = true;
   }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/32e41795/solr/core/src/test-files/solr/solr-slf4jreporter.xml
----------------------------------------------------------------------
diff --git a/solr/core/src/test-files/solr/solr-slf4jreporter.xml b/solr/core/src/test-files/solr/solr-slf4jreporter.xml
index f3144ca..6b91fe5 100644
--- a/solr/core/src/test-files/solr/solr-slf4jreporter.xml
+++ b/solr/core/src/test-files/solr/solr-slf4jreporter.xml
@@ -31,5 +31,12 @@
     <str name="filter">CONTAINER.cores</str>
     <str name="logger">foobar</str>
   </reporter>
+   <reporter name="test2" group="core" class="org.apache.solr.metrics.reporters.SolrSlf4jReporter">
+     <!-- for unit tests this is set to 1 second - DO NOT USE THIS VALUE IN PRODUCTION!
-->
+     <int name="period">1</int>
+     <str name="prefix">test</str>
+     <str name="filter">INDEX.sizeInBytes</str>
+     <str name="logger">foobar</str>
+   </reporter>
  </metrics>
 </solr>

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/32e41795/solr/core/src/test/org/apache/solr/metrics/reporters/SolrSlf4jReporterTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/metrics/reporters/SolrSlf4jReporterTest.java
b/solr/core/src/test/org/apache/solr/metrics/reporters/SolrSlf4jReporterTest.java
index 4cad788..7646864 100644
--- a/solr/core/src/test/org/apache/solr/metrics/reporters/SolrSlf4jReporterTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/reporters/SolrSlf4jReporterTest.java
@@ -82,7 +82,7 @@ public class SolrSlf4jReporterTest extends SolrTestCaseJ4 {
     if (!active) {
       fail("One or more reporters didn't become active in 20 seconds");
     }
-    Thread.sleep(5000);
+    Thread.sleep(10000);
 
     SolrDocumentList history = watcher.getHistory(-1, null);
     // dot-separated names are treated like class names and collapsed
@@ -93,6 +93,9 @@ public class SolrSlf4jReporterTest extends SolrTestCaseJ4 {
     if (history.stream().filter(d -> "foobar".equals(d.getFirstValue("logger"))).count()
== 0) {
       fail("No 'foobar' logs in: " + history.toString());
     }
+    if (history.stream().filter(d -> "x:collection1".equals(d.getFirstValue("core"))).count()
== 0) {
+      fail("No 'solr.core' or MDC context in logs: " + history.toString());
+    }
   }
 
   private static void ensureLoggingConfiguredAppropriately() throws Exception {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/32e41795/solr/solr-ref-guide/src/metrics-reporting.adoc
----------------------------------------------------------------------
diff --git a/solr/solr-ref-guide/src/metrics-reporting.adoc b/solr/solr-ref-guide/src/metrics-reporting.adoc
index f66df3e..631b5a1 100644
--- a/solr/solr-ref-guide/src/metrics-reporting.adoc
+++ b/solr/solr-ref-guide/src/metrics-reporting.adoc
@@ -244,7 +244,7 @@ The SLF4J Reporter uses the `org.apache.solr.metrics.reporters.SolrSlf4jReporter
 It takes the following arguments, in addition to the common arguments <<Reporter Arguments,above>>.
 
 `logger`::
-The name of the logger to use. Default is empty, in which case the group or registry name
will be used if specified in the plugin configuration.
+The name of the logger to use. Default is empty, in which case the group (or the initial
part of the registry name that identifies a metrics group) will be used if specified in the
plugin configuration.
 
 Users can specify logger name (and the corresponding logger configuration in e.g., Log4j
configuration) to output metrics-related logging to separate file(s), which can then be processed
by external applications.
 
@@ -263,7 +263,32 @@ type=METER, name={}, count={}, mean_rate={}, m1={}, m5={}, m15={}, rate_unit={}
 type=HISTOGRAM, name={}, count={}, min={}, max={}, mean={}, stddev={}, median={}, p75={},
p95={}, p98={}, p99={}, p999={}
 ----
 
-(curly braces added only as placeholders for actual values).
+(curly braces added here only as placeholders for actual values).
+
+Additionally, the following MDC context properties are passed to the logger and can be used
in log formats:
+
+`node_name`::
+Solr node name (for SolrCloud deployments, otherwise null), prefixed with `n:`.
+
+`registry`::
+Metric registry name, prefixed with `m:`.
+
+For reporters that are specific to a SolrCore also the following properties are available:
+
+`collection`::
+Collection name, prefixed with `c:`.
+
+`shard`::
+Shard name, prefixed with `s:`.
+
+`replica`::
+Replica name (core node name), prefixed with `r:`.
+
+`core`::
+SolrCore name, prefixed with `x:`.
+
+`tag`::
+Reporter instance tag, prefixed with `t:`.
 
 === Graphite Reporter
 


Mime
View raw message