From 4bb1ee500aca14ca329efce87f444164cf39283c Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 31 May 2019 18:02:51 +1000 Subject: [PATCH] [PIE-1578] added local transaction permissioning metrics (#1515) * added local transaction permissioning metrics Signed-off-by: Adrian Sutton --- ...untLocalConfigPermissioningController.java | 40 +++++++++- ...ocalConfigPermissioningControllerTest.java | 76 +++++++++++++++++-- .../tech/pegasys/pantheon/RunnerBuilder.java | 2 +- 3 files changed, 105 insertions(+), 13 deletions(-) diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningController.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningController.java index 2227ffcb6b..cd2528d06a 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningController.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningController.java @@ -15,6 +15,9 @@ package tech.pegasys.pantheon.ethereum.permissioning; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.Transaction; import tech.pegasys.pantheon.ethereum.permissioning.account.TransactionPermissioningProvider; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.io.IOException; @@ -37,19 +40,40 @@ public class AccountLocalConfigPermissioningController implements TransactionPer private List accountWhitelist = new ArrayList<>(); private final WhitelistPersistor whitelistPersistor; + private final Counter checkCounter; + private final Counter checkCounterPermitted; + private final Counter checkCounterUnpermitted; + public AccountLocalConfigPermissioningController( - final LocalPermissioningConfiguration configuration) { + final LocalPermissioningConfiguration configuration, final MetricsSystem metricsSystem) { this( configuration, - new WhitelistPersistor(configuration.getAccountPermissioningConfigFilePath())); + new WhitelistPersistor(configuration.getAccountPermissioningConfigFilePath()), + metricsSystem); } public AccountLocalConfigPermissioningController( final LocalPermissioningConfiguration configuration, - final WhitelistPersistor whitelistPersistor) { + final WhitelistPersistor whitelistPersistor, + final MetricsSystem metricsSystem) { this.configuration = configuration; this.whitelistPersistor = whitelistPersistor; readAccountsFromConfig(configuration); + this.checkCounter = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "account_local_check_count", + "Number of times the account local permissioning provider has been checked"); + this.checkCounterPermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "account_local_check_count_permitted", + "Number of times the account local permissioning provider has been checked and returned permitted"); + this.checkCounterUnpermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "account_local_check_count_unpermitted", + "Number of times the account local permissioning provider has been checked and returned unpermitted"); } private void readAccountsFromConfig(final LocalPermissioningConfiguration configuration) { @@ -208,11 +232,19 @@ public class AccountLocalConfigPermissioningController implements TransactionPer @Override public boolean isPermitted(final Transaction transaction) { + this.checkCounter.inc(); final Address sender = transaction.getSender(); if (sender == null) { + this.checkCounterUnpermitted.inc(); return false; } else { - return contains(sender.toString()); + if (contains(sender.toString())) { + this.checkCounterPermitted.inc(); + return true; + } else { + this.checkCounterUnpermitted.inc(); + return false; + } } } } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningControllerTest.java index fed0e12184..c0bfb7154c 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningControllerTest.java @@ -25,6 +25,9 @@ import static org.mockito.Mockito.when; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.Transaction; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -46,11 +49,35 @@ public class AccountLocalConfigPermissioningControllerTest { private AccountLocalConfigPermissioningController controller; @Mock private LocalPermissioningConfiguration permissioningConfig; @Mock private WhitelistPersistor whitelistPersistor; + @Mock private MetricsSystem metricsSystem; + @Mock private Counter checkCounter; + @Mock private Counter checkPermittedCounter; + @Mock private Counter checkUnpermittedCounter; @Before public void before() { + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "account_local_check_count", + "Number of times the account local permissioning provider has been checked")) + .thenReturn(checkCounter); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "account_local_check_count_permitted", + "Number of times the account local permissioning provider has been checked and returned permitted")) + .thenReturn(checkPermittedCounter); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "account_local_check_count_unpermitted", + "Number of times the account local permissioning provider has been checked and returned unpermitted")) + .thenReturn(checkUnpermittedCounter); + controller = - new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor); + new AccountLocalConfigPermissioningController( + permissioningConfig, whitelistPersistor, metricsSystem); } @Test @@ -59,7 +86,8 @@ public class AccountLocalConfigPermissioningControllerTest { when(permissioningConfig.getAccountWhitelist()) .thenReturn(singletonList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); controller = - new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor); + new AccountLocalConfigPermissioningController( + permissioningConfig, whitelistPersistor, metricsSystem); assertThat(controller.getAccountWhitelist()) .contains("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"); @@ -71,7 +99,8 @@ public class AccountLocalConfigPermissioningControllerTest { when(permissioningConfig.getAccountWhitelist()) .thenReturn(singletonList("0xFE3B557E8Fb62b89F4916B721be55cEb828dBd73")); controller = - new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor); + new AccountLocalConfigPermissioningController( + permissioningConfig, whitelistPersistor, metricsSystem); assertThat(controller.getAccountWhitelist()) .containsExactly("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"); @@ -82,7 +111,8 @@ public class AccountLocalConfigPermissioningControllerTest { when(permissioningConfig.isAccountWhitelistEnabled()).thenReturn(true); when(permissioningConfig.getAccountWhitelist()).thenReturn(new ArrayList<>()); controller = - new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor); + new AccountLocalConfigPermissioningController( + permissioningConfig, whitelistPersistor, metricsSystem); assertThat(controller.contains("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")).isFalse(); } @@ -245,7 +275,8 @@ public class AccountLocalConfigPermissioningControllerTest { when(permissioningConfig.getAccountWhitelist()) .thenReturn(Arrays.asList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); controller = - new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor); + new AccountLocalConfigPermissioningController( + permissioningConfig, whitelistPersistor, metricsSystem); controller.reload(); @@ -259,7 +290,8 @@ public class AccountLocalConfigPermissioningControllerTest { when(permissioningConfig.getAccountWhitelist()) .thenReturn(Arrays.asList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); controller = - new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor); + new AccountLocalConfigPermissioningController( + permissioningConfig, whitelistPersistor, metricsSystem); final Throwable thrown = catchThrowable(() -> controller.reload()); @@ -290,7 +322,8 @@ public class AccountLocalConfigPermissioningControllerTest { when(permissioningConfig.getAccountWhitelist()) .thenReturn(singletonList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); controller = - new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor); + new AccountLocalConfigPermissioningController( + permissioningConfig, whitelistPersistor, metricsSystem); assertThat(controller.contains("0xFE3B557E8Fb62b89F4916B721be55cEb828dBd73")).isTrue(); } @@ -301,15 +334,20 @@ public class AccountLocalConfigPermissioningControllerTest { when(permissioningConfig.getAccountWhitelist()) .thenReturn(singletonList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); controller = - new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor); + new AccountLocalConfigPermissioningController( + permissioningConfig, whitelistPersistor, metricsSystem); final Transaction transaction = mock(Transaction.class); when(transaction.getSender()) .thenReturn(Address.fromHexString("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); + verifyCountersUntouched(); + boolean permitted = controller.isPermitted(transaction); assertThat(permitted).isTrue(); + + verifyCountersPermitted(); } @Test @@ -317,9 +355,13 @@ public class AccountLocalConfigPermissioningControllerTest { final Transaction transactionWithoutSender = mock(Transaction.class); when(transactionWithoutSender.getSender()).thenReturn(null); + verifyCountersUntouched(); + boolean permitted = controller.isPermitted(transactionWithoutSender); assertThat(permitted).isFalse(); + + verifyCountersUnpermitted(); } private Path createPermissionsFileWithAccount(final String account) throws IOException { @@ -329,4 +371,22 @@ public class AccountLocalConfigPermissioningControllerTest { Files.write(permissionsFile, nodePermissionsFileContent.getBytes(StandardCharsets.UTF_8)); return permissionsFile; } + + private void verifyCountersUntouched() { + verify(checkCounter, times(0)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); + } + + private void verifyCountersPermitted() { + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(1)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); + } + + private void verifyCountersUnpermitted() { + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(1)).inc(); + } } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java index d469561881..8feb2b0d36 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java @@ -295,7 +295,7 @@ public class RunnerBuilder { .map( configuration -> { final AccountLocalConfigPermissioningController whitelistController = - new AccountLocalConfigPermissioningController(configuration); + new AccountLocalConfigPermissioningController(configuration, metricsSystem); transactionPool.setAccountFilter(whitelistController::contains); return whitelistController; });