Skip to content

Commit

Permalink
fix: Add query handling metrics upload (#15900)
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Hess <[email protected]>
  • Loading branch information
mhess-swl authored Oct 9, 2024
1 parent 9cebd43 commit 14c7538
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package com.hedera.node.app.workflows.handle.metric;
package com.hedera.node.app.workflows;

import static java.util.Objects.requireNonNull;

Expand All @@ -34,10 +34,10 @@
import javax.inject.Singleton;

/**
* A class to handle the metrics for the handle-workflow
* A class to handle the metrics for all operations (transactions and queries)
*/
@Singleton
public class HandleWorkflowMetrics {
public class OpWorkflowMetrics {

private static final BinaryOperator<Integer> AVERAGE = (sum, count) -> count == 0 ? 0 : sum / count;

Expand All @@ -53,12 +53,12 @@ public class HandleWorkflowMetrics {
private long gasUsedThisConsensusSecond = 0L;

/**
* Constructor for the HandleWorkflowMetrics
* Constructor for the OpWorkflowMetrics
*
* @param metrics the {@link Metrics} object where all metrics will be registered
*/
@Inject
public HandleWorkflowMetrics(@NonNull final Metrics metrics, @NonNull final ConfigProvider configProvider) {
public OpWorkflowMetrics(@NonNull final Metrics metrics, @NonNull final ConfigProvider configProvider) {
requireNonNull(metrics, "metrics must not be null");
requireNonNull(configProvider, "configProvider must not be null");

Expand Down Expand Up @@ -88,9 +88,9 @@ public HandleWorkflowMetrics(@NonNull final Metrics metrics, @NonNull final Conf
* Update the metrics for the given functionality
*
* @param functionality the {@link HederaFunctionality} for which the metrics will be updated
* @param duration the duration of the transaction in {@code ns}
* @param duration the duration of the operation in {@code ns}
*/
public void updateTransactionDuration(@NonNull final HederaFunctionality functionality, final int duration) {
public void updateDuration(@NonNull final HederaFunctionality functionality, final int duration) {
requireNonNull(functionality, "functionality must not be null");
if (functionality == HederaFunctionality.NONE) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@
import com.hedera.node.app.store.WritableStoreFactory;
import com.hedera.node.app.throttle.NetworkUtilizationManager;
import com.hedera.node.app.throttle.ThrottleServiceManager;
import com.hedera.node.app.workflows.OpWorkflowMetrics;
import com.hedera.node.app.workflows.TransactionInfo;
import com.hedera.node.app.workflows.dispatcher.TransactionDispatcher;
import com.hedera.node.app.workflows.handle.cache.CacheWarmer;
import com.hedera.node.app.workflows.handle.dispatch.ChildDispatchFactory;
import com.hedera.node.app.workflows.handle.metric.HandleWorkflowMetrics;
import com.hedera.node.app.workflows.handle.record.RecordStreamBuilder;
import com.hedera.node.app.workflows.handle.record.SystemSetup;
import com.hedera.node.app.workflows.handle.steps.HollowAccountCompletions;
Expand Down Expand Up @@ -117,7 +117,7 @@ public class HandleWorkflow {
private final BlockRecordManager blockRecordManager;
private final BlockStreamManager blockStreamManager;
private final CacheWarmer cacheWarmer;
private final HandleWorkflowMetrics handleWorkflowMetrics;
private final OpWorkflowMetrics opWorkflowMetrics;
private final ThrottleServiceManager throttleServiceManager;
private final SemanticVersion version;
private final InitTrigger initTrigger;
Expand Down Expand Up @@ -148,7 +148,7 @@ public HandleWorkflow(
@NonNull final BlockRecordManager blockRecordManager,
@NonNull final BlockStreamManager blockStreamManager,
@NonNull final CacheWarmer cacheWarmer,
@NonNull final HandleWorkflowMetrics handleWorkflowMetrics,
@NonNull final OpWorkflowMetrics opWorkflowMetrics,
@NonNull final ThrottleServiceManager throttleServiceManager,
@NonNull final SemanticVersion version,
@NonNull final InitTrigger initTrigger,
Expand Down Expand Up @@ -176,7 +176,7 @@ public HandleWorkflow(
this.blockRecordManager = requireNonNull(blockRecordManager);
this.blockStreamManager = requireNonNull(blockStreamManager);
this.cacheWarmer = requireNonNull(cacheWarmer);
this.handleWorkflowMetrics = requireNonNull(handleWorkflowMetrics);
this.opWorkflowMetrics = requireNonNull(opWorkflowMetrics);
this.throttleServiceManager = requireNonNull(throttleServiceManager);
this.version = requireNonNull(version);
this.initTrigger = requireNonNull(initTrigger);
Expand Down Expand Up @@ -323,8 +323,7 @@ private void handlePlatformTransaction(
if (blockStreamConfig.streamBlocks()) {
handleOutput.blocksItemsOrThrow().forEach(blockStreamManager::writeItem);
}
handleWorkflowMetrics.updateTransactionDuration(
userTxn.functionality(), (int) (System.nanoTime() - handleStart));
opWorkflowMetrics.updateDuration(userTxn.functionality(), (int) (System.nanoTime() - handleStart));
}

/**
Expand Down Expand Up @@ -450,7 +449,7 @@ private void updateWorkflowMetrics(@NonNull final UserTxn userTxn) {
if (userTxn.type() == GENESIS_TRANSACTION
|| userTxn.consensusNow().getEpochSecond()
> userTxn.lastHandledConsensusTime().getEpochSecond()) {
handleWorkflowMetrics.switchConsensusSecond();
opWorkflowMetrics.switchConsensusSecond();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
import com.hedera.node.app.throttle.CongestionThrottleService;
import com.hedera.node.app.throttle.NetworkUtilizationManager;
import com.hedera.node.app.throttle.ThrottleServiceManager;
import com.hedera.node.app.workflows.OpWorkflowMetrics;
import com.hedera.node.app.workflows.handle.Dispatch;
import com.hedera.node.app.workflows.handle.metric.HandleWorkflowMetrics;
import com.hedera.node.config.data.ContractsConfig;
import com.swirlds.state.spi.info.NetworkInfo;
import edu.umd.cs.findbugs.annotations.NonNull;
Expand All @@ -54,18 +54,18 @@ public class DispatchUsageManager {
EnumSet.of(HederaFunctionality.CONTRACT_CREATE, HederaFunctionality.CONTRACT_CALL, ETHEREUM_TRANSACTION);

private final NetworkInfo networkInfo;
private final HandleWorkflowMetrics handleWorkflowMetrics;
private final OpWorkflowMetrics opWorkflowMetrics;
private final ThrottleServiceManager throttleServiceManager;
private final NetworkUtilizationManager networkUtilizationManager;

@Inject
public DispatchUsageManager(
@NonNull final NetworkInfo networkInfo,
@NonNull final HandleWorkflowMetrics handleWorkflowMetrics,
@NonNull final OpWorkflowMetrics opWorkflowMetrics,
@NonNull final ThrottleServiceManager throttleServiceManager,
@NonNull final NetworkUtilizationManager networkUtilizationManager) {
this.networkInfo = requireNonNull(networkInfo);
this.handleWorkflowMetrics = requireNonNull(handleWorkflowMetrics);
this.opWorkflowMetrics = requireNonNull(opWorkflowMetrics);
this.throttleServiceManager = requireNonNull(throttleServiceManager);
this.networkUtilizationManager = requireNonNull(networkUtilizationManager);
}
Expand Down Expand Up @@ -134,7 +134,7 @@ private void leakUnusedGas(@NonNull final Dispatch dispatch) {
// EVM action tracer to get a better estimate of the actual gas used and the gas limit.
if (builder.hasContractResult()) {
final var gasUsed = builder.getGasUsedForContractTxn();
handleWorkflowMetrics.addGasUsed(gasUsed);
opWorkflowMetrics.addGasUsed(gasUsed);
final var contractsConfig = dispatch.config().getConfigData(ContractsConfig.class);
if (contractsConfig.throttleThrottleByGas()) {
final var txnInfo = dispatch.txnInfo();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import com.hedera.node.app.spi.workflows.QueryHandler;
import com.hedera.node.app.store.ReadableStoreFactory;
import com.hedera.node.app.throttle.SynchronizedThrottleAccumulator;
import com.hedera.node.app.workflows.OpWorkflowMetrics;
import com.hedera.node.app.workflows.ingest.IngestChecker;
import com.hedera.node.app.workflows.ingest.SubmissionManager;
import com.hedera.node.config.ConfigProvider;
Expand Down Expand Up @@ -101,6 +102,7 @@ public final class QueryWorkflowImpl implements QueryWorkflow {
private final FeeManager feeManager;
private final SynchronizedThrottleAccumulator synchronizedThrottleAccumulator;
private final InstantSource instantSource;
private final OpWorkflowMetrics workflowMetrics;

/**
* Constructor of {@code QueryWorkflowImpl}
Expand Down Expand Up @@ -135,7 +137,8 @@ public QueryWorkflowImpl(
@NonNull final ExchangeRateManager exchangeRateManager,
@NonNull final FeeManager feeManager,
@NonNull final SynchronizedThrottleAccumulator synchronizedThrottleAccumulator,
@NonNull final InstantSource instantSource) {
@NonNull final InstantSource instantSource,
@NonNull final OpWorkflowMetrics workflowMetrics) {
this.stateAccessor = requireNonNull(stateAccessor, "stateAccessor must not be null");
this.submissionManager = requireNonNull(submissionManager, "submissionManager must not be null");
this.ingestChecker = requireNonNull(ingestChecker, "ingestChecker must not be null");
Expand All @@ -150,10 +153,13 @@ public QueryWorkflowImpl(
this.synchronizedThrottleAccumulator =
requireNonNull(synchronizedThrottleAccumulator, "hapiThrottling must not be null");
this.instantSource = requireNonNull(instantSource);
this.workflowMetrics = requireNonNull(workflowMetrics);
}

@Override
public void handleQuery(@NonNull final Bytes requestBuffer, @NonNull final BufferedData responseBuffer) {
final long queryStart = System.nanoTime();

requireNonNull(requestBuffer);
requireNonNull(responseBuffer);

Expand Down Expand Up @@ -295,6 +301,8 @@ public void handleQuery(@NonNull final Bytes requestBuffer, @NonNull final Buffe
logger.warn("Unexpected IO exception while writing protobuf", e);
throw new StatusRuntimeException(Status.INTERNAL);
}

workflowMetrics.updateDuration(function, (int) (System.nanoTime() - queryStart));
}

private Query parseQuery(Bytes requestBuffer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
import com.hedera.node.app.state.HederaRecordCache;
import com.hedera.node.app.throttle.NetworkUtilizationManager;
import com.hedera.node.app.throttle.ThrottleServiceManager;
import com.hedera.node.app.workflows.OpWorkflowMetrics;
import com.hedera.node.app.workflows.dispatcher.TransactionDispatcher;
import com.hedera.node.app.workflows.handle.cache.CacheWarmer;
import com.hedera.node.app.workflows.handle.dispatch.ChildDispatchFactory;
import com.hedera.node.app.workflows.handle.metric.HandleWorkflowMetrics;
import com.hedera.node.app.workflows.handle.record.SystemSetup;
import com.hedera.node.app.workflows.handle.steps.HollowAccountCompletions;
import com.hedera.node.app.workflows.handle.steps.NodeStakeUpdates;
Expand Down Expand Up @@ -112,7 +112,7 @@ class HandleWorkflowTest {
private CacheWarmer cacheWarmer;

@Mock
private HandleWorkflowMetrics handleWorkflowMetrics;
private OpWorkflowMetrics opWorkflowMetrics;

@Mock
private ThrottleServiceManager throttleServiceManager;
Expand Down Expand Up @@ -173,7 +173,7 @@ void setUp() {
blockRecordManager,
blockStreamManager,
cacheWarmer,
handleWorkflowMetrics,
opWorkflowMetrics,
throttleServiceManager,
version,
initTrigger,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@

import com.hedera.hapi.node.base.HederaFunctionality;
import com.hedera.node.app.utils.TestUtils;
import com.hedera.node.app.workflows.handle.metric.HandleWorkflowMetrics;
import com.hedera.node.app.workflows.OpWorkflowMetrics;
import com.hedera.node.config.ConfigProvider;
import com.hedera.node.config.VersionedConfigImpl;
import com.hedera.node.config.testfixtures.HederaTestConfigBuilder;
import com.swirlds.metrics.api.Metrics;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class HandleWorkflowMetricsTest {
class OpWorkflowMetricsTest {

private final Metrics metrics = TestUtils.metrics();
private ConfigProvider configProvider;
Expand All @@ -44,15 +44,14 @@ void setUp() {
@SuppressWarnings("DataFlowIssue")
@Test
void testConstructorWithInvalidArguments() {
assertThatThrownBy(() -> new HandleWorkflowMetrics(null, configProvider))
.isInstanceOf(NullPointerException.class);
assertThatThrownBy(() -> new HandleWorkflowMetrics(metrics, null)).isInstanceOf(NullPointerException.class);
assertThatThrownBy(() -> new OpWorkflowMetrics(null, configProvider)).isInstanceOf(NullPointerException.class);
assertThatThrownBy(() -> new OpWorkflowMetrics(metrics, null)).isInstanceOf(NullPointerException.class);
}

@Test
void testConstructorInitializesMetrics() {
// when
new HandleWorkflowMetrics(metrics, configProvider);
new OpWorkflowMetrics(metrics, configProvider);

// then
final int transactionMetricsCount = (HederaFunctionality.values().length - 1) * 2;
Expand All @@ -62,7 +61,7 @@ void testConstructorInitializesMetrics() {
@Test
void testInitialValue() {
// given
new HandleWorkflowMetrics(metrics, configProvider);
new OpWorkflowMetrics(metrics, configProvider);

// then
assertThat(metrics.getMetric("app", "cryptoCreateDurationMax").get(VALUE))
Expand All @@ -73,22 +72,22 @@ void testInitialValue() {

@SuppressWarnings("DataFlowIssue")
@Test
void testUpdateTransactionDurationWithInvalidArguments() {
void testUpdateDurationWithInvalidArguments() {
// given
final var handleWorkflowMetrics = new HandleWorkflowMetrics(metrics, configProvider);
final var handleWorkflowMetrics = new OpWorkflowMetrics(metrics, configProvider);

// when
assertThatThrownBy(() -> handleWorkflowMetrics.updateTransactionDuration(null, 0))
assertThatThrownBy(() -> handleWorkflowMetrics.updateDuration(null, 0))
.isInstanceOf(NullPointerException.class);
}

@Test
void testUpdateTransactionDurationSingleUpdate() {
// given
final var handleWorkflowMetrics = new HandleWorkflowMetrics(metrics, configProvider);
final var handleWorkflowMetrics = new OpWorkflowMetrics(metrics, configProvider);

// when
handleWorkflowMetrics.updateTransactionDuration(HederaFunctionality.CRYPTO_CREATE, 42);
handleWorkflowMetrics.updateDuration(HederaFunctionality.CRYPTO_CREATE, 42);

// then
assertThat(metrics.getMetric("app", "cryptoCreateDurationMax").get(VALUE))
Expand All @@ -98,13 +97,13 @@ void testUpdateTransactionDurationSingleUpdate() {
}

@Test
void testUpdateTransactionDurationTwoUpdates() {
void testUpdateDurationTwoUpdates() {
// given
final var handleWorkflowMetrics = new HandleWorkflowMetrics(metrics, configProvider);
final var handleWorkflowMetrics = new OpWorkflowMetrics(metrics, configProvider);

// when
handleWorkflowMetrics.updateTransactionDuration(HederaFunctionality.CRYPTO_CREATE, 11);
handleWorkflowMetrics.updateTransactionDuration(HederaFunctionality.CRYPTO_CREATE, 22);
handleWorkflowMetrics.updateDuration(HederaFunctionality.CRYPTO_CREATE, 11);
handleWorkflowMetrics.updateDuration(HederaFunctionality.CRYPTO_CREATE, 22);

// then
assertThat(metrics.getMetric("app", "cryptoCreateDurationMax").get(VALUE))
Expand All @@ -114,14 +113,14 @@ void testUpdateTransactionDurationTwoUpdates() {
}

@Test
void testUpdateTransactionDurationThreeUpdates() {
void testUpdateDurationThreeUpdates() {
// given
final var handleWorkflowMetrics = new HandleWorkflowMetrics(metrics, configProvider);
final var handleWorkflowMetrics = new OpWorkflowMetrics(metrics, configProvider);

// when
handleWorkflowMetrics.updateTransactionDuration(HederaFunctionality.CRYPTO_CREATE, 13);
handleWorkflowMetrics.updateTransactionDuration(HederaFunctionality.CRYPTO_CREATE, 5);
handleWorkflowMetrics.updateTransactionDuration(HederaFunctionality.CRYPTO_CREATE, 3);
handleWorkflowMetrics.updateDuration(HederaFunctionality.CRYPTO_CREATE, 13);
handleWorkflowMetrics.updateDuration(HederaFunctionality.CRYPTO_CREATE, 5);
handleWorkflowMetrics.updateDuration(HederaFunctionality.CRYPTO_CREATE, 3);

// then
assertThat(metrics.getMetric("app", "cryptoCreateDurationMax").get(VALUE))
Expand All @@ -133,7 +132,7 @@ void testUpdateTransactionDurationThreeUpdates() {
@Test
void testInitialStartConsensusRound() {
// given
final var handleWorkflowMetrics = new HandleWorkflowMetrics(metrics, configProvider);
final var handleWorkflowMetrics = new OpWorkflowMetrics(metrics, configProvider);

// when
handleWorkflowMetrics.switchConsensusSecond();
Expand All @@ -146,7 +145,7 @@ void testInitialStartConsensusRound() {
@Test
void testUpdateGasZero() {
// given
final var handleWorkflowMetrics = new HandleWorkflowMetrics(metrics, configProvider);
final var handleWorkflowMetrics = new OpWorkflowMetrics(metrics, configProvider);

// when
handleWorkflowMetrics.addGasUsed(0L);
Expand All @@ -160,7 +159,7 @@ void testUpdateGasZero() {
@Test
void testUpdateGas() {
// given
final var handleWorkflowMetrics = new HandleWorkflowMetrics(metrics, configProvider);
final var handleWorkflowMetrics = new OpWorkflowMetrics(metrics, configProvider);

// when
handleWorkflowMetrics.addGasUsed(1_000_000L);
Expand Down
Loading

0 comments on commit 14c7538

Please sign in to comment.