Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[PIE-1578] added local transaction permissioning metrics #1515

Merged
merged 6 commits into from
May 31, 2019
Merged

[PIE-1578] added local transaction permissioning metrics #1515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f2ed643
added local transaction permissioning metrics
macfarla May 31, 2019
24b92b2
added local transaction permissioning metrics
macfarla May 31, 2019
29901ac
Merge branch 'master' of github.com:PegaSysEng/pantheon into pie-1578…
macfarla May 31, 2019
0cf09fa
s/transaction/account
macfarla May 31, 2019
a3b401d
Merge branch 'master' of github.com:PegaSysEng/pantheon into pie-1578…
macfarla May 31, 2019
33cbb77
s/transaction/account
macfarla May 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions ...ch/pegasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
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;
Expand All @@ -37,19 +40,40 @@ public class AccountLocalConfigPermissioningController implements TransactionPer
private List<String> 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) {
Expand Down Expand Up @@ -208,11 +232,19 @@ private List<String> normalizeAccounts(final List<String> accounts) {

@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;
}
}
}
}
76 changes: 68 additions & 8 deletions ...egasys/pantheon/ethereum/permissioning/AccountLocalConfigPermissioningControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@

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;
Expand All @@ -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
Expand All @@ -59,7 +86,8 @@ public void whenPermConfigHasAccountsShouldAddAllAccountsToWhitelist() {
when(permissioningConfig.getAccountWhitelist())
.thenReturn(singletonList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"));
controller =
new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor);
new AccountLocalConfigPermissioningController(
permissioningConfig, whitelistPersistor, metricsSystem);

assertThat(controller.getAccountWhitelist())
.contains("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73");
Expand All @@ -71,7 +99,8 @@ public void whenLoadingAccountsFromConfigShouldNormalizeAccountsToLowerCase() {
when(permissioningConfig.getAccountWhitelist())
.thenReturn(singletonList("0xFE3B557E8Fb62b89F4916B721be55cEb828dBd73"));
controller =
new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor);
new AccountLocalConfigPermissioningController(
permissioningConfig, whitelistPersistor, metricsSystem);

assertThat(controller.getAccountWhitelist())
.containsExactly("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73");
Expand All @@ -82,7 +111,8 @@ public void whenPermConfigContainsEmptyListOfAccountsContainsShouldReturnFalse()
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();
}
Expand Down Expand Up @@ -245,7 +275,8 @@ public void reloadAccountWhitelistWithValidConfigFileShouldUpdateWhitelist() thr
when(permissioningConfig.getAccountWhitelist())
.thenReturn(Arrays.asList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"));
controller =
new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor);
new AccountLocalConfigPermissioningController(
permissioningConfig, whitelistPersistor, metricsSystem);

controller.reload();

Expand All @@ -259,7 +290,8 @@ public void reloadAccountWhitelistWithErrorReadingConfigFileShouldKeepOldWhiteli
when(permissioningConfig.getAccountWhitelist())
.thenReturn(Arrays.asList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"));
controller =
new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor);
new AccountLocalConfigPermissioningController(
permissioningConfig, whitelistPersistor, metricsSystem);

final Throwable thrown = catchThrowable(() -> controller.reload());

Expand Down Expand Up @@ -290,7 +322,8 @@ public void shouldMatchAccountsWithInconsistentCasing() {
when(permissioningConfig.getAccountWhitelist())
.thenReturn(singletonList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"));
controller =
new AccountLocalConfigPermissioningController(permissioningConfig, whitelistPersistor);
new AccountLocalConfigPermissioningController(
permissioningConfig, whitelistPersistor, metricsSystem);

assertThat(controller.contains("0xFE3B557E8Fb62b89F4916B721be55cEb828dBd73")).isTrue();
}
Expand All @@ -301,25 +334,34 @@ public void isPermittedShouldCheckIfAccountExistInTheWhitelist() {
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
public void isPermittedShouldReturnFalseIfTransactionDoesNotContainSender() {
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 {
Expand All @@ -329,4 +371,22 @@ private Path createPermissionsFileWithAccount(final String account) throws IOExc
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();
}
}
2 changes: 1 addition & 1 deletion pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ public Runner build() {
.map(
configuration -> {
final AccountLocalConfigPermissioningController whitelistController =
new AccountLocalConfigPermissioningController(configuration);
new AccountLocalConfigPermissioningController(configuration, metricsSystem);
transactionPool.setAccountFilter(whitelistController::contains);
return whitelistController;
});
Expand Down