Make the hostname mandatory in the configuration
If the plugin is installed without any configuration it defaults the
hostname to localhost, assuming that Graphite is running there.
In deployments where the plugin is bundled with Gerrit, but Graphite is
not necessarily by default on the same host, this causes the logs to be
spammed with "connection refused" errors.
Make the hostname mandatory, so that if the Graphite server is actually
running at localhost it must be explicitly set in the config.
Change-Id: I3a7651f229626e2180717a4dd105f36c34433bc1
diff --git a/src/main/java/com/googlesource/gerrit/plugins/metricsreporters/GerritGraphiteReporter.java b/src/main/java/com/googlesource/gerrit/plugins/metricsreporters/GerritGraphiteReporter.java
index ab15709..6ac9db2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/metricsreporters/GerritGraphiteReporter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/metricsreporters/GerritGraphiteReporter.java
@@ -14,7 +14,6 @@
package com.googlesource.gerrit.plugins.metricsreporters;
import static com.codahale.metrics.MetricRegistry.name;
-import static com.google.common.base.MoreObjects.firstNonNull;
import com.google.gerrit.extensions.annotations.Listen;
import com.google.gerrit.extensions.annotations.PluginName;
@@ -48,7 +47,6 @@
private static final String KEY_PORT = "port";
private static final String KEY_PREFIX = "prefix";
private static final String KEY_RATE = "rate";
- private static final String DEFAULT_HOST = "localhost";
private static final int DEFAULT_PORT = 2003;
private static final String DEFAULT_PREFIX = "gerrit";
private static final TimeUnit DEFAULT_RATE_UNIT = TimeUnit.SECONDS;
@@ -63,61 +61,71 @@
@PluginName String pluginName,
MetricRegistry registry) {
Config config = configFactory.getGlobalPluginConfig(pluginName);
- String host = firstNonNull(
- config.getString(SECTION_GRAPHITE, null, KEY_HOST), DEFAULT_HOST);
- int port;
- try {
- port = config.getInt(SECTION_GRAPHITE, KEY_PORT, DEFAULT_PORT);
- } catch (IllegalArgumentException e) {
- log.warn(String.format("Invalid port value; default to %d", DEFAULT_PORT));
- port = DEFAULT_PORT;
- }
- String prefix = config.getString(SECTION_GRAPHITE, null, KEY_PREFIX);
- if (prefix == null) {
+ String host = config.getString(SECTION_GRAPHITE, null, KEY_HOST);
+
+ if (host != null) {
+ int port;
try {
- prefix = name(DEFAULT_PREFIX, InetAddress.getLocalHost().getHostName());
- } catch (UnknownHostException e) {
- log.error("Failed to get hostname", e);
- throw new RuntimeException(e);
+ port = config.getInt(SECTION_GRAPHITE, KEY_PORT, DEFAULT_PORT);
+ } catch (IllegalArgumentException e) {
+ log.warn(String.format("Invalid port value; default to %d", DEFAULT_PORT));
+ port = DEFAULT_PORT;
}
- }
+ String prefix = config.getString(SECTION_GRAPHITE, null, KEY_PREFIX);
+ if (prefix == null) {
+ try {
+ prefix = name(DEFAULT_PREFIX, InetAddress.getLocalHost().getHostName());
+ } catch (UnknownHostException e) {
+ log.error("Failed to get hostname", e);
+ throw new RuntimeException(e);
+ }
+ }
- long configRate;
- try {
- configRate = config.getTimeUnit(
- SECTION_GRAPHITE, null, KEY_RATE, DEFAULT_RATE, DEFAULT_RATE_UNIT);
- } catch (IllegalArgumentException e) {
- log.warn(String.format(
- "Invalid rate value; default to %ds", DEFAULT_RATE));
- configRate = DEFAULT_RATE;
- }
- if (configRate > 0) {
- rate = (int) configRate;
+ long configRate;
+ try {
+ configRate = config.getTimeUnit(
+ SECTION_GRAPHITE, null, KEY_RATE, DEFAULT_RATE, DEFAULT_RATE_UNIT);
+ } catch (IllegalArgumentException e) {
+ log.warn(String.format(
+ "Invalid rate value; default to %ds", DEFAULT_RATE));
+ configRate = DEFAULT_RATE;
+ }
+ if (configRate > 0) {
+ rate = (int) configRate;
+ } else {
+ log.warn(String.format(
+ "Rate value must be positive; default to %ds", DEFAULT_RATE));
+ rate = DEFAULT_RATE;
+ }
+
+ log.info(String.format(
+ "Reporting to Graphite at %s:%d with prefix %s at rate %ds",
+ host, port, prefix, rate));
+
+ graphiteReporter = GraphiteReporter.forRegistry(registry)
+ .convertRatesTo(TimeUnit.MINUTES)
+ .convertDurationsTo(TimeUnit.MILLISECONDS)
+ .prefixedWith(prefix)
+ .filter(MetricFilter.ALL)
+ .build(new Graphite(new InetSocketAddress(host, port)));
} else {
- log.warn(String.format(
- "Rate value must be positive; default to %ds", DEFAULT_RATE));
- rate = DEFAULT_RATE;
+ log.warn("No hostname configured; not reporting to Graphite");
+ graphiteReporter = null;
+ rate = 0;
}
-
- log.info(String.format(
- "Reporting to Graphite at %s:%d with prefix %s at rate %ds",
- host, port, prefix, rate));
-
- graphiteReporter = GraphiteReporter.forRegistry(registry)
- .convertRatesTo(TimeUnit.MINUTES)
- .convertDurationsTo(TimeUnit.MILLISECONDS)
- .prefixedWith(prefix)
- .filter(MetricFilter.ALL)
- .build(new Graphite(new InetSocketAddress(host, port)));
}
@Override
public void start() {
- graphiteReporter.start(rate, DEFAULT_RATE_UNIT);
+ if (graphiteReporter != null) {
+ graphiteReporter.start(rate, DEFAULT_RATE_UNIT);
+ }
}
@Override
public void stop() {
- graphiteReporter.stop();
+ if (graphiteReporter != null) {
+ graphiteReporter.stop();
+ }
}
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 3335582..b1f8e71 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -8,7 +8,8 @@
config file that controls the settings for the @PLUGIN@ plugin.
graphite.host
-: Hostname of the Graphite server. Defaults to `localhost`.
+: Hostname of the Graphite server. Mandatory. If not specified,
+ the plugin does not report to any Graphite instance.
graphite.port
: Port number of the Graphite server. Defaults to `2003`.
