From d3c67cf5e4c4f1809e7c7ff921c55562fa6cb13f Mon Sep 17 00:00:00 2001 From: Christine Poerschke Date: Tue, 4 Jul 2017 11:18:48 +0100 Subject: [PATCH] SOLR-10827: Factor out abstract FilteringSolrMetricReporter class. --- solr/CHANGES.txt | 2 +- .../metrics/FilteringSolrMetricReporter.java | 59 +++++++++++++++++++ .../reporters/SolrGangliaReporter.java | 34 ++--------- .../reporters/SolrGraphiteReporter.java | 34 ++--------- .../metrics/reporters/SolrJmxReporter.java | 51 ++++++---------- .../metrics/reporters/SolrSlf4jReporter.java | 34 ++--------- .../reporters/solr/SolrShardReporter.java | 26 ++++---- 7 files changed, 101 insertions(+), 139 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/metrics/FilteringSolrMetricReporter.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 403b6cd27e8..8c5f2c1ba1a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -72,7 +72,7 @@ Optimizations Other Changes ---------------------- -(No Changes) +* SOLR-10827: Factor out abstract FilteringSolrMetricReporter class. (Christine Poerschke) ================== 7.0.0 ================== diff --git a/solr/core/src/java/org/apache/solr/metrics/FilteringSolrMetricReporter.java b/solr/core/src/java/org/apache/solr/metrics/FilteringSolrMetricReporter.java new file mode 100644 index 00000000000..5f29f8e45d0 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/metrics/FilteringSolrMetricReporter.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.metrics; + +import java.util.ArrayList; +import java.util.List; + +import com.codahale.metrics.MetricFilter; + +public abstract class FilteringSolrMetricReporter extends SolrMetricReporter { + + protected List filters = new ArrayList<>(); + + public FilteringSolrMetricReporter(SolrMetricManager metricManager, String registryName) { + super(metricManager, registryName); + } + + public void setFilter(List filters) { + if (filters == null || filters.isEmpty()) { + return; + } + this.filters.addAll(filters); + } + + public void setFilter(String filter) { + if (filter != null && !filter.isEmpty()) { + this.filters.add(filter); + } + } + + /** + * Report only metrics with names matching any of the prefix filters. + * If the filters list is empty then all names will match. + */ + protected MetricFilter newMetricFilter() { + final MetricFilter filter; + if (!filters.isEmpty()) { + filter = new SolrMetricManager.PrefixFilter(filters); + } else { + filter = MetricFilter.ALL; + } + return filter; + } + +} diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrGangliaReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrGangliaReporter.java index bf95ef422b9..8d77a023c63 100644 --- a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrGangliaReporter.java +++ b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrGangliaReporter.java @@ -17,26 +17,24 @@ package org.apache.solr.metrics.reporters; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; import java.util.concurrent.TimeUnit; import com.codahale.metrics.MetricFilter; import com.codahale.metrics.ganglia.GangliaReporter; import info.ganglia.gmetric4j.gmetric.GMetric; + +import org.apache.solr.metrics.FilteringSolrMetricReporter; import org.apache.solr.metrics.SolrMetricManager; -import org.apache.solr.metrics.SolrMetricReporter; /** * */ -public class SolrGangliaReporter extends SolrMetricReporter { +public class SolrGangliaReporter extends FilteringSolrMetricReporter { private String host = null; private int port = -1; private boolean multicast; private String instancePrefix = null; - private List filters = new ArrayList<>(); private boolean testing; private GangliaReporter reporter; @@ -68,25 +66,6 @@ public class SolrGangliaReporter extends SolrMetricReporter { this.instancePrefix = prefix; } - /** - * Report only metrics with names matching any of the prefix filters. - * @param filters list of 0 or more prefixes. If the list is empty then - * all names will match. - */ - public void setFilter(List filters) { - if (filters == null || filters.isEmpty()) { - return; - } - this.filters.addAll(filters); - } - - // due to vagaries of SolrPluginUtils.invokeSetters we need this too - public void setFilter(String filter) { - if (filter != null && !filter.isEmpty()) { - this.filters.add(filter); - } - } - public void setMulticast(boolean multicast) { this.multicast = multicast; } @@ -141,12 +120,7 @@ public class SolrGangliaReporter extends SolrMetricReporter { .convertRatesTo(TimeUnit.SECONDS) .convertDurationsTo(TimeUnit.MILLISECONDS) .prefixedWith(instancePrefix); - MetricFilter filter; - if (!filters.isEmpty()) { - filter = new SolrMetricManager.PrefixFilter(filters); - } else { - filter = MetricFilter.ALL; - } + final MetricFilter filter = newMetricFilter(); builder = builder.filter(filter); reporter = builder.build(ganglia); reporter.start(period, TimeUnit.SECONDS); diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrGraphiteReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrGraphiteReporter.java index c0977186e45..68111d59368 100644 --- a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrGraphiteReporter.java +++ b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrGraphiteReporter.java @@ -17,8 +17,6 @@ package org.apache.solr.metrics.reporters; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; import java.util.concurrent.TimeUnit; import com.codahale.metrics.MetricFilter; @@ -26,19 +24,19 @@ import com.codahale.metrics.graphite.Graphite; import com.codahale.metrics.graphite.GraphiteReporter; import com.codahale.metrics.graphite.GraphiteSender; import com.codahale.metrics.graphite.PickledGraphite; + +import org.apache.solr.metrics.FilteringSolrMetricReporter; import org.apache.solr.metrics.SolrMetricManager; -import org.apache.solr.metrics.SolrMetricReporter; /** * Metrics reporter that wraps {@link com.codahale.metrics.graphite.GraphiteReporter}. */ -public class SolrGraphiteReporter extends SolrMetricReporter { +public class SolrGraphiteReporter extends FilteringSolrMetricReporter { private String host = null; private int port = -1; private boolean pickled = false; private String instancePrefix = null; - private List filters = new ArrayList<>(); private GraphiteReporter reporter = null; private static final ReporterClientCache serviceRegistry = new ReporterClientCache<>(); @@ -66,25 +64,6 @@ public class SolrGraphiteReporter extends SolrMetricReporter { this.instancePrefix = prefix; } - /** - * Report only metrics with names matching any of the prefix filters. - * @param filters list of 0 or more prefixes. If the list is empty then - * all names will match. - */ - public void setFilter(List filters) { - if (filters == null || filters.isEmpty()) { - return; - } - this.filters.addAll(filters); - } - - public void setFilter(String filter) { - if (filter != null && !filter.isEmpty()) { - this.filters.add(filter); - } - } - - public void setPickled(boolean pickled) { this.pickled = pickled; } @@ -113,12 +92,7 @@ public class SolrGraphiteReporter extends SolrMetricReporter { .prefixedWith(instancePrefix) .convertRatesTo(TimeUnit.SECONDS) .convertDurationsTo(TimeUnit.MILLISECONDS); - MetricFilter filter; - if (!filters.isEmpty()) { - filter = new SolrMetricManager.PrefixFilter(filters); - } else { - filter = MetricFilter.ALL; - } + final MetricFilter filter = newMetricFilter(); builder = builder.filter(filter); reporter = builder.build(graphite); reporter.start(period, TimeUnit.SECONDS); diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java index 170dd338960..b7504b80214 100644 --- a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java +++ b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java @@ -22,9 +22,7 @@ import javax.management.ObjectInstance; import javax.management.ObjectName; import java.lang.invoke.MethodHandles; -import java.util.ArrayList; import java.util.HashSet; -import java.util.List; import java.util.Locale; import java.util.Set; @@ -33,6 +31,8 @@ import com.codahale.metrics.JmxReporter; import com.codahale.metrics.MetricFilter; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.MetricRegistryListener; + +import org.apache.solr.metrics.FilteringSolrMetricReporter; import org.apache.solr.metrics.MetricsMap; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricReporter; @@ -46,7 +46,7 @@ import org.slf4j.LoggerFactory; *

NOTE: {@link JmxReporter} that this class uses exports only newly added metrics (it doesn't * process already existing metrics in a registry)

*/ -public class SolrJmxReporter extends SolrMetricReporter { +public class SolrJmxReporter extends FilteringSolrMetricReporter { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -56,7 +56,6 @@ public class SolrJmxReporter extends SolrMetricReporter { private String agentId; private String serviceUrl; private String rootName; - private List filters = new ArrayList<>(); private JmxReporter reporter; private MetricRegistry registry; @@ -103,16 +102,7 @@ public class SolrJmxReporter extends SolrMetricReporter { } JmxObjectNameFactory jmxObjectNameFactory = new JmxObjectNameFactory(pluginInfo.name, fullDomain); registry = metricManager.registry(registryName); - // filter out MetricsMap gauges - we have a better way of handling them - MetricFilter mmFilter = (name, metric) -> !(metric instanceof MetricsMap); - MetricFilter filter; - if (filters.isEmpty()) { - filter = mmFilter; - } else { - // apply also prefix filters - SolrMetricManager.PrefixFilter prefixFilter = new SolrMetricManager.PrefixFilter(filters); - filter = new SolrMetricManager.AndFilter(prefixFilter, mmFilter); - } + final MetricFilter filter = newMetricFilter(); reporter = JmxReporter.forRegistry(registry) .registerWith(mBeanServer) @@ -128,6 +118,21 @@ public class SolrJmxReporter extends SolrMetricReporter { log.info("JMX monitoring for '" + fullDomain + "' (registry '" + registryName + "') enabled at server: " + mBeanServer); } + @Override + protected MetricFilter newMetricFilter() { + // filter out MetricsMap gauges - we have a better way of handling them + final MetricFilter mmFilter = (name, metric) -> !(metric instanceof MetricsMap); + final MetricFilter filter; + if (filters.isEmpty()) { + filter = mmFilter; + } else { + // apply also prefix filters + SolrMetricManager.PrefixFilter prefixFilter = new SolrMetricManager.PrefixFilter(filters); + filter = new SolrMetricManager.AndFilter(prefixFilter, mmFilter); + } + return filter; + } + /** * Stops the reporter from publishing metrics. */ @@ -222,24 +227,6 @@ public class SolrJmxReporter extends SolrMetricReporter { return domain; } - /** - * Report only metrics with names matching any of the prefix filters. - * @param filters list of 0 or more prefixes. If the list is empty then - * all names will match. - */ - public void setFilter(List filters) { - if (filters == null || filters.isEmpty()) { - return; - } - this.filters.addAll(filters); - } - - public void setFilter(String filter) { - if (filter != null && !filter.isEmpty()) { - this.filters.add(filter); - } - } - /** * Return the reporter's MBeanServer. * 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 e2c6015b173..930bd31cb13 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,13 @@ package org.apache.solr.metrics.reporters; import java.io.IOException; import java.lang.invoke.MethodHandles; -import java.util.ArrayList; -import java.util.List; import java.util.concurrent.TimeUnit; import com.codahale.metrics.MetricFilter; import com.codahale.metrics.Slf4jReporter; + +import org.apache.solr.metrics.FilteringSolrMetricReporter; import org.apache.solr.metrics.SolrMetricManager; -import org.apache.solr.metrics.SolrMetricReporter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,14 +41,13 @@ import org.slf4j.LoggerFactory; * metrics group, eg. solr.jvm * */ -public class SolrSlf4jReporter extends SolrMetricReporter { +public class SolrSlf4jReporter extends FilteringSolrMetricReporter { @SuppressWarnings("unused") // we need this to pass validate-source-patterns private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private String instancePrefix = null; private String logger = null; - private List filters = new ArrayList<>(); private Slf4jReporter reporter; /** @@ -67,25 +65,6 @@ public class SolrSlf4jReporter extends SolrMetricReporter { this.instancePrefix = prefix; } - /** - * Report only metrics with names matching any of the prefix filters. - * @param filters list of 0 or more prefixes. If the list is empty then - * all names will match. - */ - public void setFilter(List filters) { - if (filters == null || filters.isEmpty()) { - return; - } - this.filters.addAll(filters); - } - - public void setFilter(String filter) { - if (filter != null && !filter.isEmpty()) { - this.filters.add(filter); - } - } - - public void setLogger(String logger) { this.logger = logger; } @@ -102,12 +81,7 @@ public class SolrSlf4jReporter extends SolrMetricReporter { .convertRatesTo(TimeUnit.SECONDS) .convertDurationsTo(TimeUnit.MILLISECONDS); - MetricFilter filter; - if (!filters.isEmpty()) { - filter = new SolrMetricManager.PrefixFilter(filters); - } else { - filter = MetricFilter.ALL; - } + final MetricFilter filter = newMetricFilter(); builder = builder.filter(filter); if (logger == null || logger.isEmpty()) { // construct logger name from Group diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrShardReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrShardReporter.java index e9710b88d43..086c81289a3 100644 --- a/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrShardReporter.java +++ b/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrShardReporter.java @@ -30,11 +30,13 @@ import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.Replica; import org.apache.solr.core.SolrCore; import org.apache.solr.handler.admin.MetricsCollectorHandler; +import org.apache.solr.metrics.FilteringSolrMetricReporter; import org.apache.solr.metrics.SolrMetricManager; -import org.apache.solr.metrics.SolrMetricReporter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.codahale.metrics.MetricFilter; + /** * This class reports selected metrics from replicas to shard leader. *

The following configuration properties are supported:

@@ -56,7 +58,7 @@ import org.slf4j.LoggerFactory; * </reporter> * */ -public class SolrShardReporter extends SolrMetricReporter { +public class SolrShardReporter extends FilteringSolrMetricReporter { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); public static final List DEFAULT_FILTERS = new ArrayList(){{ @@ -70,7 +72,6 @@ public class SolrShardReporter extends SolrMetricReporter { }}; private String handler = MetricsCollectorHandler.HANDLER_PATH; - private List filters = new ArrayList<>(); private SolrReporter reporter; @@ -89,19 +90,6 @@ public class SolrShardReporter extends SolrMetricReporter { this.handler = handler; } - public void setFilter(List filterConfig) { - if (filterConfig == null || filterConfig.isEmpty()) { - return; - } - filters.addAll(filterConfig); - } - - public void setFilter(String filter) { - if (filter != null && !filter.isEmpty()) { - this.filters.add(filter); - } - } - @Override protected void doInit() { if (filters.isEmpty()) { @@ -110,6 +98,12 @@ public class SolrShardReporter extends SolrMetricReporter { // start in setCore(SolrCore) when core is available } + @Override + protected MetricFilter newMetricFilter() { + // unsupported here since setCore(SolrCore) directly uses the this.filters + throw new UnsupportedOperationException(getClass().getCanonicalName()+".newMetricFilter() is not supported"); + } + @Override protected void validate() throws IllegalStateException { // (period < 1) means "don't start reporter" and so no (period > 0) validation needed