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

Soroban metrics improvement #4175

Merged
merged 7 commits into from
Feb 1, 2024
Merged

Soroban metrics improvement #4175

merged 7 commits into from
Feb 1, 2024

Conversation

jayz22
Copy link
Contributor

@jayz22 jayz22 commented Jan 30, 2024

Description

Resolves all the items in this checklist: https://github.com/stellar/stellar-core-internal/issues/236#issuecomment-1915049419

Most of them are just some naming and types change. There is one structural change:

A new SorobanLedgerMetrics was added in LedgerManager which tracks soroban resource usage per ledger across all soroban ops (invoke_host_fn, restore_footprint, extend_ttl), and publishes them to new metrics "soroban.ledger.xxx".

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

@jayz22 jayz22 force-pushed the metrics branch 2 times, most recently from 70f4bad to 1b71127 Compare January 30, 2024 17:16
@jayz22
Copy link
Contributor Author

jayz22 commented Jan 31, 2024

@MonsieurNicolas thanks for the review, and sorry about the confusion. Most of the confusion comes from "read" == both read and modified, and "write" == only modified, and "entry" == LedgerEntry which could be {ContractDataEntry, ContractCodeEntry}, and we also have "data" and "code" which are just those two variants.
I've cleaned up the wording of the entire section, hope that makes it clear now.

@graydon
Copy link
Contributor

graydon commented Feb 1, 2024

r+ fed81d0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants