Skip to content

Commit

Permalink
feat: SCSC metrics for #TXs that fail pureCheck (P1 level)
Browse files Browse the repository at this point in the history
- counts for CONTRACT_CREATE / CONTRACT_CALL / ETHEREUM_TRANSACTION that fail pureCheck for any reason
- and for those that fail pureCheck because they don't even have the intrinsic gas amount
- and for Type 3 format (BLOB) Ethereum transactions (that we don't support)

By:
- Change plumbing for metrics into smart contract service

Signed-off-by: David S Bakin <[email protected]>
  • Loading branch information
david-bakin-sl committed Dec 2, 2024
1 parent f881978 commit 0650a76
Show file tree
Hide file tree
Showing 21 changed files with 492 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.hedera.node.app.spi.throttle.Throttle;
import com.swirlds.common.crypto.Signature;
import com.swirlds.config.api.Configuration;
import com.swirlds.metrics.api.Metrics;
import com.swirlds.state.lifecycle.Service;
import com.swirlds.state.lifecycle.info.NodeInfo;
import edu.umd.cs.findbugs.annotations.NonNull;
Expand Down Expand Up @@ -105,6 +106,12 @@ public Signature sign(final byte[] ledgerId) {
*/
Supplier<NodeInfo> selfNodeInfoSupplier();

/**
* The supplier of (platform) metrics
* @return the supplier
*/
Supplier<Metrics> metricsSupplier();

/**
* The application's strategy for creating {@link Throttle} instances.
* @return the throttle factory
Expand Down
1 change: 1 addition & 0 deletions hedera-node/hedera-app-spi/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
requires transitive com.hedera.node.hapi;
requires transitive com.swirlds.common;
requires transitive com.swirlds.config.api;
requires transitive com.swirlds.metrics.api;
requires transitive com.swirlds.state.api;
requires transitive com.hedera.pbj.runtime;
requires static com.github.spotbugs.annotations;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.swirlds.common.crypto.Hash;
import com.swirlds.common.metrics.noop.NoOpMetrics;
import com.swirlds.metrics.api.Metrics;
import com.swirlds.platform.state.service.PlatformStateService;
import com.swirlds.platform.state.service.schemas.V0540PlatformStateSchema;
import com.swirlds.platform.system.Round;
Expand Down Expand Up @@ -104,6 +105,7 @@ public class BlockStreamManagerBenchmark {
private static final String SAMPLE_BLOCK = "sample.blk.gz";
private static final Instant FAKE_CONSENSUS_NOW = Instant.ofEpochSecond(1_234_567L, 890);
private static final Timestamp FAKE_CONSENSUS_TIME = new Timestamp(1_234_567L, 890);
private static final Metrics NO_OP_METRICS = new NoOpMetrics();
private static final SemanticVersion VERSION = new SemanticVersion(0, 56, 0, "", "");

public static void main(String... args) throws Exception {
Expand All @@ -124,6 +126,7 @@ public static void main(String... args) throws Exception {
UNAVAILABLE_GOSSIP,
configProvider::getConfiguration,
() -> DEFAULT_NODE_INFO,
() -> NO_OP_METRICS,
(split, snapshots) -> {
throw new UnsupportedOperationException();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.swirlds.common.crypto.Hash;
import com.swirlds.common.metrics.noop.NoOpMetrics;
import com.swirlds.metrics.api.Metrics;
import com.swirlds.platform.state.service.PlatformStateService;
import com.swirlds.platform.state.service.schemas.V0540PlatformStateSchema;
import com.swirlds.platform.system.Round;
Expand Down Expand Up @@ -84,6 +85,7 @@ public class StandaloneRoundManagement {
private static final String SAMPLE_BLOCK = "sample.blk.gz";
private static final Instant FAKE_CONSENSUS_NOW = Instant.ofEpochSecond(1_234_567L, 890);
private static final Timestamp FAKE_CONSENSUS_TIME = new Timestamp(1_234_567L, 890);
private static final Metrics NO_OP_METRICS = new NoOpMetrics();
private static final SemanticVersion VERSION = new SemanticVersion(0, 56, 0, "", "");

private static final int NUM_ROUNDS = 10000;
Expand All @@ -100,6 +102,7 @@ public class StandaloneRoundManagement {
UNAVAILABLE_GOSSIP,
configProvider::getConfiguration,
() -> DEFAULT_NODE_INFO,
() -> NO_OP_METRICS,
(split, snapshots) -> {
throw new UnsupportedOperationException();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ public Hedera(
this,
configSupplier,
() -> daggerApp.networkInfo().selfNodeInfo(),
() -> metrics,

Check warning on line 432 in hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java#L432

Added line #L432 was not covered by tests
new AppThrottleFactory(
configSupplier,
() -> daggerApp.workingStateAccessor().getState(),
Expand Down Expand Up @@ -602,6 +603,8 @@ public void onStateInitialized(
trigger,
RosterUtils.buildAddressBook(platform.getRoster()),
platform.getContext().getConfiguration());

contractServiceImpl.registerMetrics();

Check warning on line 607 in hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java#L607

Added line #L607 was not covered by tests
}
// With the States API grounded in the working state, we can create the object graph from it
initializeDagger(state, trigger);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.hedera.node.app.spi.signatures.SignatureVerifier;
import com.hedera.node.app.spi.throttle.Throttle;
import com.swirlds.config.api.Configuration;
import com.swirlds.metrics.api.Metrics;
import com.swirlds.state.lifecycle.info.NodeInfo;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.time.InstantSource;
Expand All @@ -39,5 +40,6 @@ public record AppContextImpl(
@NonNull Gossip gossip,
@NonNull Supplier<Configuration> configSupplier,
@NonNull Supplier<NodeInfo> selfNodeInfoSupplier,
@NonNull Supplier<Metrics> metricsSupplier,
@NonNull Throttle.Factory throttleFactory)
implements AppContext {}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.swirlds.common.crypto.CryptographyHolder;
import com.swirlds.common.metrics.noop.NoOpMetrics;
import com.swirlds.metrics.api.Metrics;
import com.swirlds.state.State;
import com.swirlds.state.lifecycle.info.NodeInfo;
import edu.umd.cs.findbugs.annotations.NonNull;
Expand Down Expand Up @@ -111,6 +112,7 @@ private ExecutorComponent newExecutorComponent(
UNAVAILABLE_GOSSIP,
bootstrapConfigProvider::getConfiguration,
() -> DEFAULT_NODE_INFO,
() -> NO_OP_METRICS,

Check warning on line 115 in hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/standalone/TransactionExecutors.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/standalone/TransactionExecutors.java#L115

Added line #L115 was not covered by tests
new AppThrottleFactory(
configProvider::getConfiguration,
() -> state,
Expand All @@ -122,7 +124,7 @@ private ExecutorComponent newExecutorComponent(
ForkJoinPool.commonPool(),
new TssLibraryImpl(appContext),
ForkJoinPool.commonPool(),
new NoOpMetrics());
NO_OP_METRICS);
final var contractService = new ContractServiceImpl(appContext, NOOP_VERIFICATION_STRATEGIES, tracerBinding);
final var fileService = new FileServiceImpl();
final var scheduleService = new ScheduleServiceImpl();
Expand All @@ -133,7 +135,7 @@ private ExecutorComponent newExecutorComponent(
.fileServiceImpl(fileService)
.contractServiceImpl(contractService)
.scheduleServiceImpl(scheduleService)
.metrics(new NoOpMetrics())
.metrics(NO_OP_METRICS)
.throttleFactory(appContext.throttleFactory())
.build();
componentRef.set(component);
Expand All @@ -159,4 +161,6 @@ public List<OperationTracer> get() {
return OPERATION_TRACERS.get();
}
}

private static final Metrics NO_OP_METRICS = new NoOpMetrics();
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ class IngestComponentTest {

private HederaInjectionComponent app;

private static final Metrics NO_OP_METRICS = new NoOpMetrics();

@BeforeEach
void setUp() {
final Configuration configuration = HederaTestConfigBuilder.createConfig();
Expand All @@ -125,6 +127,7 @@ void setUp() {
UNAVAILABLE_GOSSIP,
() -> configuration,
() -> DEFAULT_NODE_INFO,
() -> NO_OP_METRICS,
throttleFactory);
given(tssBaseService.tssHandlers())
.willReturn(new TssHandlers(tssMessageHandler, tssVoteHandler, tssShareSignatureHandler));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ private State genesisState(@NonNull final Map<String, String> overrides) {
UNAVAILABLE_GOSSIP,
() -> config,
() -> DEFAULT_NODE_INFO,
() -> NO_OP_METRICS,
new AppThrottleFactory(
() -> config, () -> state, () -> ThrottleDefinitions.DEFAULT, ThrottleAccumulator::new));
registerServices(appContext, config, servicesRegistry);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.hedera.node.app.service.contract.impl;

import com.hedera.node.app.service.contract.impl.exec.metrics.ContractMetrics;
import com.hedera.node.app.service.contract.impl.exec.scope.VerificationStrategies;
import com.hedera.node.app.service.contract.impl.handlers.ContractHandlers;
import com.hedera.node.app.spi.signatures.SignatureVerifier;
Expand Down Expand Up @@ -50,11 +51,17 @@ ContractServiceComponent create(
@BindsInstance InstantSource instantSource,
@BindsInstance SignatureVerifier signatureVerifier,
@BindsInstance VerificationStrategies verificationStrategies,
@BindsInstance @Nullable Supplier<List<OperationTracer>> addOnTracers);
@BindsInstance @Nullable Supplier<List<OperationTracer>> addOnTracers,
@BindsInstance ContractMetrics contractMetrics);
}

/**
* @return all contract transaction handlers
*/
ContractHandlers handlers();

/**
* @return contract metrics collection, instance
*/
ContractMetrics contractMetrics();
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static java.util.Objects.requireNonNull;

import com.hedera.node.app.service.contract.ContractService;
import com.hedera.node.app.service.contract.impl.exec.metrics.ContractMetrics;
import com.hedera.node.app.service.contract.impl.exec.scope.DefaultVerificationStrategies;
import com.hedera.node.app.service.contract.impl.exec.scope.VerificationStrategies;
import com.hedera.node.app.service.contract.impl.handlers.ContractHandlers;
Expand Down Expand Up @@ -61,14 +62,17 @@ public ContractServiceImpl(
@Nullable final VerificationStrategies verificationStrategies,
@Nullable final Supplier<List<OperationTracer>> addOnTracers) {
requireNonNull(appContext);
final var metricsSupplier = requireNonNull(appContext.metricsSupplier());
final var contractMetrics = new ContractMetrics(metricsSupplier);
this.component = DaggerContractServiceComponent.factory()
.create(
appContext.instantSource(),
// (FUTURE) Inject the signature verifier instance into the IsAuthorizedSystemContract
// C.f. https://github.com/hashgraph/hedera-services/issues/14248
appContext.signatureVerifier(),
Optional.ofNullable(verificationStrategies).orElseGet(DefaultVerificationStrategies::new),
addOnTracers);
addOnTracers,
contractMetrics);
}

@Override
Expand All @@ -77,10 +81,25 @@ public void registerSchemas(@NonNull final SchemaRegistry registry) {
registry.register(new V0500ContractSchema());
}

/**
* Create the metrics for the smart contracts service. This needs to be delayed until _after_
* the metrics are available - which happens after `Hedera.initializeStatesApi`.
*/
public void registerMetrics() {
component.contractMetrics().createContractMetrics();
}

Check warning on line 90 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/ContractServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/ContractServiceImpl.java#L89-L90

Added lines #L89 - L90 were not covered by tests

/**
* @return all contract transaction handlers
*/
public ContractHandlers handlers() {
return component.handlers();
}

/**
* @return the contract metrics collector
*/
public ContractMetrics contractMetrics() {
return component.contractMetrics();

Check warning on line 103 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/ContractServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/ContractServiceImpl.java#L103

Added line #L103 was not covered by tests
}
}
Loading

0 comments on commit 0650a76

Please sign in to comment.