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

Enhancements and fixes related to logging, metrics, and testing #6052

Merged
merged 8 commits into from
Dec 14, 2024

Conversation

jutaro
Copy link
Contributor

@jutaro jutaro commented Dec 6, 2024

Summary

This pull request addresses multiple enhancements and fixes related to logging, metrics, and testing in the project.


Changes

  1. Let the Forging Mode metrics respect the command line config

    • Resolves #5751.
    • Fixes a metric to track the current forging mode on a block producer node.
    • Ensures visibility into the forging mode to improve monitoring and debugging.
  2. Enhanced Logging for Chain Events

    • Added the following fields to log messages for better traceability:
      • tipBlockHash: Hash of the current tip block.
      • tipBlockParentHash: Hash of the parent block of the tip.
      • tipBlockIssuerVerificationKeyHash: Hash of the verification key of the tip block issuer.
    • Affected log messages:
      • SwitchedToAFork
      • AddedToCurrentChain
  3. Consistency Testing for Mainnet Configuration

    • Added a test case to ensure mainnet configuration consistency.
    • Validates the correctness of the mainnet configuration to prevent misconfigurations.

Testing

  • Verified the new metric appears correctly in monitoring systems during runtime.
  • Confirmed the additional fields are included in the logs for the relevant events (SwitchedToAFork, AddedToCurrentChain).
  • Successfully ran the new mainnet configuration test case.

Checklist

  • Corrected metric for current forging mode.
  • Updated log messages with additional fields.
  • Implemented and verified the mainnet configuration consistency test.
  • Updated relevant documentation if necessary.

Linked Issue

Closes #5751

@jutaro jutaro requested a review from a team as a code owner December 6, 2024 13:09
@jutaro jutaro force-pushed the jutaro/tracer_adds branch from e85b48e to a2d1784 Compare December 6, 2024 13:11
@jutaro jutaro force-pushed the jutaro/tracer_adds branch 3 times, most recently from a4be095 to 973d9cd Compare December 11, 2024 13:50
@TerminadaDrep
Copy link

Would it be difficult to also enable logging of the reason for "SwitchedToAFork"? IE: Longer chain vs Equal length but terminal block had lower VRF?
That would close feature request 6014

@mgmeier mgmeier force-pushed the jutaro/tracer_adds branch 4 times, most recently from b75ff9a to 4dee70f Compare December 13, 2024 11:31
@mgmeier mgmeier requested review from a team as code owners December 13, 2024 11:31
@mgmeier
Copy link
Contributor

mgmeier commented Dec 13, 2024

@TerminadaDrep I've looked at the issue, and the data traced by SwitchedToAFork would be sufficient to give an underlying cause as to why a certain chain candidate won the Praos ordering over the previous one.
However, determining that cause given the old and new chain selection data needs to be implemented, and is outside the scope of this PR. We'll address it in a future one.

@jutaro jutaro force-pushed the jutaro/tracer_adds branch from 7c7c2f4 to afaf32e Compare December 13, 2024 14:59
Copy link
Contributor

@mgmeier mgmeier left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @jutaro

@mgmeier mgmeier added this pull request to the merge queue Dec 14, 2024
Merged via the queue into master with commit e746469 Dec 14, 2024
24 checks passed
@mgmeier mgmeier deleted the jutaro/tracer_adds branch December 14, 2024 16:39
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.

Missing metric for current forging mode on a blockproducer node
3 participants