Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve tx type count observability in pool #11286

Closed
mattsse opened this issue Sep 27, 2024 · 1 comment · Fixed by #11403
Closed

Improve tx type count observability in pool #11286

mattsse opened this issue Sep 27, 2024 · 1 comment · Fixed by #11403
Assignees
Labels
A-observability Related to tracing, metrics, logs and other observability tools A-tx-pool Related to the transaction mempool C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started

Comments

@mattsse
Copy link
Collaborator

mattsse commented Sep 27, 2024

Describe the feature

currently we're only tracking txs by pool, which means that these metrics only track non pending blob txs:

/// Number of transactions in the blob sub-pool
pub(crate) blob_pool_transactions: Gauge,
/// Total amount of memory used by the transactions in the blob sub-pool in bytes
pub(crate) blob_pool_size_bytes: Gauge,

/// A set of validated blob transactions in the pool that are __not pending__.

hence these metrics aren't useful to monitor how many blob transactions are currently in the entire txpool.

we would like this for all transaction types

TODO

  • introduce new metrics for legacy,eip2930,1559,4844 count

we can either try tracking this on a per tx operation basis or by checking the total list by doing a fold over all transactions in the pool, which is more expensive.

/// _All_ transaction in the pool sorted by their sender and nonce pair.
txs: BTreeMap<TransactionId, PoolInternalTransaction<T>>,

tracking all tx ops is more complex, so imo we're okay with updating these metrics when we update the pool for now this should be accurate enough:

self.metrics.performed_state_updates.increment(1);

Additional context

No response

@mattsse mattsse added C-enhancement New feature or request S-needs-triage This issue needs to be labelled labels Sep 27, 2024
@mattsse mattsse added D-good-first-issue Nice and easy! A great choice to get started A-observability Related to tracing, metrics, logs and other observability tools A-tx-pool Related to the transaction mempool and removed S-needs-triage This issue needs to be labelled labels Sep 27, 2024
@caglaryucekaya
Copy link
Contributor

I'd like to work on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-observability Related to tracing, metrics, logs and other observability tools A-tx-pool Related to the transaction mempool C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants