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

Add more prometheus metric for block exec, commit and tx recheck #46

Closed
Tracked by #1
0Tech opened this issue Nov 1, 2023 · 3 comments
Closed
Tracked by #1

Add more prometheus metric for block exec, commit and tx recheck #46

0Tech opened this issue Nov 1, 2023 · 3 comments
Assignees

Comments

@0Tech
Copy link

0Tech commented Nov 1, 2023

The subject of the triage:

@tkxkd0159
Copy link
Member

tkxkd0159 commented Nov 7, 2023

cometbft still doesn't log following metrics

  • BlockExecutionTime : gauge value for measuring execBlockOnProxyApp<BeginBlock, DeliverTx, EndBlock> processing time => cometbft log this as histogram value
  • BlockCommitTime : gauge value for measuring blockExec.Commit processing time
  • RecheckTime : gauge value for measuring mempool.Recheck processing titme
    This is not core feature. So it depends on personal preference. We need to suggest it to upstream if we really need to aggregate this metrics.

@tkxkd0159 tkxkd0159 changed the title Triage on ostracon#132 feat: metric for block exec, commit and tx recheck Nov 7, 2023
@tkxkd0159 tkxkd0159 changed the title feat: metric for block exec, commit and tx recheck Add more prometheus metric for block exec, commit and tx recheck Nov 7, 2023
@0Tech
Copy link
Author

0Tech commented Nov 15, 2023

We should consult with the clients about the usage.

@tkxkd0159
Copy link
Member

Need to follow up at #53

tkxkd0159 pushed a commit that referenced this issue Apr 24, 2024
This PR superceeds cometbft#79 with some adjustments to the segments of code
timed as well as bucket sizes. The majority of the code was done by
William in cometbft#79. I tried to fine tune the measurements to exclude proto
marshalling/unmarshalling where I think it made sense.

Closes #46 

Blocked on benchmarking to confirm it is measuring what we want.
(follow up) The metrics gave us nice and realistic measurements in our
benchmarks.

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments

---------

Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants