feat: add OTel stats to mempool and ntx state#1073
Conversation
Mirko-von-Leipzig
force-pushed
the
mirko/otels
branch
from
d6324bd to
33f6f18
Compare
Mirko-von-Leipzig
force-pushed
the
mirko/otels
branch
from
5855cd1 to
d984758
Compare
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a couple of small comments inline.
| /// The amount of transactions which are root nodes. | ||
| pub fn num_roots(&self) -> usize { | ||
| self.inner.num_roots() | ||
| } |
There was a problem hiding this comment.
Does the "root node" here mean transactions that have no parents? (i.e., they don't depend on other transactions). If so, I would add this to the comment.
| span.set_attribute("mempool.transactions.total", self.transactions.len()); | ||
| span.set_attribute("mempool.transactions.roots", self.transactions.num_roots()); | ||
| span.set_attribute("mempool.accounts", self.state.num_accounts()); | ||
| span.set_attribute("mempool.nullifiers", self.state.num_nullifiers()); | ||
| span.set_attribute("mempool.output_notes", self.state.num_notes_created()); | ||
| span.set_attribute("mempool.batches.pending", self.batches.num_pending()); | ||
| span.set_attribute("mempool.batches.proven", self.batches.num_proven()); | ||
| span.set_attribute("mempool.batches.total", self.batches.len()); | ||
| span.set_attribute("mempool.batches.roots", self.batches.num_roots()); |
There was a problem hiding this comment.
nit (feel free to ignore): it is not immediately obvious what mempool.transactions.roots and mempool.batches.roots means. Maybe there is a better name for them?
sergerad
left a comment
sergerad
left a comment
There was a problem hiding this comment.
LGTM TY.
Btw I couldn't seem to find any events containing target field in devnet yesterday which was surprising. The other fields seemed to come through just fine.
|
I'll merge as is. We can address the comments in subsequent PRs (if needed). |
4 participants
This PR adds open-telemetry stats to the mempool and the ntb state.
These likely aren't perfectly labeled, but should let us sneak a peak while things are running at least.