Add support for async metrics, make world size the first async metric (#220)

* add support for async metrics, make world size the first async metric

* clean up execution logic a bit

Author: radiant-ai

src/main/java/de/sldk/mc/MetricRegistry.java

@@ -4,6 +4,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
 
 public class MetricRegistry {
 
@@ -23,8 +24,11 @@ public void register(Metric metric) {
         this.metrics.add(metric);
     }
 
-    void collectMetrics() {
-        this.metrics.forEach(Metric::collect);
+    CompletableFuture<Void> collectMetrics() {
+        /* Combine all Completable futures into a single one */
+        return CompletableFuture.allOf(this.metrics.stream()
+                .map(Metric::collect)
+                .toArray(CompletableFuture[]::new));
     }
 
 }

src/main/java/de/sldk/mc/MetricsController.java

@@ -10,7 +10,6 @@
 
 import java.io.IOException;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
 import java.util.logging.Level;
 
 public class MetricsController extends AbstractHandler {
@@ -31,17 +30,8 @@ public void handle(String target, Request request, HttpServletRequest httpServle
             return;
         }
 
-        /*
-         * Bukkit API calls have to be made from the main thread.
-         * That's why we use the BukkitScheduler to retrieve the server stats.
-         * */
-        Future<Object> future = exporter.getServer().getScheduler().callSyncMethod(exporter, () -> {
-            metricRegistry.collectMetrics();
-            return null;
-        });
-
         try {
-            future.get();
+            metricRegistry.collectMetrics().get();
 
             response.setStatus(HttpStatus.OK_200);
             response.setContentType(TextFormat.CONTENT_TYPE_004);

src/main/java/de/sldk/mc/metrics/Metric.java

@@ -2,8 +2,11 @@
 
 import io.prometheus.client.Collector;
 import io.prometheus.client.CollectorRegistry;
+import org.bukkit.Bukkit;
 import org.bukkit.plugin.Plugin;
 
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
 import java.util.logging.Logger;
 
 public abstract class Metric {
@@ -24,21 +27,56 @@ protected Plugin getPlugin() {
         return plugin;
     }
 
-    public void collect() {
-
-        if (!enabled) {
-            return;
-        }
-
-        try {
-            doCollect();
-        } catch (Exception e) {
-            logException(e);
-        }
+    public CompletableFuture<Void> collect() {
+        return CompletableFuture.runAsync(() -> {
+            if (!enabled) {
+                return;
+            }
+
+            /* If metric is capable of async execution run it on a thread pool */
+            if (isAsyncCapable()) {
+
+                try {
+                    doCollect();
+                }
+                catch (Exception e) {
+                    logException(e);
+                }
+                return;
+            }
+
+            /*
+            * Otherwise run the metric on the main thread and make the
+            * thread on thread pool wait for completion
+            */
+            try {
+                Bukkit.getScheduler().callSyncMethod(plugin, () -> {
+                    try {
+                        doCollect();
+                    }
+                    catch (Exception e) {
+                        logException(e);
+                    }
+                    return null;
+                }).get();
+            } catch (InterruptedException | ExecutionException e) {
+                logException(e);
+            }
+        });
     }
 
     protected abstract void doCollect();
 
+    /**
+     * This method is called during the Metric collection
+     * to determine if it is safe to do it async.
+     * By default, all Metrics are sync unless this method
+     * is overridden.
+     */
+    protected boolean isAsyncCapable() {
+        return false;
+    }
+
     private void logException(Exception e) {
         final Logger log = plugin.getLogger();
         final String className = getClass().getSimpleName();

src/main/java/de/sldk/mc/metrics/WorldSize.java

@@ -36,4 +36,9 @@ public void collect(World world) {
             log.throwing(this.getClass().getSimpleName(), "collect", t);
         }
     }
+
+    @Override
+    protected boolean isAsyncCapable() {
+        return true;
+    }
 }

src/test/java/de/sldk/mc/exporter/PrometheusExporterTest.java

@@ -2,17 +2,13 @@
 
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.when;
 
 import de.sldk.mc.MetricsServer;
 import de.sldk.mc.PrometheusExporter;
 import io.prometheus.client.CollectorRegistry;
 import io.prometheus.client.Counter;
 import io.prometheus.client.exporter.common.TextFormat;
 import io.restassured.RestAssured;
-import org.bukkit.Server;
-import org.bukkit.scheduler.BukkitScheduler;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.util.URIUtil;
 import org.junit.jupiter.api.AfterEach;
@@ -24,17 +20,12 @@
 
 import java.io.IOException;
 import java.net.ServerSocket;
-import java.util.concurrent.CompletableFuture;
 
 @ExtendWith(MockitoExtension.class)
 public class PrometheusExporterTest {
 
 	@Mock
 	private PrometheusExporter exporterMock;
-	@Mock
-	private Server mockServer;
-	@Mock
-	private BukkitScheduler mockScheduler;
 
 	private int metricsServerPort;
 	private MetricsServer metricsServer;
@@ -60,7 +51,6 @@ void cleanup() throws Exception {
 
 	@Test
 	void metrics_server_should_return_valid_prometheus_response() {
-		mockBukkitApis();
 		mockPrometheusCounter("mc_mock_metric", "This is a mock metric", 419);
 
 		String requestPath = URIUtil.newURI("http", "localhost", metricsServerPort, "/metrics", null);
@@ -78,12 +68,6 @@ void metrics_server_should_return_valid_prometheus_response() {
 		assertThat(lines[2]).isEqualTo("mc_mock_metric_total 419.0");
 	}
 
-	private void mockBukkitApis() {
-		when(exporterMock.getServer()).thenReturn(mockServer);
-		when(mockServer.getScheduler()).thenReturn(mockScheduler);
-		when(mockScheduler.callSyncMethod(any(), any())).thenReturn(CompletableFuture.completedFuture(null));
-	}
-
 	private void mockPrometheusCounter(String name, String help, int value) {
 		Counter mockPrometheusCounter = Counter.build().name(name).help(help).register();
 		mockPrometheusCounter.inc(value);