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

Fixed two bugs related to metrics #562

Merged
merged 4 commits into from
Mar 21, 2023

Conversation

sergio-mena
Copy link
Contributor

@sergio-mena sergio-mena commented Mar 20, 2023

Contributes to #10

When testing in prometheus-enabled testnets, we caught two places where the metrics gathering were causing a panic.
This PR also includes adjustments to e2e ci.toml config that is able to trigger these bugs.
Finally (not important, but for the record), I have included the time window I used to extract metrics from the 200-node test.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

@sergio-mena sergio-mena added the abci Application blockchain interface label Mar 20, 2023
@sergio-mena sergio-mena requested a review from a team as a code owner March 20, 2023 21:57
@sergio-mena sergio-mena self-assigned this Mar 20, 2023
@@ -11,6 +11,7 @@ process_proposal_delay = "100ms"
check_tx_delay = "0ms"
# The most common case (e.g. Cosmos SDK-based chains).
abci_protocol = "builtin"
prometheus = true
Copy link
Contributor

Choose a reason for hiding this comment

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

So we are running prometheus enabled nodes in our CI as well? Is that going to cause too much noise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prometheus is enabled from now on, but no server set up. Although it might seem a bit silly, it would have caught the two panics I saw when I started the DO testnet.

@sergio-mena sergio-mena merged commit 1f6be5b into feature/abci++vef Mar 21, 2023
@sergio-mena sergio-mena deleted the sergio/10-fix-metric-related-panics branch March 21, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abci Application blockchain interface
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants