ddanielr commented on code in PR #3297:
URL: https://github.com/apache/accumulo/pull/3297#discussion_r1167357351
##########
test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java:
##########
@@ -158,4 +174,25 @@ public void registerMetrics(MeterRegistry registry) {
// unused; this class only extends MetricsProducer to easily reference its
methods/constants
}
+ @Test
+ public void metricTags() throws Exception {
+
+ doWorkToGenerateMetrics();
+ cluster.stop();
+
+ List<String> statsDMetrics;
+
+ // loop until we run out of lines or until we see all expected metrics
+ while (!(statsDMetrics = sink.getLines()).isEmpty()) {
+ statsDMetrics.stream().filter(line -> line.startsWith("accumulo"))
+ .map(TestStatsDSink::parseStatsDMetric).forEach(a -> {
+ var t = a.getTags();
+ log.trace("METRICS, name: '{}' num tags: {}, tags: {}",
a.getName(), t.size(), t);
+ // check hostname is always set and is valid
+ assertNotEquals("0.0.0.0", a.getTags().get("host"));
+ // check the length of the tag value is sane
+ a.getTags().forEach((k, v) -> assertTrue(v.length() < 128));
Review Comment:
Nit: Use a named variable like `maxTagLength=128` value for readability and
future troubleshooting of test failures.
##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsUtil.java:
##########
@@ -79,9 +79,14 @@ private static void initializeMetrics(boolean enabled,
boolean jvmMetricsEnabled
List<Tag> tags = new ArrayList<>();
tags.add(Tag.of("process.name", processName));
- if (address != null) {
- tags.add(Tag.of("Address", address.toString()));
+ if (address != null && !address.getHost().isEmpty()) {
+ tags.add(Tag.of("host", address.getHost()));
}
+
+ if (address != null && address.getPort() > 0) {
+ tags.add(Tag.of("port", Integer.toString(address.getPort())));
+ }
+
Review Comment:
Use a nested if statement since both checks rely on the same primary
condition.
```suggestion
if (address != null) {
if (!address.getHost().isEmpty()) {
tags.add(Tag.of("host", address.getHost()));
}
if (address.getPort() > 0) {
tags.add(Tag.of("port", Integer.toString(address.getPort())));
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
