You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by da...@apache.org on 2018/08/06 04:16:04 UTC
[31/48] lucene-solr:jira/http2: SOLR-12344: SolrSlf4jReporter doesn't
set MDC context.
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/5de10c79
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/5de10c79
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/5de10c79
Branch: refs/heads/jira/http2
Commit: 5de10c79668bf786d9699db992bf85e2f4beb8b4
Parents: 868e970
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:29:47 2018 +0200
----------------------------------------------------------------------
solr/CHANGES.txt | 2 +
.../apache/solr/metrics/SolrMetricManager.java | 14 +++++
.../metrics/reporters/SolrSlf4jReporter.java | 63 +++++++++++++++++---
.../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(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/5de10c79/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 93171af..1b45436 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -196,6 +196,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/5de10c79/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/5de10c79/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 fdb0e2a..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,14 +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}.
@@ -37,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 {
@@ -47,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.
*
@@ -71,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)
@@ -83,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")) {
@@ -98,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/5de10c79/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/5de10c79/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/5de10c79/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