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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Metrics and DEBG log for late gossip blocks #2533

Closed
wants to merge 6 commits into from
Closed

[Merged by Bors] - Metrics and DEBG log for late gossip blocks #2533

wants to merge 6 commits into from

Conversation

paulhauner
Copy link
Member

@paulhauner paulhauner commented Aug 21, 2021 •
edited

Issue Addressed

Which issue # does this PR address?

Proposed Changes

  • Add a counter metric to log when a block is received late from gossip.
  • Also push a DEBG log for the above condition.
  • Use Debug (?) instead of Display (%) for a bunch of logs in the beacon processor, so we don't have to deal with concatenated block roots.
  • Add new ERRO and CRIT to HTTP API to alert users when they're publishing late blocks.

Additional Info

NA

@paulhauner paulhauner added work-in-progress PR is a work-in-progress v1.5.0 For inclusion in v1.5.0 release labels Aug 21, 2021
@paulhauner paulhauner mentioned this pull request Aug 21, 2021
Closed
@paulhauner paulhauner added v1.5.1 To be included in the v1.5.1 relase and removed v1.5.0 For inclusion in v1.5.0 release labels Aug 22, 2021
michaelsproul
michaelsproul requested changes Aug 23, 2021
View reviewed changes
Copy link
Member

@michaelsproul michaelsproul left a comment •
edited

Choose a reason for hiding this comment

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

It seems that past Paul already thought of and implemented both these changes 😁

beacon_node/http_api/src/lib.rs Outdated Show resolved Hide resolved
beacon_node/network/src/beacon_processor/worker/gossip_methods.rs Show resolved Hide resolved
@paulhauner
Copy link
Member Author

Thanks for the review! I added a5fe3d4, to achieve:

  • Remove duplicate logs in HTTP API.
  • Change the existing HTTP API WARN to ERRO when block is >=2s late.
  • Use block_delay key in related logs to make it easier for me to grep related things.
  • Remove "msg" field from late gossip block (I had this when I was going to make it a WARN, seems unnecessary now).

@paulhauner paulhauner added v1.5.0 For inclusion in v1.5.0 release and removed v1.5.1 To be included in the v1.5.1 relase labels Aug 23, 2021
michaelsproul
michaelsproul approved these changes Aug 23, 2021
View reviewed changes
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

LGTM!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed work-in-progress PR is a work-in-progress labels Aug 23, 2021
@michaelsproul michaelsproul marked this pull request as ready for review August 23, 2021 00:56
@paulhauner
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Aug 23, 2021
@paulhauner
Metrics and DEBG log for late gossip blocks (#2533)
f2a8c62 
## Issue Addressed

Which issue # does this PR address?

## Proposed Changes

- Add a counter metric to log when a block is received late from gossip.
- Also push a `DEBG` log for the above condition.
- Use Debug (`?`) instead of Display (`%`) for a bunch of logs in the beacon processor, so we don't have to deal with concatenated block roots.
- Add new ERRO and CRIT to HTTP API to alert users when they're publishing late blocks.

## Additional Info

NA
@bors
Copy link

bors bot commented Aug 23, 2021

Pull request successfully merged into unstable.

Build succeeded:

@bors bors bot changed the title Metrics and DEBG log for late gossip blocks [Merged by Bors] - Metrics and DEBG log for late gossip blocks Aug 23, 2021
@bors bors bot closed this Aug 23, 2021
pawanjay176 pushed a commit to pawanjay176/lighthouse that referenced this pull request Aug 27, 2021
@paulhauner @pawanjay176
Metrics and DEBG log for late gossip blocks (sigp#2533)
ce20412 
## Issue Addressed

Which issue # does this PR address?

## Proposed Changes

- Add a counter metric to log when a block is received late from gossip.
- Also push a `DEBG` log for the above condition.
- Use Debug (`?`) instead of Display (`%`) for a bunch of logs in the beacon processor, so we don't have to deal with concatenated block roots.
- Add new ERRO and CRIT to HTTP API to alert users when they're publishing late blocks.

## Additional Info

NA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelsproul michaelsproul michaelsproul approved these changes

Assignees
No one assigned
Labels
ready-for-merge This PR is ready to merge. v1.5.0 For inclusion in v1.5.0 release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@paulhauner @michaelsproul