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

Enhance mithril-common logging #1940

Merged
merged 5 commits into from
Sep 20, 2024
Merged

Enhance mithril-common logging #1940

merged 5 commits into from
Sep 20, 2024

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Sep 19, 2024

Content

This PR add a src tag with the component name on all components in mithril-common that have a logger. This change allow us to remove all prefixes that were previously used to tag a log source.

This have the added benefit to add the source component to logs that previously did not use prefixes.

IE a log that previously written like this:

{"msg":"⟳ Preload Cardano Transactions - Started","v":0,"name":"slog-rs","level":30}
{"msg":"⟳ Preload Cardano Transactions - Finished","v":0,"name":"slog-rs","level":30}
{"msg":"⟳ Next Preload Cardano Transactions will start in 10 s","v":0,"name":"slog-rs","level":30}
{"msg":"ChainReaderBlockStreamer polls next","v":0,"name":"slog-rs","level":20,"pid":80105}

Have their prefixes supersede by a src tag like this:

{"msg":"Started","v":0,"name":"slog-rs","level":30,"src":"CardanoTransactionsPreloader"}
{"msg":"Locking signed entity type","v":0,"name":"slog-rs","level":20,"src":"CardanoTransactionsPreloader","entity_type":"CardanoTransactions"}
{"msg":"Releasing signed entity type","v":0,"name":"slog-rs","level":20,"src":"CardanoTransactionsPreloader","entity_type":"CardanoTransactions"}
{"msg":"polls next","v":0,"name":"slog-rs","level":20,"src":"ChainReaderBlockStreamer"}

And logs that did not use prefix now have the src tag to identify the source component, ie:

{"msg":"Verifying certificate","v":0,"name":"slog-rs","level":20,"src":"MithrilCertificateVerifier","certificate_signed_entity_type":"MithrilStakeDistribution(Epoch(11))","certificate_epoch":"Epoch(11)","certificate_previous_hash":"","certificate_hash":"632455d1d184b380acfb5ba71233a82b216266bc1dcebee312039ccd2a0d9ef5"}

Pre-submit checklist

  • Branch
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Comments

Also this PR remove the dev dependency to slog_scope from mithril-common has we now have test tools to construct Logger when a test need one.

Copy link

github-actions bot commented Sep 19, 2024

Test Results

    4 files  ±0     54 suites  ±0   9m 40s ⏱️ +4s
1 295 tests +2  1 295 ✅ +2  0 💤 ±0  0 ❌ ±0 
1 506 runs  +2  1 506 ✅ +2  0 💤 ±0  0 ❌ ±0 

Results for commit 8582a8c. ± Comparison against base commit 64daa84.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

LGTM
Good job, that will help to filter logs

@Alenar Alenar merged commit c8c3136 into main Sep 20, 2024
39 of 49 checks passed
@Alenar Alenar deleted the djo/refactor_common_logging branch September 20, 2024 12:50
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