From 44860bd48a31c76e11808d914be016df7196596a Mon Sep 17 00:00:00 2001 From: Christine Poerschke Date: Mon, 26 Jun 2017 16:26:37 +0100 Subject: [PATCH] SOLR-10823: Add reporting period to SolrMetricReporter base class. --- solr/CHANGES.txt | 2 ++ .../apache/solr/metrics/SolrMetricReporter.java | 15 +++++++++++++++ .../metrics/reporters/SolrGangliaReporter.java | 5 ----- .../metrics/reporters/SolrGraphiteReporter.java | 5 ----- .../solr/metrics/reporters/SolrJmxReporter.java | 5 ++++- .../solr/metrics/reporters/SolrSlf4jReporter.java | 5 ----- .../reporters/solr/SolrClusterReporter.java | 12 +----------- .../metrics/reporters/solr/SolrShardReporter.java | 12 +----------- .../solr/metrics/SolrMetricManagerTest.java | 5 +++++ .../metrics/reporters/MockMetricReporter.java | 3 +++ 10 files changed, 31 insertions(+), 38 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 800b645d590..5b82f1e558f 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -517,6 +517,8 @@ Other Changes with point fields and provides control over dynamic fields used for the raw amount and currency code sub-fields. (hossman, Steve Rowe) +* SOLR-10823: Add reporting period to SolrMetricReporter base class. (Christine Poerschke) + ================== 6.6.1 ================== Bug Fixes diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricReporter.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricReporter.java index 222377b3c1b..3f7f73cba30 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricReporter.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricReporter.java @@ -36,6 +36,7 @@ public abstract class SolrMetricReporter implements Closeable, PluginInfoInitial protected final SolrMetricManager metricManager; protected PluginInfo pluginInfo; protected boolean enabled = true; + protected int period = SolrMetricManager.DEFAULT_CLOUD_REPORTER_PERIOD; /** * Create a reporter for metrics managed in a named registry. @@ -85,6 +86,20 @@ public abstract class SolrMetricReporter implements Closeable, PluginInfoInitial } } + /** + * @param period - in seconds + */ + public void setPeriod(int period) { + this.period = period; + } + + /** + * @return period, in seconds + */ + public int getPeriod() { + return period; + } + /** * Get the effective {@link PluginInfo} instance that was used for * initialization of this plugin. 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 8d879401bfd..bf95ef422b9 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 @@ -35,7 +35,6 @@ public class SolrGangliaReporter extends SolrMetricReporter { private String host = null; private int port = -1; private boolean multicast; - private int period = 60; private String instancePrefix = null; private List filters = new ArrayList<>(); private boolean testing; @@ -88,10 +87,6 @@ public class SolrGangliaReporter extends SolrMetricReporter { } } - public void setPeriod(int period) { - this.period = period; - } - public void setMulticast(boolean multicast) { this.multicast = multicast; } 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 e74bb354375..c0977186e45 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 @@ -36,7 +36,6 @@ public class SolrGraphiteReporter extends SolrMetricReporter { private String host = null; private int port = -1; - private int period = 60; private boolean pickled = false; private String instancePrefix = null; private List filters = new ArrayList<>(); @@ -90,10 +89,6 @@ public class SolrGraphiteReporter extends SolrMetricReporter { this.pickled = pickled; } - public void setPeriod(int period) { - this.period = period; - } - @Override protected void doInit() { if (reporter != null) { 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 68a94ef7d1d..170dd338960 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 @@ -70,6 +70,7 @@ public class SolrJmxReporter extends SolrMetricReporter { */ public SolrJmxReporter(SolrMetricManager metricManager, String registryName) { super(metricManager, registryName); + period = 0; // setting to zero to indicate not applicable setDomain(registryName); } @@ -151,7 +152,9 @@ public class SolrJmxReporter extends SolrMetricReporter { */ @Override protected void validate() throws IllegalStateException { - // Nothing to validate + if (period != 0) { + throw new IllegalStateException("Init argument 'period' is not supported for "+getClass().getCanonicalName()); + } } 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 e3018852dde..e2c6015b173 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 @@ -47,7 +47,6 @@ public class SolrSlf4jReporter extends SolrMetricReporter { @SuppressWarnings("unused") // we need this to pass validate-source-patterns private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private int period = 60; private String instancePrefix = null; private String logger = null; private List filters = new ArrayList<>(); @@ -91,10 +90,6 @@ public class SolrSlf4jReporter extends SolrMetricReporter { this.logger = logger; } - public void setPeriod(int period) { - this.period = period; - } - @Override protected void doInit() { if (instancePrefix == null) { diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java index 1e64b8a5876..d8714d497d1 100644 --- a/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java +++ b/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java @@ -124,7 +124,6 @@ public class SolrClusterReporter extends SolrMetricReporter { }}; private String handler = MetricsCollectorHandler.HANDLER_PATH; - private int period = SolrMetricManager.DEFAULT_CLOUD_REPORTER_PERIOD; private List reports = new ArrayList<>(); private SolrReporter reporter; @@ -143,10 +142,6 @@ public class SolrClusterReporter extends SolrMetricReporter { this.handler = handler; } - public void setPeriod(int period) { - this.period = period; - } - public void setReport(List reportConfig) { if (reportConfig == null || reportConfig.isEmpty()) { return; @@ -169,11 +164,6 @@ public class SolrClusterReporter extends SolrMetricReporter { } } - // for unit tests - int getPeriod() { - return period; - } - List getReports() { return reports; } @@ -187,7 +177,7 @@ public class SolrClusterReporter extends SolrMetricReporter { @Override protected void validate() throws IllegalStateException { - // Nothing to validate + // (period < 1) means "don't start reporter" and so no (period > 0) validation needed } @Override 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 b3b055d9f0b..e9710b88d43 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 @@ -70,7 +70,6 @@ public class SolrShardReporter extends SolrMetricReporter { }}; private String handler = MetricsCollectorHandler.HANDLER_PATH; - private int period = SolrMetricManager.DEFAULT_CLOUD_REPORTER_PERIOD; private List filters = new ArrayList<>(); private SolrReporter reporter; @@ -90,10 +89,6 @@ public class SolrShardReporter extends SolrMetricReporter { this.handler = handler; } - public void setPeriod(int period) { - this.period = period; - } - public void setFilter(List filterConfig) { if (filterConfig == null || filterConfig.isEmpty()) { return; @@ -107,11 +102,6 @@ public class SolrShardReporter extends SolrMetricReporter { } } - // for unit tests - int getPeriod() { - return period; - } - @Override protected void doInit() { if (filters.isEmpty()) { @@ -122,7 +112,7 @@ public class SolrShardReporter extends SolrMetricReporter { @Override protected void validate() throws IllegalStateException { - // Nothing to validate + // (period < 1) means "don't start reporter" and so no (period > 0) validation needed } @Override diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java index d30611904c3..5739e53454e 100644 --- a/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java +++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java @@ -249,6 +249,11 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 { } + @Test + public void testDefaultCloudReporterPeriodUnchanged() throws Exception { + assertEquals(60, SolrMetricManager.DEFAULT_CLOUD_REPORTER_PERIOD); + } + private PluginInfo createPluginInfo(String name, String group, String registry) { Map attrs = new HashMap<>(); attrs.put("name", name); diff --git a/solr/core/src/test/org/apache/solr/metrics/reporters/MockMetricReporter.java b/solr/core/src/test/org/apache/solr/metrics/reporters/MockMetricReporter.java index e6110ba50ae..f815c7c694f 100644 --- a/solr/core/src/test/org/apache/solr/metrics/reporters/MockMetricReporter.java +++ b/solr/core/src/test/org/apache/solr/metrics/reporters/MockMetricReporter.java @@ -53,6 +53,9 @@ public class MockMetricReporter extends SolrMetricReporter { if (configurable == null) { throw new IllegalStateException("MockMetricReporter::configurable not defined."); } + if (period < 1) { + throw new IllegalStateException("Init argument 'period' is in time unit 'seconds' and must be at least 1."); + } } public void setConfigurable(String configurable) {