From 33b88befc7e771961e4e1b371aa402644e6bc438 Mon Sep 17 00:00:00 2001 From: courtneyeh Date: Fri, 13 Oct 2023 17:41:44 +1100 Subject: [PATCH] Refactoring logic into ValidatorDutyMetrics --- .../client/ValidatorClientService.java | 18 +++++- .../validator/client/duties/DutyType.java | 6 +- .../duties/SlotBasedScheduledDuties.java | 56 ++----------------- .../client/duties/ValidatorDutyMetrics.java | 51 +++++++++++++++++ .../client/AttestationDutySchedulerTest.java | 3 +- .../client/BlockDutySchedulerTest.java | 2 +- .../duties/SlotBasedScheduledDutiesTest.java | 2 +- 7 files changed, 77 insertions(+), 61 deletions(-) create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/ValidatorDutyMetrics.java 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 96ca4a0cc00..7d5cb4ce2e3 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 @@ -20,6 +20,7 @@ import java.util.List; import java.util.Optional; import java.util.Random; +import java.util.function.Function; import java.util.function.Supplier; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -53,7 +54,10 @@ import tech.pegasys.teku.validator.client.doppelganger.DoppelgangerDetector; import tech.pegasys.teku.validator.client.duties.BeaconCommitteeSubscriptions; import tech.pegasys.teku.validator.client.duties.BlockDutyFactory; +import tech.pegasys.teku.validator.client.duties.Duty; +import tech.pegasys.teku.validator.client.duties.DutyResult; import tech.pegasys.teku.validator.client.duties.SlotBasedScheduledDuties; +import tech.pegasys.teku.validator.client.duties.ValidatorDutyMetrics; import tech.pegasys.teku.validator.client.duties.attestations.AttestationDutyFactory; import tech.pegasys.teku.validator.client.duties.synccommittee.ChainHeadTracker; import tech.pegasys.teku.validator.client.duties.synccommittee.SyncCommitteeScheduledDuties; @@ -419,6 +423,15 @@ private void scheduleValidatorsDuties( new AttestationDutyFactory(spec, forkProvider, validatorApiChannel); final BeaconCommitteeSubscriptions beaconCommitteeSubscriptions = new BeaconCommitteeSubscriptions(validatorApiChannel); + + final Function> dutyFunction; + if (metricsOn) { + dutyFunction = Duty::performDuty; + } else { + final ValidatorDutyMetrics metrics = ValidatorDutyMetrics.create(metricsSystem); + dutyFunction = metrics::performDutyWithMetrics; + } + final DutyLoader attestationDutyLoader = new RetryingDutyLoader<>( asyncRunner, @@ -427,7 +440,7 @@ private void scheduleValidatorsDuties( forkProvider, dependentRoot -> new SlotBasedScheduledDuties<>( - attestationDutyFactory, dependentRoot, metricsSystem, metricsOn), + attestationDutyFactory, dependentRoot, dutyFunction), validators, validatorIndexProvider, beaconCommitteeSubscriptions, @@ -438,8 +451,7 @@ private void scheduleValidatorsDuties( new BlockProductionDutyLoader( validatorApiChannel, dependentRoot -> - new SlotBasedScheduledDuties<>( - blockDutyFactory, dependentRoot, metricsSystem, metricsOn), + new SlotBasedScheduledDuties<>(blockDutyFactory, dependentRoot, dutyFunction), validators, validatorIndexProvider)); validatorTimingChannels.add(new BlockDutyScheduler(metricsSystem, blockDutyLoader, spec)); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/DutyType.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/DutyType.java index b5583d05e69..ffe5764fdb0 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/DutyType.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/DutyType.java @@ -14,9 +14,9 @@ package tech.pegasys.teku.validator.client.duties; public enum DutyType { - ATTESTATION_AGGREGATION("attestation aggregation"), - ATTESTATION_PRODUCTION("attestation production"), - BLOCK_PRODUCTION("block production"); + ATTESTATION_AGGREGATION("attestation_aggregation"), + ATTESTATION_PRODUCTION("attestation_production"), + BLOCK_PRODUCTION("block_production"); private final String type; 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 c8109533067..7424e1f26c5 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,11 +18,7 @@ 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; @@ -34,36 +30,15 @@ public class SlotBasedScheduledDuties

implements private final DutyFactory dutyFactory; private final Bytes32 dependentRoot; - private final OperationTimer attestationProductionDutyTimer; - private final OperationTimer blockProductionDutyTimer; - private final OperationTimer attestationAggregationDutyTimer; - - private final boolean metricsOn; + private final Function> dutyFunction; public SlotBasedScheduledDuties( final DutyFactory dutyFactory, final Bytes32 dependentRoot, - final MetricsSystem metricsSystem, - final boolean metricsOn) { + final Function> dutyFunction) { this.dutyFactory = dutyFactory; this.dependentRoot = dependentRoot; - this.metricsOn = metricsOn; - - 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"); + this.dutyFunction = dutyFunction; } public Bytes32 getDependentRoot() { @@ -119,30 +94,7 @@ private SafeFuture performDutyForSlot( return SafeFuture.completedFuture(DutyResult.NO_OP); } - 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; - }); + return dutyFunction.apply(duty); } private void discardDutiesBeforeSlot( diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/ValidatorDutyMetrics.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/ValidatorDutyMetrics.java new file mode 100644 index 00000000000..bdd57d3b9e0 --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/ValidatorDutyMetrics.java @@ -0,0 +1,51 @@ +/* + * Copyright Consensys Software Inc., 2023 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.validator.client.duties; + +import org.hyperledger.besu.plugin.services.MetricsSystem; +import org.hyperledger.besu.plugin.services.metrics.LabelledMetric; +import org.hyperledger.besu.plugin.services.metrics.OperationTimer; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.metrics.TekuMetricCategory; + +public class ValidatorDutyMetrics { + private final LabelledMetric dutyMetric; + + ValidatorDutyMetrics(final LabelledMetric dutyMetric) { + this.dutyMetric = dutyMetric; + } + + public static ValidatorDutyMetrics create(final MetricsSystem metricsSystem) { + final LabelledMetric dutyMetric = + metricsSystem.createLabelledTimer( + TekuMetricCategory.VALIDATOR, + "duty_timer", + "Timer recording the time taken to perform a duty", + "type", + "step"); + return new ValidatorDutyMetrics(dutyMetric); + } + + public SafeFuture performDutyWithMetrics(final Duty duty) { + final String dutyType = duty.getType().getType(); + final OperationTimer timer = dutyMetric.labels(dutyType, "total"); + final OperationTimer.TimingContext context = timer.startTimer(); + return duty.performDuty() + .thenApply( + result -> { + context.stopTimer(); + return result; + }); + } +} 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 0362a2f0f52..bfe3134f6cc 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 @@ -44,6 +44,7 @@ import tech.pegasys.teku.validator.api.AttesterDuties; import tech.pegasys.teku.validator.api.AttesterDuty; import tech.pegasys.teku.validator.client.duties.BeaconCommitteeSubscriptions; +import tech.pegasys.teku.validator.client.duties.Duty; import tech.pegasys.teku.validator.client.duties.DutyResult; import tech.pegasys.teku.validator.client.duties.SlotBasedScheduledDuties; import tech.pegasys.teku.validator.client.duties.attestations.AggregationDuty; @@ -748,7 +749,7 @@ private void createDutySchedulerWithRealDuties() { forkProvider, dependentRoot -> new SlotBasedScheduledDuties<>( - attestationDutyFactory, dependentRoot, metricsSystem, false), + attestationDutyFactory, dependentRoot, Duty::performDuty), 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 76b464bccf4..ebeb04e4998 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 @@ -323,7 +323,7 @@ private void createDutySchedulerWithRealDuties() { validatorApiChannel, dependentRoot -> new SlotBasedScheduledDuties<>( - blockDutyFactory, dependentRoot, metricsSystem, false), + blockDutyFactory, dependentRoot, Duty::performDuty), 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 c9f6abe4bd5..bc6250f76bc 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 @@ -51,7 +51,7 @@ class SlotBasedScheduledDutiesTest { private final SlotBasedScheduledDuties duties = new SlotBasedScheduledDuties<>( - dutyFactory, Bytes32.fromHexString("0x838382"), metricsSystem, false); + dutyFactory, Bytes32.fromHexString("0x838382"), Duty::performDuty); @Test public void shouldDiscardMissedProductionDuties() {