dlmarion commented on code in PR #5021:
URL: https://github.com/apache/accumulo/pull/5021#discussion_r1820651445
##########
server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java:
##########
@@ -251,23 +206,42 @@ public DistributionStatisticConfig configure(Meter.Id id,
String userRegistryFactories =
context.getConfiguration().get(Property.GENERAL_MICROMETER_FACTORY);
+ List<MeterRegistry> registries = new ArrayList<>();
+
for (String factoryName : getTrimmedStrings(userRegistryFactories)) {
try {
MeterRegistry registry = getRegistryFromFactory(factoryName,
context);
registry.config().commonTags(commonTags.values());
registry.config().meterFilter(replicationFilter);
- addRegistry(registry);
+ registries.add(registry);
} catch (ReflectiveOperationException ex) {
LOG.warn("Could not load registry {}", factoryName, ex);
}
}
- pendingRegistries.forEach(registry -> composite.add(registry));
+ if (registries.size() == 1) {
+ meterRegistry = registries.get(0);
+ } else {
+ var composite = new CompositeMeterRegistry();
+ composite.config().commonTags(commonTags.values());
+ registries.forEach(composite::add);
+ meterRegistry = composite;
+ }
+
+ if (jvmMetricsEnabled) {
+ LOG.info("enabling detailed jvm, classloader, jvm gc and process
metrics");
+ new ClassLoaderMetrics().bindTo(meterRegistry);
+ new JvmMemoryMetrics().bindTo(meterRegistry);
+ jvmGcMetrics = new JvmGcMetrics();
+ jvmGcMetrics.bindTo(meterRegistry);
+ new ProcessorMetrics().bindTo(meterRegistry);
+ new JvmThreadMetrics().bindTo(meterRegistry);
+ }
LOG.info("Metrics initialization. Register producers: {}", producers);
- producers.forEach(p -> p.registerMetrics(composite));
+ producers.forEach(p -> p.registerMetrics(meterRegistry));
- Metrics.globalRegistry.add(composite);
+ Metrics.globalRegistry.add(meterRegistry);
Review Comment:
I'm wondering if we should just add each MeterRegistry to the global
registry directly, and then call p.registerMetrics using the global registry.
I'm not sure what benefit there is to using a composite registry in this code.
If we were to do this, then I think we would want to add the user
MeterRegistry's to the global first, then set up the JVM metrics and the
MetricsProducers.
--
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]
