ctubbsii commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1163411652
##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java:
##########
@@ -594,6 +601,8 @@ public interface MetricsProducer {
Logger LOG = LoggerFactory.getLogger(MetricsProducer.class);
+ String METRICS_APP_PREFIX = "accumulo.app.";
Review Comment:
"app" sounds like "mobile app", and sounds a bit "trendy" and less
serious/scientific. Maybe "server"? Or just "accumulo."? Or
"accumulo.application"?
##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +93,24 @@ public void runServer() throws Exception {
}
}
+ @Override
+ public void registerMetrics(MeterRegistry registry) {
+ lowMemoryMetricGuage =
+ Gauge
+ .builder(METRICS_APP_PREFIX + applicationName + "." + hostname +
"."
+ + METRICS_APP_LOW_MEMORY, this, this::lowMemDetected)
+ .description(
+ "reports 1 when process memory usage is above threshold, 0
when memory is okay") // optional
+ .register(registry);
+ }
+
+ private int lowMemDetected(AbstractServer abstractServer) {
+ if (abstractServer.context.getLowMemoryDetector().isRunningLowOnMemory()) {
+ return 1;
+ }
+ return 0;
Review Comment:
```suggestion
return
abstractServer.context.getLowMemoryDetector().isRunningLowOnMemory() ? 1 : 0;
```
Or, this looks cleaner, because it seems to be passing the current object to
itself:
```suggestion
return context.getLowMemoryDetector().isRunningLowOnMemory() ? 1 : 0;
```
##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -35,21 +36,25 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public abstract class AbstractServer implements AutoCloseable, Runnable {
+import io.micrometer.core.instrument.Gauge;
+import io.micrometer.core.instrument.MeterRegistry;
+
+public abstract class AbstractServer implements AutoCloseable,
MetricsProducer, Runnable {
private final ServerContext context;
protected final String applicationName;
private final String hostname;
- private final Logger log;
+
+ private Gauge lowMemoryMetricGuage = null;
protected AbstractServer(String appName, ConfigOpts opts, String[] args) {
- this.log = LoggerFactory.getLogger(getClass().getName());
this.applicationName = appName;
opts.parseArgs(appName, args);
var siteConfig = opts.getSiteConfiguration();
this.hostname = siteConfig.get(Property.GENERAL_PROCESS_BIND_ADDRESS);
SecurityUtil.serverLogin(siteConfig);
context = new ServerContext(siteConfig);
+ Logger log = LoggerFactory.getLogger(getClass().getName());
Review Comment:
I think you can just use the class, rather than explicitly call `.getName()`.
```suggestion
Logger log = LoggerFactory.getLogger(getClass());
```
--
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]
