Switch to a fail fast approach when creating duplicate metrics

Signed-off-by: Fabio Di Fabio <[email protected]>
hyperledger · Nov 21, 2024 · 1a6416c · 1a6416c
1 parent ab12657
commit 1a6416c
Show file tree

Hide file tree

Showing 34 changed files with 435 additions and 352 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Breaking Changes
 - Removed Retesteth rpc service and commands [#7833](https://github.com/hyperledger/besu/pull/7783)
+- With the switch to a fail fast approach when trying to create a metric that already exists, it is possible that plugins and other application depending on metric system needs to adapt to the new behavior
 
 ### Upcoming Breaking Changes
 - Plugin API will be deprecating the BesuContext interface to be replaced with the ServiceManager interface.
@@ -25,6 +26,7 @@
 - Add a method to check if a metric category is enabled to the plugin API [#7832](https://github.com/hyperledger/besu/pull/7832)
 - Add a new metric collector for counters which get their value from suppliers [#7894](https://github.com/hyperledger/besu/pull/7894)
 - Add account and state overrides to `eth_call` [#7801](https://github.com/hyperledger/besu/pull/7801) and `eth_estimateGas` [#7890](https://github.com/hyperledger/besu/pull/7890)
+- Switch to a fail fast approach when creating duplicate metrics [#7899](https://github.com/hyperledger/besu/pull/7899)
 
 ### Bug fixes
 - Fix registering new metric categories from plugins [#7825](https://github.com/hyperledger/besu/pull/7825)

diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
@@ -53,6 +53,7 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.ipc.JsonRpcIpcConfiguration;
 import org.hyperledger.besu.ethereum.api.jsonrpc.ipc.JsonRpcIpcService;
 import org.hyperledger.besu.ethereum.api.jsonrpc.methods.JsonRpcMethodsFactory;
+import org.hyperledger.besu.ethereum.api.jsonrpc.metrics.RpcMetrics;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketMessageHandler;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketService;
@@ -839,6 +840,8 @@ public Runner build() {
 
     Optional<JsonRpcHttpService> jsonRpcHttpService = Optional.empty();
 
+    final var rpcMetrics = new RpcMetrics(metricsSystem);
+
     if (jsonRpcConfiguration.isEnabled()) {
       final Map<String, JsonRpcMethod> nonEngineMethods =
           jsonRpcMethods(
@@ -851,7 +854,7 @@ public Runner build() {
               transactionPool,
               miningConfiguration,
               miningCoordinator,
-              metricsSystem,
+              rpcMetrics,
               supportedCapabilities,
               jsonRpcConfiguration.getRpcApis().stream()
                   .filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
@@ -875,7 +878,7 @@ public Runner build() {
                   vertx,
                   dataDir,
                   jsonRpcConfiguration,
-                  metricsSystem,
+                  rpcMetrics,
                   natService,
                   nonEngineMethods,
                   new HealthService(new LivenessCheck()),
@@ -898,7 +901,7 @@ public Runner build() {
               transactionPool,
               miningConfiguration,
               miningCoordinator,
-              metricsSystem,
+              rpcMetrics,
               supportedCapabilities,
               engineJsonRpcConfiguration.get().getRpcApis(),
               filterManager,
@@ -938,7 +941,7 @@ public Runner build() {
                   vertx,
                   dataDir,
                   engineJsonRpcConfiguration.orElse(JsonRpcConfiguration.createEngineDefault()),
-                  metricsSystem,
+                  rpcMetrics,
                   natService,
                   websocketMethodsFactory.methods(),
                   Optional.ofNullable(engineSocketConfig),
@@ -991,7 +994,7 @@ public Runner build() {
               transactionPool,
               miningConfiguration,
               miningCoordinator,
-              metricsSystem,
+              rpcMetrics,
               supportedCapabilities,
               webSocketConfiguration.getRpcApis().stream()
                   .filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
@@ -1072,7 +1075,7 @@ public Runner build() {
               transactionPool,
               miningConfiguration,
               miningCoordinator,
-              metricsSystem,
+              rpcMetrics,
               supportedCapabilities,
               jsonRpcIpcConfiguration.getEnabledApis().stream()
                   .filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
@@ -1113,7 +1116,7 @@ public Runner build() {
               transactionPool,
               miningConfiguration,
               miningCoordinator,
-              metricsSystem,
+              rpcMetrics,
               supportedCapabilities,
               inProcessRpcConfiguration.getInProcessRpcApis(),
               filterManager,
@@ -1275,7 +1278,7 @@ private Map<String, JsonRpcMethod> jsonRpcMethods(
       final TransactionPool transactionPool,
       final MiningConfiguration miningConfiguration,
       final MiningCoordinator miningCoordinator,
-      final ObservableMetricsSystem metricsSystem,
+      final RpcMetrics rpcMetrics,
       final Set<Capability> supportedCapabilities,
       final Collection<String> jsonRpcApis,
       final FilterManager filterManager,
@@ -1310,7 +1313,7 @@ private Map<String, JsonRpcMethod> jsonRpcMethods(
                 transactionPool,
                 miningConfiguration,
                 miningCoordinator,
-                metricsSystem,
+                rpcMetrics,
                 supportedCapabilities,
                 accountAllowlistController,
                 nodeAllowlistController,

diff --git a/...ration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestMethodsFactory.java b/...ration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestMethodsFactory.java
@@ -27,6 +27,7 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.filter.FilterManagerBuilder;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.methods.JsonRpcMethodsFactory;
+import org.hyperledger.besu.ethereum.api.jsonrpc.metrics.RpcMetrics;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration;
 import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
 import org.hyperledger.besu.ethereum.blockcreation.PoWMiningCoordinator;
@@ -46,7 +47,6 @@
 import org.hyperledger.besu.ethereum.permissioning.AccountLocalConfigPermissioningController;
 import org.hyperledger.besu.ethereum.permissioning.NodeLocalConfigPermissioningController;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
-import org.hyperledger.besu.metrics.ObservableMetricsSystem;
 import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem;
 import org.hyperledger.besu.metrics.prometheus.MetricsConfiguration;
 import org.hyperledger.besu.nat.NatService;
@@ -157,7 +157,7 @@ public Map<String, JsonRpcMethod> methods() {
     final TransactionPool transactionPool = mock(TransactionPool.class);
     final MiningConfiguration miningConfiguration = mock(MiningConfiguration.class);
     final PoWMiningCoordinator miningCoordinator = mock(PoWMiningCoordinator.class);
-    final ObservableMetricsSystem metricsSystem = new NoOpMetricsSystem();
+    final RpcMetrics rpcMetrics = new RpcMetrics(new NoOpMetricsSystem());
     final Optional<AccountLocalConfigPermissioningController> accountWhitelistController =
         Optional.of(mock(AccountLocalConfigPermissioningController.class));
     final Optional<NodeLocalConfigPermissioningController> nodeWhitelistController =
@@ -203,7 +203,7 @@ public Map<String, JsonRpcMethod> methods() {
             transactionPool,
             miningConfiguration,
             miningCoordinator,
-            metricsSystem,
+            rpcMetrics,
             new HashSet<>(),
             accountWhitelistController,
             nodeWhitelistController,

diff --git a/...eum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java b/...eum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java
@@ -32,22 +32,18 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.health.HealthService;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.Logging403ErrorHandler;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod;
+import org.hyperledger.besu.ethereum.api.jsonrpc.metrics.RpcMetrics;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketMessageHandler;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.subscription.SubscriptionManager;
 import org.hyperledger.besu.ethereum.api.tls.TlsClientAuthConfiguration;
 import org.hyperledger.besu.ethereum.api.tls.TlsConfiguration;
 import org.hyperledger.besu.ethereum.eth.manager.EthScheduler;
-import org.hyperledger.besu.metrics.BesuMetricCategory;
-import org.hyperledger.besu.metrics.opentelemetry.OpenTelemetrySystem;
 import org.hyperledger.besu.nat.NatMethod;
 import org.hyperledger.besu.nat.NatService;
 import org.hyperledger.besu.nat.core.domain.NatServiceType;
 import org.hyperledger.besu.nat.core.domain.NetworkProtocol;
 import org.hyperledger.besu.nat.upnp.UpnpNatManager;
-import org.hyperledger.besu.plugin.services.MetricsSystem;
-import org.hyperledger.besu.plugin.services.metrics.LabelledMetric;
-import org.hyperledger.besu.plugin.services.metrics.OperationTimer;
 import org.hyperledger.besu.util.ExceptionUtils;
 import org.hyperledger.besu.util.NetworkUtility;
 
@@ -62,13 +58,10 @@
 import javax.annotation.Nullable;
 
 import com.google.common.annotations.VisibleForTesting;
-import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.StatusCode;
-import io.opentelemetry.api.trace.Tracer;
-import io.opentelemetry.api.trace.TracerProvider;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapGetter;
 import io.opentelemetry.context.propagation.TextMapPropagator;
@@ -134,9 +127,7 @@ public String get(final @Nullable HttpServerRequest carrier, final String key) {
   private final Map<String, JsonRpcMethod> rpcMethods;
   private final NatService natService;
   private final Path dataDir;
-  private final LabelledMetric<OperationTimer> requestTimer;
-  private TracerProvider tracerProvider;
-  private Tracer tracer;
+  private final RpcMetrics rpcMetrics;
   private final int maxActiveConnections;
   private final AtomicInteger activeConnectionsCount = new AtomicInteger();
 
@@ -149,15 +140,13 @@ public String get(final @Nullable HttpServerRequest carrier, final String key) {
   private final HealthService livenessService;
   private final HealthService readinessService;
 
-  private final MetricsSystem metricsSystem;
-
   /**
    * Construct a EngineJsonRpcService to handle either http or websocket clients
    *
    * @param vertx The vertx process that will be running this service
    * @param dataDir The data directory where requests can be buffered
    * @param config Configuration for the rpc methods being loaded
-   * @param metricsSystem The metrics service that activities should be reported to
+   * @param rpcMetrics The RPC metrics
    * @param natService The NAT environment manager.
    * @param methods The json rpc methods that should be enabled
    * @param maybeSockets websocket configuration to use
@@ -170,7 +159,7 @@ public EngineJsonRpcService(
       final Vertx vertx,
       final Path dataDir,
       final JsonRpcConfiguration config,
-      final MetricsSystem metricsSystem,
+      final RpcMetrics rpcMetrics,
       final NatService natService,
       final Map<String, JsonRpcMethod> methods,
       final Optional<WebSocketConfiguration> maybeSockets,
@@ -179,16 +168,8 @@ public EngineJsonRpcService(
       final HealthService livenessService,
       final HealthService readinessService) {
     this.dataDir = dataDir;
-    this.requestTimer =
-        metricsSystem.createLabelledTimer(
-            BesuMetricCategory.RPC,
-            "request_time",
-            "Time taken to process a JSON-RPC request",
-            "methodName");
+    this.rpcMetrics = rpcMetrics;
     JsonRpcProcessor jsonRpcProcessor = new BaseJsonRpcProcessor();
-    if (metricsSystem instanceof OpenTelemetrySystem) {
-      this.tracerProvider = ((OpenTelemetrySystem) metricsSystem).getTracerProvider();
-    }
 
     this.socketConfiguration =
         maybeSockets.isPresent() ? maybeSockets.get() : WebSocketConfiguration.createDefault();
@@ -216,17 +197,12 @@ public EngineJsonRpcService(
     this.livenessService = livenessService;
     this.readinessService = readinessService;
     this.maxActiveConnections = config.getMaxActiveConnections();
-    this.metricsSystem = metricsSystem;
   }
 
   public CompletableFuture<Void> start() {
     LOG.info("Starting JSON-RPC service on {}:{}", config.getHost(), config.getPort());
     LOG.debug("max number of active connections {}", maxActiveConnections);
-    if (this.tracerProvider != null) {
-      this.tracer = tracerProvider.get("org.hyperledger.besu.jsonrpc", "1.0.0");
-    } else {
-      this.tracer = OpenTelemetry.noop().getTracer("org.hyperledger.besu.jsonrpc", "1.0.0");
-    }
+
     final CompletableFuture<Void> resultFuture = new CompletableFuture<>();
     try {
       // Create the HTTP server and a router object.
@@ -454,23 +430,23 @@ private Router buildRouter() {
               new JsonRpcExecutor(
                   new AuthenticatedJsonRpcProcessor(
                       new TimedJsonRpcProcessor(
-                          new TracedJsonRpcProcessor(new BaseJsonRpcProcessor(), metricsSystem),
-                          requestTimer),
+                          new TracedJsonRpcProcessor(new BaseJsonRpcProcessor(), rpcMetrics),
+                          rpcMetrics),
                       authenticationService.get(),
                       config.getNoAuthRpcApis()),
                   rpcMethods),
-              tracer,
+              rpcMetrics.getTracer(),
               config),
           false);
     } else {
       mainRoute.blockingHandler(
           HandlerFactory.jsonRpcExecutor(
               new JsonRpcExecutor(
                   new TimedJsonRpcProcessor(
-                      new TracedJsonRpcProcessor(new BaseJsonRpcProcessor(), metricsSystem),
-                      requestTimer),
+                      new TracedJsonRpcProcessor(new BaseJsonRpcProcessor(), rpcMetrics),
+                      rpcMetrics),
                   rpcMethods),
-              tracer,
+              rpcMetrics.getTracer(),
               config),
           false);
     }
@@ -497,7 +473,8 @@ private void createSpan(final RoutingContext routingContext) {
     Context parent =
         traceFormats.extract(Context.current(), routingContext.request(), requestAttributesGetter);
     final Span serverSpan =
-        tracer
+        rpcMetrics
+            .getTracer()
             .spanBuilder(address.host() + ":" + address.port())
             .setParent(parent)
             .setSpanKind(SpanKind.SERVER)