Allow applicationName to be configured
The cloudwatch plugin used to explicitly look at the instanceId and add
it as a label for the generated metrics.
This is fine for single Gerrit installations, but it does not work for
HA setups where the instance ID is the same for all Gerrit peers in the
cluster, making it impossible to differentiate metrics origin.
Allow an explicit `applicationName`, defaulting to the instanceId, to be
configured so that Gerrit admin can explicitly decide the value of this
Gerrit's metric dimension.
Bug: Issue 16772
Change-Id: Id792c09c0af071024bcf10383a3d9dea7006f13b
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 5cc01de..bc80ddd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporter.java
@@ -47,11 +47,11 @@
.withHighResolution();
config
- .getMaybeInstanceId()
+ .getMaybeApplicationName()
.ifPresent(
- instanceId ->
+ applicationName ->
cloudWatchReporterBuilder.withGlobalDimensions(
- String.format("InstanceId=%s", instanceId)));
+ String.format("ApplicationName=%s", applicationName)));
if (config.getDryRun()) {
cloudWatchReporterBuilder.withDryRun();
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 62535dd..2cdf3f9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfig.java
@@ -32,6 +32,7 @@
class GerritCloudwatchReporterConfig {
protected static final String KEY_NAMESPACE = "namespace";
+ protected static final String KEY_APPLICATION_NAME = "applicationName";
protected static final String KEY_RATE = "rate";
protected static final String KEY_DRYRUN = "dryRun";
protected static final String KEY_INITIAL_DELAY = "initialDelay";
@@ -51,7 +52,7 @@
private final Boolean dryRun;
private final MetricFilter exclusionFilter;
private final Boolean jvmMetrics;
- private final Optional<String> maybeInstanceId;
+ private final Optional<String> maybeApplicationName;
@Inject
public GerritCloudwatchReporterConfig(
@@ -60,7 +61,8 @@
@Nullable @GerritInstanceId String instanceId) {
PluginConfig pluginConfig = configFactory.getFromGerritConfig(pluginName);
- this.maybeInstanceId = Optional.ofNullable(instanceId);
+ this.maybeApplicationName =
+ Optional.ofNullable(pluginConfig.getString(KEY_APPLICATION_NAME, instanceId));
this.namespace = pluginConfig.getString(KEY_NAMESPACE, DEFAULT_NAMESPACE);
@@ -118,7 +120,7 @@
return (s, metric) -> !filter.test(s);
}
- public Optional<String> getMaybeInstanceId() {
- return maybeInstanceId;
+ public Optional<String> getMaybeApplicationName() {
+ return maybeApplicationName;
}
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index b29d4d2..973a44d 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -16,19 +16,6 @@
[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,
@@ -54,6 +41,16 @@
* AWS Docs: [Namespaces](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/cloudwatch_concepts.html#Namespace)
* Example: "my-gerrit-metrics"
+* `plugin.@PLUGIN@.applicationName` (Optional): The name of the application
+ emitting metrics. this plugin will hydrate all the metrics sent to CloudWatch
+ with an additional dimension named `ApplicationName`.
+ * Type: String
+ * Default: The gerrit [instanceId](https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#gerrit.instanceId), when defined. null otherwise.
+ * Example: "gerrit-uk-1"
+ * Notes: This is useful as it allows to correlate cloudwatch metrics to
+ specific instances they originated from. If not set and instanceId is not
+ configured, then metrics will not have any `ApplicationName` dimension.
+
* `plugin.@PLUGIN@.rate` (Optional): The rate at which metrics should be fired to AWS.
* Type: Time
* Default: "60s"
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 dc00d27..7f2fd63 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/GerritCloudwatchReporterConfigTest.java
@@ -57,7 +57,7 @@
.isEqualTo(GerritCloudwatchReporterConfig.DEFAULT_DRY_RUN);
assertThat(reporterConfig.getJvmMetrics())
.isEqualTo(GerritCloudwatchReporterConfig.DEFAULT_JVM_METRICS);
- assertThat(reporterConfig.getMaybeInstanceId()).isEqualTo(Optional.empty());
+ assertThat(reporterConfig.getMaybeApplicationName()).isEqualTo(Optional.empty());
}
@Test
@@ -68,6 +68,7 @@
globalPluginConfig.setString(GerritCloudwatchReporterConfig.KEY_INITIAL_DELAY, "20s");
globalPluginConfig.setBoolean(GerritCloudwatchReporterConfig.KEY_DRYRUN, true);
globalPluginConfig.setBoolean(GerritCloudwatchReporterConfig.KEY_JVM_METRICS, true);
+ globalPluginConfig.setString(GerritCloudwatchReporterConfig.KEY_APPLICATION_NAME, "appName");
when(configFactory.getFromGerritConfig(PLUGIN_NAME))
.thenReturn(globalPluginConfig.asPluginConfig());
@@ -79,7 +80,17 @@
assertThat(reporterConfig.getRate()).isEqualTo(180);
assertThat(reporterConfig.getDryRun()).isTrue();
assertThat(reporterConfig.getJvmMetrics()).isTrue();
- assertThat(reporterConfig.getMaybeInstanceId()).isEqualTo(Optional.of(gerritInstanceId));
+ assertThat(reporterConfig.getMaybeApplicationName()).isEqualTo(Optional.of("appName"));
+ }
+
+ @Test
+ public void shouldSetApplicationNameWithInstanceIdWhenNotOverridden() {
+ when(configFactory.getFromGerritConfig(PLUGIN_NAME))
+ .thenReturn(emptyGlobalPluginConfig.asPluginConfig());
+ reporterConfig =
+ new GerritCloudwatchReporterConfig(configFactory, PLUGIN_NAME, gerritInstanceId);
+
+ assertThat(reporterConfig.getMaybeApplicationName()).isEqualTo(Optional.of(gerritInstanceId));
}
@Test
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 94aaf21..c4b8a3c 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/MetricsReporterCloudwatchIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/metricsreportercloudwatch/MetricsReporterCloudwatchIT.java
@@ -45,6 +45,7 @@
sysModule = "com.googlesource.gerrit.plugins.metricsreportercloudwatch.GerritCloudwatchModule")
public class MetricsReporterCloudwatchIT extends LightweightPluginDaemonTest {
private static final String GERRIT_INSTANCE_ID = "testInstanceId";
+ private static final String APPLICATION_NAME = "testApplicationName";
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";
@@ -142,7 +143,7 @@
@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 {
+ public void shouldAddInstanceIdAsApplicationNameDimensionWhenAvailable() throws Exception {
InMemoryLoggerAppender dryRunMetricsOutput = newInMemoryLogger();
waitUntil(
@@ -152,7 +153,27 @@
.anyMatch(
l ->
l.contains(
- String.format("Name=InstanceId, Value=%s", GERRIT_INSTANCE_ID))));
+ String.format("Name=ApplicationName, Value=%s", GERRIT_INSTANCE_ID))));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.metrics-reporter-cloudwatch.dryrun", value = "true")
+ @GerritConfig(
+ name = "plugin.metrics-reporter-cloudwatch.applicationName",
+ value = APPLICATION_NAME)
+ @GerritConfig(name = "plugin.metrics-reporter-cloudwatch.rate", value = TEST_TIMEOUT)
+ @GerritConfig(name = "gerrit.instanceId", value = GERRIT_INSTANCE_ID)
+ public void shouldAddApplicationNameConfigValueAsDimensionWhenAvailable() throws Exception {
+ InMemoryLoggerAppender dryRunMetricsOutput = newInMemoryLogger();
+
+ waitUntil(
+ () ->
+ dryRunMetricsOutput
+ .metricsStream()
+ .anyMatch(
+ l ->
+ l.contains(
+ String.format("Name=ApplicationName, Value=%s", APPLICATION_NAME))));
}
private static InMemoryLoggerAppender newInMemoryLogger() {
