From 2341ba15508107541fc267ffef1e733ce4276dc9 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 31 May 2019 13:49:26 +1000 Subject: [PATCH] [PIE-1577] added node local metrics (#1514) * added node local metrics Signed-off-by: Adrian Sutton --- .../ethereum/p2p/network/P2PNetworkTest.java | 2 +- ...odeLocalConfigPermissioningController.java | 41 +++++++++-- .../NodePermissioningControllerFactory.java | 5 +- ...ocalConfigPermissioningControllerTest.java | 71 +++++++++++++++++-- 4 files changed, 106 insertions(+), 13 deletions(-) diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java index e713fded9f..eed39ba9ad 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java @@ -279,7 +279,7 @@ public class P2PNetworkTest { final NodeLocalConfigPermissioningController localWhitelistController = new NodeLocalConfigPermissioningController( - config, Collections.emptyList(), selfEnode.getNodeId()); + config, Collections.emptyList(), selfEnode.getNodeId(), new NoOpMetricsSystem()); // turn on whitelisting by adding a different node NOT remote node localWhitelistController.addNode( EnodeURL.builder() diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java index 607acf5b1d..608cc7853d 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java @@ -14,6 +14,9 @@ package tech.pegasys.pantheon.ethereum.permissioning; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningProvider; import tech.pegasys.pantheon.ethereum.permissioning.node.NodeWhitelistUpdatedEvent; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.Subscribers; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.enode.EnodeURL; @@ -46,27 +49,50 @@ public class NodeLocalConfigPermissioningController implements NodePermissioning private final Subscribers> nodeWhitelistUpdatedObservers = new Subscribers<>(); + private final Counter checkCounter; + private final Counter checkCounterPermitted; + private final Counter checkCounterUnpermitted; + public NodeLocalConfigPermissioningController( final LocalPermissioningConfiguration permissioningConfiguration, final List fixedNodes, - final BytesValue localNodeId) { + final BytesValue localNodeId, + final MetricsSystem metricsSystem) { this( permissioningConfiguration, fixedNodes, localNodeId, - new WhitelistPersistor(permissioningConfiguration.getNodePermissioningConfigFilePath())); + new WhitelistPersistor(permissioningConfiguration.getNodePermissioningConfigFilePath()), + metricsSystem); } public NodeLocalConfigPermissioningController( final LocalPermissioningConfiguration configuration, final List fixedNodes, final BytesValue localNodeId, - final WhitelistPersistor whitelistPersistor) { + final WhitelistPersistor whitelistPersistor, + final MetricsSystem metricsSystem) { this.configuration = configuration; this.fixedNodes = fixedNodes; this.localNodeId = localNodeId; this.whitelistPersistor = whitelistPersistor; readNodesFromConfig(configuration); + + this.checkCounter = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_local_check_count", + "Number of times the node local permissioning provider has been checked"); + this.checkCounterPermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_local_check_count_permitted", + "Number of times the node local permissioning provider has been checked and returned permitted"); + this.checkCounterUnpermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_local_check_count_unpermitted", + "Number of times the node local permissioning provider has been checked and returned unpermitted"); } private void readNodesFromConfig(final LocalPermissioningConfiguration configuration) { @@ -297,6 +323,13 @@ public class NodeLocalConfigPermissioningController implements NodePermissioning @Override public boolean isPermitted(final EnodeURL sourceEnode, final EnodeURL destinationEnode) { - return isPermitted(sourceEnode) && isPermitted(destinationEnode); + this.checkCounter.inc(); + if (isPermitted(sourceEnode) && isPermitted(destinationEnode)) { + this.checkCounterPermitted.inc(); + return true; + } else { + this.checkCounterUnpermitted.inc(); + return false; + } } } diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java index 823bd298e1..1acf07ef30 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java @@ -45,7 +45,10 @@ public class NodePermissioningControllerFactory { if (localPermissioningConfiguration.isNodeWhitelistEnabled()) { NodeLocalConfigPermissioningController localProvider = new NodeLocalConfigPermissioningController( - localPermissioningConfiguration, new ArrayList<>(fixedNodes), localNodeId); + localPermissioningConfiguration, + new ArrayList<>(fixedNodes), + localNodeId, + metricsSystem); providers.add(localProvider); } } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java index fbb70c4258..568d762a2b 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java @@ -27,6 +27,9 @@ import static org.mockito.Mockito.when; import static tech.pegasys.pantheon.ethereum.permissioning.NodeLocalConfigPermissioningController.NodesWhitelistResult; import tech.pegasys.pantheon.ethereum.permissioning.node.NodeWhitelistUpdatedEvent; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.io.IOException; @@ -47,7 +50,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -@RunWith(MockitoJUnitRunner.class) +@RunWith(MockitoJUnitRunner.StrictStubs.class) public class NodeLocalConfigPermissioningControllerTest { @Mock private WhitelistPersistor whitelistPersistor; @@ -61,16 +64,40 @@ public class NodeLocalConfigPermissioningControllerTest { private final EnodeURL selfEnode = EnodeURL.fromString( "enode://5f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.10:1111"); + @Mock private MetricsSystem metricsSystem; + @Mock private Counter checkCounter; + @Mock private Counter checkPermittedCounter; + @Mock private Counter checkUnpermittedCounter; @Before public void setUp() { bootnodesList.clear(); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_local_check_count", + "Number of times the node local permissioning provider has been checked")) + .thenReturn(checkCounter); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_local_check_count_permitted", + "Number of times the node local permissioning provider has been checked and returned permitted")) + .thenReturn(checkPermittedCounter); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_local_check_count_unpermitted", + "Number of times the node local permissioning provider has been checked and returned unpermitted")) + .thenReturn(checkUnpermittedCounter); + controller = new NodeLocalConfigPermissioningController( LocalPermissioningConfiguration.createDefault(), bootnodesList, selfEnode.getNodeId(), - whitelistPersistor); + whitelistPersistor, + metricsSystem); } @Test @@ -165,8 +192,14 @@ public class NodeLocalConfigPermissioningControllerTest { "enode://bbbb80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303"; controller.addNodes(Arrays.asList(peer1)); - assertThat(controller.isPermitted(peer2)).isFalse(); + + verifyCountersUntouched(); + + assertThat(controller.isPermitted(EnodeURL.fromString(peer2), EnodeURL.fromString(peer1))) + .isFalse(); + + verifyCountersUnpermitted(); } @Test @@ -261,7 +294,13 @@ public class NodeLocalConfigPermissioningControllerTest { @Test public void whenCheckingIfNodeIsPermittedOrderDoesNotMatter() { controller.addNodes(Arrays.asList(enode1)); + + verifyCountersUntouched(); + assertThat(controller.isPermitted(EnodeURL.fromString(enode1), selfEnode)).isTrue(); + + verifyCountersPermitted(); + assertThat(controller.isPermitted(selfEnode, EnodeURL.fromString(enode1))).isTrue(); } @@ -302,7 +341,7 @@ public class NodeLocalConfigPermissioningControllerTest { .thenReturn(Arrays.asList(URI.create(expectedEnodeURL))); controller = new NodeLocalConfigPermissioningController( - permissioningConfig, bootnodesList, selfEnode.getNodeId()); + permissioningConfig, bootnodesList, selfEnode.getNodeId(), metricsSystem); controller.reload(); @@ -322,7 +361,7 @@ public class NodeLocalConfigPermissioningControllerTest { .thenReturn(Arrays.asList(URI.create(expectedEnodeURI))); controller = new NodeLocalConfigPermissioningController( - permissioningConfig, bootnodesList, selfEnode.getNodeId()); + permissioningConfig, bootnodesList, selfEnode.getNodeId(), metricsSystem); final Throwable thrown = catchThrowable(() -> controller.reload()); @@ -421,7 +460,7 @@ public class NodeLocalConfigPermissioningControllerTest { when(permissioningConfig.getNodeWhitelist()).thenReturn(Arrays.asList(URI.create(enode1))); controller = new NodeLocalConfigPermissioningController( - permissioningConfig, bootnodesList, selfEnode.getNodeId()); + permissioningConfig, bootnodesList, selfEnode.getNodeId(), metricsSystem); controller.subscribeToListUpdatedEvent(consumer); controller.reload(); @@ -444,7 +483,7 @@ public class NodeLocalConfigPermissioningControllerTest { when(permissioningConfig.getNodeWhitelist()).thenReturn(Arrays.asList(URI.create(enode1))); controller = new NodeLocalConfigPermissioningController( - permissioningConfig, bootnodesList, selfEnode.getNodeId()); + permissioningConfig, bootnodesList, selfEnode.getNodeId(), metricsSystem); controller.subscribeToListUpdatedEvent(consumer); controller.reload(); @@ -459,4 +498,22 @@ public class NodeLocalConfigPermissioningControllerTest { 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(); + } }