From 079c04c7d529b896775fe311f86a18e1d32768ad Mon Sep 17 00:00:00 2001 From: courtneyeh Date: Tue, 10 Oct 2023 14:01:43 +1100 Subject: [PATCH] Add timing metrics to SlotBasedScheduledDuties --- .../client/ValidatorClientService.java | 6 +- .../duties/SlotBasedScheduledDuties.java | 57 ++++++++++++++++++- .../client/AttestationDutySchedulerTest.java | 4 +- .../client/BlockDutySchedulerTest.java | 3 +- .../duties/SlotBasedScheduledDutiesTest.java | 4 +- 5 files changed, 66 insertions(+), 8 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index f6052d4ca03..3edc6d19ae9 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -409,7 +409,8 @@ private void scheduleValidatorsDuties( validatorApiChannel, forkProvider, dependentRoot -> - new SlotBasedScheduledDuties<>(attestationDutyFactory, dependentRoot), + new SlotBasedScheduledDuties<>( + attestationDutyFactory, dependentRoot, metricsSystem), validators, validatorIndexProvider, beaconCommitteeSubscriptions, @@ -419,7 +420,8 @@ private void scheduleValidatorsDuties( asyncRunner, new BlockProductionDutyLoader( validatorApiChannel, - dependentRoot -> new SlotBasedScheduledDuties<>(blockDutyFactory, dependentRoot), + dependentRoot -> + new SlotBasedScheduledDuties<>(blockDutyFactory, dependentRoot, metricsSystem), validators, validatorIndexProvider)); validatorTimingChannels.add(new BlockDutyScheduler(metricsSystem, blockDutyLoader, spec)); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/SlotBasedScheduledDuties.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/SlotBasedScheduledDuties.java index 070e28929ce..4071cb70895 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/SlotBasedScheduledDuties.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/SlotBasedScheduledDuties.java @@ -18,7 +18,11 @@ import java.util.function.Consumer; import java.util.function.Function; import org.apache.tuweni.bytes.Bytes32; +import org.hyperledger.besu.plugin.services.MetricsSystem; +import org.hyperledger.besu.plugin.services.metrics.OperationTimer; import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; +import tech.pegasys.teku.infrastructure.metrics.TekuMetricCategory; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.validator.client.Validator; @@ -30,10 +34,32 @@ public class SlotBasedScheduledDuties

implements private final DutyFactory dutyFactory; private final Bytes32 dependentRoot; + private final OperationTimer attestationProductionDutyTimer; + private final OperationTimer blockProductionDutyTimer; + private final OperationTimer attestationAggregationDutyTimer; + public SlotBasedScheduledDuties( - final DutyFactory dutyFactory, final Bytes32 dependentRoot) { + final DutyFactory dutyFactory, + final Bytes32 dependentRoot, + final MetricsSystem metricsSystem) { this.dutyFactory = dutyFactory; this.dependentRoot = dependentRoot; + + this.attestationProductionDutyTimer = + metricsSystem.createTimer( + TekuMetricCategory.VALIDATOR, + "attestation_duty_timer", + "Timer recording the time taken to perform an attestation duty"); + this.blockProductionDutyTimer = + metricsSystem.createTimer( + TekuMetricCategory.VALIDATOR, + "block_duty_timer", + "Timer recording the time taken to perform a block duty"); + this.attestationAggregationDutyTimer = + metricsSystem.createTimer( + TekuMetricCategory.VALIDATOR, + "aggregation_duty_timer", + "Timer recording the time taken to perform an aggregation duty"); } public Bytes32 getDependentRoot() { @@ -84,11 +110,36 @@ private SafeFuture performDutyForSlot( final NavigableMap duties, final UInt64 slot) { discardDutiesBeforeSlot(duties, slot); - final Duty duty = duties.remove(slot); // todo this is the duty! + final Duty duty = duties.remove(slot); if (duty == null) { return SafeFuture.completedFuture(DutyResult.NO_OP); } - return duty.performDuty(); // todo bring into method and override to do metrics + + final boolean metricsOn = true; // todo work out if metrics are turned on + return metricsOn ? performDutyWithMetrics(duty) : duty.performDuty(); + } + + private OperationTimer getMetricTimer(final DutyType type) { + if (type.equals(DutyType.ATTESTATION_AGGREGATION)) { + return attestationAggregationDutyTimer; + } else if (type.equals(DutyType.ATTESTATION_PRODUCTION)) { + return attestationProductionDutyTimer; + } else if (type.equals(DutyType.BLOCK_PRODUCTION)) { + return blockProductionDutyTimer; + } else { + throw new InvalidConfigurationException(type.getType() + " is an invalid duty type"); + } + } + + private SafeFuture performDutyWithMetrics(final Duty duty) { + final OperationTimer timer = getMetricTimer(duty.getType()); + final OperationTimer.TimingContext context = timer.startTimer(); + return duty.performDuty() + .thenApply( + result -> { + context.stopTimer(); + return result; + }); } private void discardDutiesBeforeSlot( diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/AttestationDutySchedulerTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/AttestationDutySchedulerTest.java index 0d94b329080..a66c892ad7e 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/AttestationDutySchedulerTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/AttestationDutySchedulerTest.java @@ -746,7 +746,9 @@ private void createDutySchedulerWithRealDuties() { new AttestationDutyLoader( validatorApiChannel, forkProvider, - dependentRoot -> new SlotBasedScheduledDuties<>(attestationDutyFactory, dependentRoot), + dependentRoot -> + new SlotBasedScheduledDuties<>( + attestationDutyFactory, dependentRoot, metricsSystem), new OwnedValidators(Map.of(VALIDATOR1_KEY, validator1, VALIDATOR2_KEY, validator2)), validatorIndexProvider, beaconCommitteeSubscriptions, diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/BlockDutySchedulerTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/BlockDutySchedulerTest.java index 4f8339a18ed..0b729b16697 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/BlockDutySchedulerTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/BlockDutySchedulerTest.java @@ -322,7 +322,8 @@ private void createDutySchedulerWithRealDuties() { new BlockProductionDutyLoader( validatorApiChannel, dependentRoot -> - new SlotBasedScheduledDuties<>(blockDutyFactory, dependentRoot), + new SlotBasedScheduledDuties<>( + blockDutyFactory, dependentRoot, metricsSystem), new OwnedValidators( Map.of(VALIDATOR1_KEY, validator1, VALIDATOR2_KEY, validator2)), validatorIndexProvider)), diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/SlotBasedScheduledDutiesTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/SlotBasedScheduledDutiesTest.java index ca3aaca3064..07d5f15ac8b 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/SlotBasedScheduledDutiesTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/SlotBasedScheduledDutiesTest.java @@ -26,6 +26,7 @@ import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.Test; import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.metrics.StubMetricsSystem; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.TestSpecFactory; @@ -37,6 +38,7 @@ class SlotBasedScheduledDutiesTest { private final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec); public static final UInt64 TWO = UInt64.valueOf(2); private final Validator validator = mock(Validator.class); + final StubMetricsSystem metricsSystem = new StubMetricsSystem(); @SuppressWarnings("unchecked") private final Function productionDutyAdder = mock(Function.class); @@ -48,7 +50,7 @@ class SlotBasedScheduledDutiesTest { private final DutyFactory dutyFactory = mock(DutyFactory.class); private final SlotBasedScheduledDuties duties = - new SlotBasedScheduledDuties<>(dutyFactory, Bytes32.fromHexString("0x838382")); + new SlotBasedScheduledDuties<>(dutyFactory, Bytes32.fromHexString("0x838382"), metricsSystem); @Test public void shouldDiscardMissedProductionDuties() {