Merge branch 'stable-3.2'
* stable-3.2:
Add InstanceId dimension, when available
Correct unnecessary builder reassignment
Change-Id: Ibcff702b27b21507bd8929f2cb90f3dda8a1594b
diff --git a/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporter.java b/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporter.java
index 56b6769..4803e14 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporter.java
@@ -33,7 +33,8 @@
throws IllegalStateException {
this.config = config;
- CloudWatchAsyncClientBuilder cloudWatchAsyncClientBuilder = CloudWatchAsyncClient.builder();
+ final CloudWatchAsyncClientBuilder cloudWatchAsyncClientBuilder =
+ CloudWatchAsyncClient.builder();
CloudWatchReporter.Builder cloudWatchReporterBuilder =
CloudWatchReporter.forRegistry(
@@ -45,12 +46,19 @@
.withReportRawCountValue()
.withHighResolution();
+ config
+ .getMaybeInstanceId()
+ .ifPresent(
+ instanceId ->
+ cloudWatchReporterBuilder.withGlobalDimensions(
+ String.format("InstanceId=%s", instanceId)));
+
if (config.getDryRun()) {
- cloudWatchReporterBuilder = cloudWatchReporterBuilder.withDryRun();
+ cloudWatchReporterBuilder.withDryRun();
}
if (config.getJvmMetrics()) {
- cloudWatchReporterBuilder = cloudWatchReporterBuilder.withJvmMetrics();
+ cloudWatchReporterBuilder.withJvmMetrics();
}
cloudWatchReporter = cloudWatchReporterBuilder.build();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfig.java b/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfig.java
index 951f526..62535dd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfig.java
@@ -16,13 +16,16 @@
import static java.util.stream.Collectors.toList;
import com.codahale.metrics.MetricFilter;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.config.ConfigUtil;
+import com.google.gerrit.server.config.GerritInstanceId;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.Inject;
import java.util.Arrays;
import java.util.List;
+import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
import java.util.regex.Pattern;
@@ -48,12 +51,17 @@
private final Boolean dryRun;
private final MetricFilter exclusionFilter;
private final Boolean jvmMetrics;
+ private final Optional<String> maybeInstanceId;
@Inject
public GerritCloudwatchReporterConfig(
- PluginConfigFactory configFactory, @PluginName String pluginName) {
+ PluginConfigFactory configFactory,
+ @PluginName String pluginName,
+ @Nullable @GerritInstanceId String instanceId) {
PluginConfig pluginConfig = configFactory.getFromGerritConfig(pluginName);
+ this.maybeInstanceId = Optional.ofNullable(instanceId);
+
this.namespace = pluginConfig.getString(KEY_NAMESPACE, DEFAULT_NAMESPACE);
this.dryRun = pluginConfig.getBoolean(KEY_DRYRUN, DEFAULT_DRY_RUN);
@@ -109,4 +117,8 @@
s -> excludedMetricPatterns.stream().anyMatch(e -> e.matcher(s).matches());
return (s, metric) -> !filter.test(s);
}
+
+ public Optional<String> getMaybeInstanceId() {
+ return maybeInstanceId;
+ }
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index cb645c5..b29d4d2 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -16,6 +16,19 @@
[here](https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/credentials.html) and
[here](https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/setup-credentials.html)
+## InstanceId Dimension
+
+Gerrit can be optionally configured to have a unique identifier, the
+[instanceId](https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#gerrit.instanceId),
+which represents a specific instance within a group of Gerrit instances.
+
+When the instanceId is set this plugin will hydrate all the metrics sent to CloudWatch with
+an additional dimension named `InstanceId`, populated with the value of the `gerrit.instanceId`
+configuration.
+
+This is useful as it allows to correlate cloudwatch metrics to specific instances
+they originated from.
+
## Metrics Reporter
* `plugin.@PLUGIN@.dryRun` (Optional): the reporter will log.DEBUG the metrics,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfigTest.java
index d1895d6..dc00d27 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfigTest.java
@@ -22,6 +22,7 @@
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
import java.util.Arrays;
+import java.util.Optional;
import java.util.regex.PatternSyntaxException;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
@@ -31,6 +32,7 @@
@RunWith(MockitoJUnitRunner.class)
public class GerritCloudwatchReporterConfigTest {
+ private static final String gerritInstanceId = "testInstanceId";
private static final String PLUGIN_NAME = "foo";
private final PluginConfig.Update emptyGlobalPluginConfig =
PluginConfig.Update.forTest(PLUGIN_NAME, new Config());
@@ -43,7 +45,7 @@
public void shouldGetAllDefaultsWhenConfigurationIsEmpty() {
when(configFactory.getFromGerritConfig(PLUGIN_NAME))
.thenReturn(emptyGlobalPluginConfig.asPluginConfig());
- reporterConfig = new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME);
+ reporterConfig = new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME, null);
assertThat(reporterConfig.getInitialDelay())
.isEqualTo(GerritCloudwatchReporterConfig.DEFAULT_INITIAL_DELAY_SECS);
@@ -55,6 +57,7 @@
.isEqualTo(GerritCloudwatchReporterConfig.DEFAULT_DRY_RUN);
assertThat(reporterConfig.getJvmMetrics())
.isEqualTo(GerritCloudwatchReporterConfig.DEFAULT_JVM_METRICS);
+ assertThat(reporterConfig.getMaybeInstanceId()).isEqualTo(Optional.empty());
}
@Test
@@ -68,13 +71,15 @@
when(configFactory.getFromGerritConfig(PLUGIN_NAME))
.thenReturn(globalPluginConfig.asPluginConfig());
- reporterConfig = new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME);
+ reporterConfig =
+ new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME, gerritInstanceId);
assertThat(reporterConfig.getInitialDelay()).isEqualTo(20);
assertThat(reporterConfig.getNamespace()).isEqualTo("foobar");
assertThat(reporterConfig.getRate()).isEqualTo(180);
assertThat(reporterConfig.getDryRun()).isTrue();
assertThat(reporterConfig.getJvmMetrics()).isTrue();
+ assertThat(reporterConfig.getMaybeInstanceId()).isEqualTo(Optional.of(gerritInstanceId));
}
@Test
@@ -85,7 +90,8 @@
when(configFactory.getFromGerritConfig(PLUGIN_NAME))
.thenReturn(globalPluginConfig.asPluginConfig());
- reporterConfig = new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME);
+ reporterConfig =
+ new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME, gerritInstanceId);
MetricFilter exclusionFilter = reporterConfig.getExclusionFilter();
assertThat(exclusionFilter.matches("foo/metrics/for/testing", new Counter())).isFalse();
@@ -106,7 +112,7 @@
assertThrows(
PatternSyntaxException.class,
() -> {
- new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME);
+ new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME, gerritInstanceId);
});
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/MetricsReporterCloudwatchIT.java b/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/MetricsReporterCloudwatchIT.java
index 43b448f..94aaf21 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/MetricsReporterCloudwatchIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/MetricsReporterCloudwatchIT.java
@@ -44,6 +44,7 @@
name = "metrics-reporter-cloudwatch",
sysModule = "com.googlesource.gerrit.plugins.metricsreportercloudwatch.GerritCloudwatchModule")
public class MetricsReporterCloudwatchIT extends LightweightPluginDaemonTest {
+ private static final String GERRIT_INSTANCE_ID = "testInstanceId";
private static final String TEST_METRIC_NAME = "test/metric/name";
private static final long TEST_METRIC_INCREMENT = 1234567L;
private static final String TEST_JVM_METRIC_NAME = "jvm.uptime";
@@ -137,6 +138,23 @@
});
}
+ @Test
+ @GerritConfig(name = "plugin.metrics-reporter-cloudwatch.dryrun", value = "true")
+ @GerritConfig(name = "plugin.metrics-reporter-cloudwatch.rate", value = TEST_TIMEOUT)
+ @GerritConfig(name = "gerrit.instanceId", value = GERRIT_INSTANCE_ID)
+ public void shouldAddInstanceIdDimensionWhenAvailable() throws Exception {
+ InMemoryLoggerAppender dryRunMetricsOutput = newInMemoryLogger();
+
+ waitUntil(
+ () ->
+ dryRunMetricsOutput
+ .metricsStream()
+ .anyMatch(
+ l ->
+ l.contains(
+ String.format("Name=InstanceId, Value=%s", GERRIT_INSTANCE_ID))));
+ }
+
private static InMemoryLoggerAppender newInMemoryLogger() {
InMemoryLoggerAppender dryRunMetricsOutput = new InMemoryLoggerAppender();
for (Enumeration<?> logger = LogManager.getCurrentLoggers(); logger.hasMoreElements(); ) {
