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

Switch to a fail fast approach when creating duplicate metrics #7899

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
21 changes: 12 additions & 9 deletions besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -851,7 +854,7 @@ public Runner build() {
transactionPool,
miningConfiguration,
miningCoordinator,
metricsSystem,
rpcMetrics,
supportedCapabilities,
jsonRpcConfiguration.getRpcApis().stream()
.filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
Expand All @@ -875,7 +878,7 @@ public Runner build() {
vertx,
dataDir,
jsonRpcConfiguration,
metricsSystem,
rpcMetrics,
natService,
nonEngineMethods,
new HealthService(new LivenessCheck()),
Expand All @@ -898,7 +901,7 @@ public Runner build() {
transactionPool,
miningConfiguration,
miningCoordinator,
metricsSystem,
rpcMetrics,
supportedCapabilities,
engineJsonRpcConfiguration.get().getRpcApis(),
filterManager,
Expand Down Expand Up @@ -938,7 +941,7 @@ public Runner build() {
vertx,
dataDir,
engineJsonRpcConfiguration.orElse(JsonRpcConfiguration.createEngineDefault()),
metricsSystem,
rpcMetrics,
natService,
websocketMethodsFactory.methods(),
Optional.ofNullable(engineSocketConfig),
Expand Down Expand Up @@ -991,7 +994,7 @@ public Runner build() {
transactionPool,
miningConfiguration,
miningCoordinator,
metricsSystem,
rpcMetrics,
supportedCapabilities,
webSocketConfiguration.getRpcApis().stream()
.filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
Expand Down Expand Up @@ -1072,7 +1075,7 @@ public Runner build() {
transactionPool,
miningConfiguration,
miningCoordinator,
metricsSystem,
rpcMetrics,
supportedCapabilities,
jsonRpcIpcConfiguration.getEnabledApis().stream()
.filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
Expand Down Expand Up @@ -1113,7 +1116,7 @@ public Runner build() {
transactionPool,
miningConfiguration,
miningCoordinator,
metricsSystem,
rpcMetrics,
supportedCapabilities,
inProcessRpcConfiguration.getInProcessRpcApis(),
filterManager,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1310,7 +1313,7 @@ private Map<String, JsonRpcMethod> jsonRpcMethods(
transactionPool,
miningConfiguration,
miningCoordinator,
metricsSystem,
rpcMetrics,
supportedCapabilities,
accountAllowlistController,
nodeAllowlistController,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -203,7 +203,7 @@ public Map<String, JsonRpcMethod> methods() {
transactionPool,
miningConfiguration,
miningCoordinator,
metricsSystem,
rpcMetrics,
new HashSet<>(),
accountWhitelistController,
nodeWhitelistController,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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();

Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
Expand All @@ -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)
Expand Down
Loading
Loading