Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/main/java/de/sldk/mc/MetricRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CompletableFuture;

public class MetricRegistry {

Expand All @@ -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));
}

}
12 changes: 1 addition & 11 deletions src/main/java/de/sldk/mc/MetricsController.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down
60 changes: 49 additions & 11 deletions src/main/java/de/sldk/mc/metrics/Metric.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/de/sldk/mc/metrics/WorldSize.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,9 @@ public void collect(World world) {
log.throwing(this.getClass().getSimpleName(), "collect", t);
}
}

@Override
protected boolean isAsyncCapable() {
return true;
}
}
16 changes: 0 additions & 16 deletions src/test/java/de/sldk/mc/exporter/PrometheusExporterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand Down