Include add reason to exported metrics

Signed-off-by: Fabio Di Fabio <[email protected]>
hyperledger · Sep 4, 2024 · 26bcb4d · 26bcb4d
1 parent 7e6a9c9
commit 26bcb4d
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 23 deletions.
diff --git a/.../src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolMetrics.java b/.../src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolMetrics.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.ethereum.eth.transactions;
 
 import org.hyperledger.besu.datatypes.TransactionType;
+import org.hyperledger.besu.ethereum.eth.transactions.layered.TransactionsLayer.AddReason;
 import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
 import org.hyperledger.besu.metrics.BesuMetricCategory;
 import org.hyperledger.besu.metrics.ReplaceableDoubleSupplier;
@@ -68,6 +69,7 @@ public TransactionPoolMetrics(final MetricsSystem metricsSystem) {
             "Count of transactions added to the transaction pool",
             "source",
             "priority",
+            "reason",
             "layer");
 
     removedCounter =
@@ -215,11 +217,13 @@ public void initExpiredMessagesCounter(final String message) {
             SKIPPED_MESSAGES_LOGGING_THRESHOLD));
   }
 
-  public void incrementAdded(final PendingTransaction pendingTransaction, final String layer) {
+  public void incrementAdded(
+      final PendingTransaction pendingTransaction, final AddReason addReason, final String layer) {
     addedCounter
         .labels(
             location(pendingTransaction.isReceivedFromLocalSource()),
             priority(pendingTransaction.hasPriority()),
+            addReason.label(),
             layer)
         .inc();
   }

diff --git a/...ava/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java b/...ava/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java
@@ -186,7 +186,7 @@ public TransactionAddedResult add(
     if (addStatus.isSuccess()) {
       final var addedPendingTransaction =
           addReason.makeCopy() ? pendingTransaction.detachedCopy() : pendingTransaction;
-      processAdded(addedPendingTransaction);
+      processAdded(addedPendingTransaction, addReason);
       addStatus.maybeReplacedTransaction().ifPresent(this::replaced);
 
       nextLayer.notifyAdded(addedPendingTransaction);
@@ -245,7 +245,7 @@ private void tryFillGap(
               pendingTransaction.getNonce(),
               remainingPromotionsPerType);
       if (promotedTx != null) {
-        processAdded(promotedTx);
+        processAdded(promotedTx, AddReason.PROMOTED);
         if (!maybeFull()) {
           tryFillGap(ADDED, promotedTx, remainingPromotionsPerType);
         }
@@ -331,12 +331,12 @@ protected TransactionAddedResult addToNextLayer(
     return nextLayer.add(pendingTransaction, nextLayerDistance, addReason);
   }
 
-  private void processAdded(final PendingTransaction addedTx) {
+  private void processAdded(final PendingTransaction addedTx, final AddReason addReason) {
     pendingTransactions.put(addedTx.getHash(), addedTx);
     final var senderTxs = txsBySender.computeIfAbsent(addedTx.getSender(), s -> new TreeMap<>());
     senderTxs.put(addedTx.getNonce(), addedTx);
     increaseCounters(addedTx);
-    metrics.incrementAdded(addedTx, name());
+    metrics.incrementAdded(addedTx, addReason, name());
     internalAdd(senderTxs, addedTx);
   }
 
@@ -468,7 +468,7 @@ final void promoteTransactions() {
       nextLayer
           .promote(
               this::promotionFilter, cacheFreeSpace(), freeSlots, getRemainingPromotionsPerType())
-          .forEach(this::processAdded);
+          .forEach(addedTx -> processAdded(addedTx, AddReason.PROMOTED));
     }
   }
 

diff --git a/...c/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/TransactionsLayer.java b/...c/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/TransactionsLayer.java
@@ -125,14 +125,18 @@ enum AddReason {
     /** When adding a tx, that is not present in the pool. */
     NEW(true, true),
     /** When adding a tx as result of an internal move between layers. */
-    MOVE(false, false);
+    MOVE(false, false),
+    /** When adding a tx as result of a promotion from a lower layer. */
+    PROMOTED(false, false);
 
     private final boolean sendNotification;
     private final boolean makeCopy;
+    private final String label;
 
     AddReason(final boolean sendNotification, final boolean makeCopy) {
       this.sendNotification = sendNotification;
       this.makeCopy = makeCopy;
+      this.label = name().toLowerCase(Locale.ROOT);
     }
 
     /**
@@ -153,6 +157,10 @@ public boolean sendNotification() {
     public boolean makeCopy() {
       return makeCopy;
     }
+
+    public String label() {
+      return label;
+    }
   }
 
   enum RemovalReason {

diff --git a/.../java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseTransactionPoolTest.java b/.../java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseTransactionPoolTest.java
@@ -35,6 +35,7 @@
 import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction;
 import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolMetrics;
+import org.hyperledger.besu.ethereum.eth.transactions.layered.TransactionsLayer.AddReason;
 import org.hyperledger.besu.evm.account.Account;
 import org.hyperledger.besu.metrics.StubMetricsSystem;
 import org.hyperledger.besu.testutil.DeterministicEthScheduler;
@@ -258,9 +259,10 @@ protected void addLocalTransactions(
     }
   }
 
-  protected long getAddedCount(final String source, final String priority, final String layer) {
+  protected long getAddedCount(
+      final String source, final String priority, final AddReason addReason, final String layer) {
     return metricsSystem.getCounterValue(
-        TransactionPoolMetrics.ADDED_COUNTER_NAME, source, priority, layer);
+        TransactionPoolMetrics.ADDED_COUNTER_NAME, source, priority, addReason.label(), layer);
   }
 
   protected long getRemovedCount(

diff --git a/...rg/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java b/...rg/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java
@@ -20,6 +20,7 @@
 import static org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult.ALREADY_KNOWN;
 import static org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult.NONCE_TOO_FAR_IN_FUTURE_FOR_SENDER;
 import static org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult.REJECTED_UNDERPRICED_REPLACEMENT;
+import static org.hyperledger.besu.ethereum.eth.transactions.layered.TransactionsLayer.AddReason.NEW;
 import static org.hyperledger.besu.ethereum.eth.transactions.layered.TransactionsLayer.RemovalReason.DROPPED;
 import static org.hyperledger.besu.ethereum.eth.transactions.layered.TransactionsLayer.RemovalReason.REPLACED;
 import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.GAS_PRICE_BELOW_CURRENT_BASE_FEE;
@@ -207,14 +208,14 @@ public void addRemoteTransactions() {
         createRemotePendingTransaction(transaction0), Optional.empty());
     assertThat(pendingTransactions.size()).isEqualTo(1);
 
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(1);
 
     pendingTransactions.addTransaction(
         createRemotePendingTransaction(transaction1), Optional.empty());
     assertThat(pendingTransactions.size()).isEqualTo(2);
 
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(2);
   }
 
@@ -574,7 +575,7 @@ public void replaceTransactionWithSameSenderAndNonce() {
     assertTransactionPending(pendingTransactions, transaction1b);
     assertTransactionPending(pendingTransactions, transaction2);
     assertThat(pendingTransactions.size()).isEqualTo(2);
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(3);
     assertThat(
             getRemovedCount(
@@ -612,7 +613,7 @@ public void replaceTransactionWithSameSenderAndNonce_multipleReplacements() {
     assertTransactionPending(pendingTransactions, independentTx);
 
     assertThat(pendingTransactions.size()).isEqualTo(2);
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(replacedTxCount + 2);
     assertThat(
             getRemovedCount(
@@ -660,9 +661,9 @@ public void replaceTransactionWithSameSenderAndNonce_multipleReplacements() {
 
     final int localDuplicateCount = replacedTxCount - remoteDuplicateCount;
     assertThat(pendingTransactions.size()).isEqualTo(2);
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(remoteDuplicateCount + 1);
-    assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(LOCAL, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(localDuplicateCount + 1);
     assertThat(
             getRemovedCount(
@@ -814,36 +815,40 @@ public void shouldNotIncrementAddedCounterWhenRemoteTransactionAlreadyPresent()
     pendingTransactions.addTransaction(
         createLocalPendingTransaction(transaction0), Optional.empty());
     assertThat(pendingTransactions.size()).isEqualTo(1);
-    assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(LOCAL, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(1);
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())).isZero();
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
+        .isZero();
 
     assertThat(
             pendingTransactions.addTransaction(
                 createRemotePendingTransaction(transaction0), Optional.empty()))
         .isEqualTo(ALREADY_KNOWN);
     assertThat(pendingTransactions.size()).isEqualTo(1);
-    assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(LOCAL, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(1);
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())).isZero();
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
+        .isZero();
   }
 
   @Test
   public void shouldNotIncrementAddedCounterWhenLocalTransactionAlreadyPresent() {
     pendingTransactions.addTransaction(
         createRemotePendingTransaction(transaction0), Optional.empty());
     assertThat(pendingTransactions.size()).isEqualTo(1);
-    assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name())).isZero();
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(LOCAL, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
+        .isZero();
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(1);
 
     assertThat(
             pendingTransactions.addTransaction(
                 createLocalPendingTransaction(transaction0), Optional.empty()))
         .isEqualTo(ALREADY_KNOWN);
     assertThat(pendingTransactions.size()).isEqualTo(1);
-    assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name())).isZero();
-    assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name()))
+    assertThat(getAddedCount(LOCAL, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
+        .isZero();
+    assertThat(getAddedCount(REMOTE, NO_PRIORITY, NEW, layers.prioritizedTransactions.name()))
         .isEqualTo(1);
   }