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

Mining Parameters Metrics #6587

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

Brindrajsinh-Chauhan
Copy link
Contributor

PR description

This PR adds metrics to the Mining Parameters minGasPrice and minPriorityFee as ethereum_min_gas_price and ethereum_min_priority_fee respectively to expose runtime Values of those properties set via cli arguments or RPC methods as a gauge.

Fixed Issue(s)

#6291

Copy link

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests
  • I thought about running CI.
  • If I did not run CI, I ran as much locally as possible before pushing.

@Brindrajsinh-Chauhan Brindrajsinh-Chauhan force-pushed the gas-fee-metrics branch 5 times, most recently from f246e5a to e28757e Compare February 19, 2024 05:07
Copy link
Contributor

@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Brindrajsinh-Chauhan thanks for working on this.
Your solution works, but there should be a simpler way to achieve the same, touching very few files and avoiding keeping 2 copies of the values.
See my suggestion for a simple snippet of code as example of how to implement the simpler solution, and let me know if it is not clear

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>
Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>
Copy link
Contributor

@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some code that I think could be removed

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>
@fab-10 fab-10 enabled auto-merge (squash) February 20, 2024 15:11
@fab-10 fab-10 merged commit 1b335ad into hyperledger:main Feb 20, 2024
63 checks passed
suraneti pushed a commit to suraneti/besu that referenced this pull request Feb 28, 2024
* mining parameter metrics

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

* update changelog

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

* remove unwanted code

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

---------

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>
amsmota pushed a commit to Citi/besu that referenced this pull request Apr 16, 2024
* mining parameter metrics

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

* update changelog

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

* remove unwanted code

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

---------

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>
Signed-off-by: amsmota <antonio.mota@citi.com>
amsmota pushed a commit to Citi/besu that referenced this pull request Apr 16, 2024
* mining parameter metrics

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

* update changelog

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

* remove unwanted code

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>

---------

Signed-off-by: Brindrajsinh-Chauhan <brindrajsinh@gmail.com>
Signed-off-by: amsmota <antonio.mota@citi.com>
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.

2 participants