metrics: count large txn groups and msigs by algorandskiy · Pull Request #4833 · algorand/go-algorand

algorandskiy · 2022-11-24T01:24:04Z

Summary

Add more metrics to better understand traffic.

Test Plan

Existing tests

codecov · 2022-11-24T01:48:50Z

Codecov Report

Merging #4833 (468cc32) into master (1cf857f) will decrease coverage by 0.04%.
The diff coverage is 45.45%.

@@            Coverage Diff             @@
##           master    #4833      +/-   ##
==========================================
- Coverage   53.80%   53.76%   -0.05%     
==========================================
  Files         421      421              
  Lines       53827    53849      +22     
==========================================
- Hits        28963    28952      -11     
- Misses      22516    22542      +26     
- Partials     2348     2355       +7

Impacted Files	Coverage Δ
crypto/batchverifier.go	`100.00% <ø> (ø)`
data/txHandler.go	`31.81% <0.00%> (-0.74%)`	⬇️
data/transactions/verify/txn.go	`74.46% <55.55%> (-1.57%)`	⬇️
ledger/roundlru.go	`90.56% <0.00%> (-5.67%)`	⬇️
ledger/tracker.go	`74.26% <0.00%> (-4.65%)`	⬇️
network/wsPeer.go	`67.06% <0.00%> (-1.91%)`	⬇️
catchup/peerSelector.go	`98.95% <0.00%> (-1.05%)`	⬇️
catchup/service.go	`68.96% <0.00%> (-0.50%)`	⬇️
network/wsNetwork.go	`64.74% <0.00%> (ø)`
ledger/blockqueue.go	`88.50% <0.00%> (+2.87%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

algonautshant

Looks great.
Some minor comments and one possible bug?

algonautshant · 2022-11-24T04:07:28Z

 var logicErrTotal = metrics.MakeCounter(metrics.MetricName{Name: "algod_ledger_logic_err", Description: "Total transaction scripts executed and errored"})
+var msigLessOrEqual4 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_4", Description: "Total transactions with 1-4 msigs"})
+var msigLessOrEqual16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_4_16", Description: "Total transactions with 4-16 msigs"})
+var msigMore16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_16", Description: "Total transactions with 16+ msigs"})


nit: (personal preference...)

Suggested change

var msigMore16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_16", Description: "Total transactions with 16+ msigs"})

var msig17orMore = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_17", Description: "Total transactions with 17 or more msigs"})

algonautshant · 2022-11-24T04:08:55Z

+var msigMore16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_16", Description: "Total transactions with 16+ msigs"})
+var msigLsigLessOrEqual4 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_lsig_4", Description: "Total transaction scripts with 1-4 msigs"})
+var msigLsigLessOrEqual16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_lsig_4_16", Description: "Total transaction scripts with 4-16 msigs"})
+var msigLsigMore16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_lsig_16", Description: "Total transaction scripts with 16+ msigs"})


nit: (personal preference...)

Suggested change

var msigLsigMore16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_lsig_16", Description: "Total transaction scripts with 16+ msigs"})

var msigLsig17orMore = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_lsig_17", Description: "Total transaction scripts with 17 or more msigs"})

algorandskiy · 2022-11-28T13:27:56Z

Removed tx group size early filtering, renamed metrics according to current state

cce · 2022-11-28T19:48:04Z

+	if ntx == config.MaxTxGroupSize {
+		transactionMessageTxGroupFull.Inc(nil)
+	} else if ntx > config.MaxTxGroupSize {
+		transactionMessageTxGroupExcessive.Inc(nil)


Could we also add a break out of the for loop above right after ntx++ (or in the for condition) if it exceeds MaxTxGroupSize?

it is here https://github.com/algorand/go-algorand/pull/4806/files#diff-e99aa9ea7548f8d632bcd018217876927f23e40a31f9328f0681e74f967d0753R394

cce · 2022-11-28T19:53:20Z

+		counter := 0
+		for _, subsigi := range s.Msig.Subsigs {
+			if (subsigi.Sig != crypto.Signature{}) {
+				counter++


Another way to count this would be to call batchVerifier.GetNumberOfEnqueuedSignatures() before and after the call to crypto.MultisigBatchPrep and calculate the difference

you could also have crypto.MultisigBatchPrep return a (count, err)

cce

looks correct, a test would be nice to prove it, and eventually we may want to build a rudimentary histogram-to-counter system that sets up buckets of value ranges (0-5, 6-10, etc) and assign them to counters so we don't have to have duplicate value-to-counter code like this start to pop up in more places

algonautshant

Looks great!

metrics: count large txgroups and msigs

cbc112c

algorandskiy added Team Carbon-11 Skip-Release-Notes labels Nov 24, 2022

algorandskiy requested review from algonautshant and cce November 24, 2022 01:24

algorandskiy self-assigned this Nov 24, 2022

algonautshant previously approved these changes Nov 24, 2022

View reviewed changes

CR fixes

3a5d5ad

algorandskiy dismissed algonautshant’s stale review via 3a5d5ad November 24, 2022 15:42

algorandskiy commented Nov 24, 2022

View reviewed changes

Comment thread data/txHandler.go Outdated

algorandskiy force-pushed the pavel/metrics-tx-type branch from cfc2fd8 to 60f5822 Compare November 24, 2022 16:54

Add a full group metric

9d0d9f2

algorandskiy force-pushed the pavel/metrics-tx-type branch from 60f5822 to 9d0d9f2 Compare November 28, 2022 13:21

meter 4-10 bucket, not 4-16

468cc32

algorandskiy requested a review from algonautshant November 28, 2022 15:01

cce reviewed Nov 28, 2022

View reviewed changes

cce approved these changes Nov 28, 2022

View reviewed changes

algonautshant approved these changes Nov 28, 2022

View reviewed changes

algorandskiy merged commit 381a835 into algorand:master Nov 28, 2022

tmc pushed a commit to tmc/go-algorand that referenced this pull request Mar 7, 2025

metrics: count large txn groups and msigs (algorand#4833)

55811d6

algorandskiy deleted the pavel/metrics-tx-type branch March 16, 2026 20:05

	var msigMore16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_16", Description: "Total transactions with 16+ msigs"})
	var msig17orMore = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_17", Description: "Total transactions with 17 or more msigs"})

	var msigLsigMore16 = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_lsig_16", Description: "Total transaction scripts with 16+ msigs"})
	var msigLsig17orMore = metrics.MakeCounter(metrics.MetricName{Name: "algod_verify_msig_lsig_17", Description: "Total transaction scripts with 17 or more msigs"})

Conversation

algorandskiy commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

codecov Bot commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

algonautshant left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

algonautshant Nov 24, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

algonautshant Nov 24, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

algorandskiy commented Nov 28, 2022

Uh oh!

cce Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

algorandskiy Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

cce Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

cce Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

cce left a comment

Choose a reason for hiding this comment

Uh oh!

algonautshant left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

algorandskiy commented Nov 24, 2022 •

edited

Loading

codecov Bot commented Nov 24, 2022 •

edited

Loading

cce Nov 28, 2022 •

edited

Loading