Shutdown MetricsSystem when stopping MetricsService (hyperledger#7958)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
ahamlat · Dec 2, 2024 · 812bafe · 812bafe
1 parent 1dbc590
commit 812bafe
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 12 deletions.
diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/MetricsService.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/MetricsService.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.metrics;
 
 import org.hyperledger.besu.metrics.opentelemetry.MetricsOtelPushService;
+import org.hyperledger.besu.metrics.opentelemetry.OpenTelemetrySystem;
 import org.hyperledger.besu.metrics.prometheus.MetricsConfiguration;
 import org.hyperledger.besu.metrics.prometheus.MetricsHttpService;
 import org.hyperledger.besu.metrics.prometheus.MetricsPushGatewayService;
@@ -48,13 +49,14 @@ static Optional<MetricsService> create(
         return Optional.of(
             new MetricsHttpService(configuration, (PrometheusMetricsSystem) metricsSystem));
       } else if (configuration.isPushEnabled()) {
-        return Optional.of(new MetricsPushGatewayService(configuration, metricsSystem));
+        return Optional.of(
+            new MetricsPushGatewayService(configuration, (PrometheusMetricsSystem) metricsSystem));
       } else {
         return Optional.empty();
       }
     } else if (configuration.getProtocol() == MetricsProtocol.OPENTELEMETRY) {
       if (configuration.isEnabled()) {
-        return Optional.of(new MetricsOtelPushService());
+        return Optional.of(new MetricsOtelPushService((OpenTelemetrySystem) metricsSystem));
       } else {
         return Optional.empty();
       }

diff --git a/...core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/MetricsOtelPushService.java b/...core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/MetricsOtelPushService.java
@@ -26,9 +26,16 @@
 public class MetricsOtelPushService implements MetricsService {
 
   private static final Logger LOG = LoggerFactory.getLogger(MetricsOtelPushService.class);
-
-  /** Instantiates a new Metrics open telemetry push service. */
-  public MetricsOtelPushService() {}
+  private final OpenTelemetrySystem metricsSystem;
+
+  /**
+   * Instantiates a new Metrics open telemetry push service.
+   *
+   * @param metricsSystem The OpenTelemetry metrics system
+   */
+  public MetricsOtelPushService(final OpenTelemetrySystem metricsSystem) {
+    this.metricsSystem = metricsSystem;
+  }
 
   @Override
   public CompletableFuture<?> start() {
@@ -39,6 +46,7 @@ public CompletableFuture<?> start() {
 
   @Override
   public CompletableFuture<?> stop() {
+    metricsSystem.shutdown();
     return CompletableFuture.completedFuture(null);
   }
 

diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsHttpService.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsHttpService.java
@@ -121,6 +121,7 @@ private boolean hostIsInAllowlist(final String hostHeader) {
 
   @Override
   public CompletableFuture<?> stop() {
+    metricsSystem.shutdown();
     if (httpServer == null) {
       return CompletableFuture.completedFuture(null);
     }

diff --git a/...core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsPushGatewayService.java b/...core/src/main/java/org/hyperledger/besu/metrics/prometheus/MetricsPushGatewayService.java
@@ -17,7 +17,6 @@
 import static com.google.common.base.Preconditions.checkArgument;
 
 import org.hyperledger.besu.metrics.MetricsService;
-import org.hyperledger.besu.plugin.services.MetricsSystem;
 
 import java.io.IOException;
 import java.util.Optional;
@@ -37,7 +36,7 @@ public class MetricsPushGatewayService implements MetricsService {
   private PushGateway pushGateway;
   private ScheduledExecutorService scheduledExecutorService;
   private final MetricsConfiguration config;
-  private final MetricsSystem metricsSystem;
+  private final PrometheusMetricsSystem metricsSystem;
 
   /**
    * Instantiates a new Metrics push gateway service.
@@ -46,7 +45,7 @@ public class MetricsPushGatewayService implements MetricsService {
    * @param metricsSystem the metrics system
    */
   public MetricsPushGatewayService(
-      final MetricsConfiguration configuration, final MetricsSystem metricsSystem) {
+      final MetricsConfiguration configuration, final PrometheusMetricsSystem metricsSystem) {
     this.metricsSystem = metricsSystem;
     validateConfig(configuration);
     config = configuration;
@@ -59,9 +58,6 @@ private void validateConfig(final MetricsConfiguration config) {
     checkArgument(
         !(config.isEnabled() && config.isPushEnabled()),
         "Metrics Push Gateway Service cannot run concurrent with the normal metrics.");
-    checkArgument(
-        metricsSystem instanceof PrometheusMetricsSystem,
-        "Push Gateway requires a Prometheus Metrics System.");
   }
 
   @Override
@@ -73,7 +69,7 @@ public CompletableFuture<?> start() {
 
     pushGateway =
         PushGateway.builder()
-            .registry(((PrometheusMetricsSystem) metricsSystem).getRegistry())
+            .registry(metricsSystem.getRegistry())
             .address(config.getPushHost() + ":" + config.getPushPort())
             .job(config.getPrometheusJob())
             .build();
@@ -90,6 +86,7 @@ public CompletableFuture<?> start() {
 
   @Override
   public CompletableFuture<?> stop() {
+    metricsSystem.shutdown();
     final CompletableFuture<?> resultFuture = new CompletableFuture<>();
     try {
       // Calling shutdown now cancels the pending push, which is desirable.