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

Add best block indicator to informant message + print parent block on import message #4021

Merged
merged 5 commits into from
Apr 8, 2024
Merged

Add best block indicator to informant message + print parent block on import message #4021

merged 5 commits into from
Apr 8, 2024

Conversation

skunert
Copy link
Contributor

@skunert skunert commented Apr 8, 2024

Sometimes you need to debug some issues just by the logs and reconstruct what happened.
In these scenarios it would be nice to know if a block was imported as best block, and what it parent was.
So here I propose to change the output of the informant to this:

2024-04-05 20:38:22.004  INFO ⋮substrate: [Parachain] ✨ Imported #18 (0xe7b3…4555 -> 0xbd6f…ced7)    
2024-04-05 20:38:24.005  INFO ⋮substrate: [Parachain] ✨ Imported #19 (0xbd6f…ced7 -> 0x4dd0…d81f)    
2024-04-05 20:38:24.011  INFO ⋮substrate: [jobless-children-5352] 🌟 Imported #42 (0xed2e…27fc -> 0x718f…f30e)    
2024-04-05 20:38:26.005  INFO ⋮substrate: [Parachain] ✨ Imported #20 (0x4dd0…d81f -> 0x6e85…e2b8)    
2024-04-05 20:38:28.004  INFO ⋮substrate: [Parachain] 🌟 Imported #21 (0x6e85…e2b8 -> 0xad53…2a97)    
2024-04-05 20:38:30.004  INFO ⋮substrate: [Parachain] 🌟 Imported #22 (0xad53…2a97 -> 0xa874…890f)

@skunert skunert added the T0-node This PR/Issue is related to the topic “node”. label Apr 8, 2024
@skunert skunert requested a review from a team April 8, 2024 07:28
koute
koute approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@koute koute left a comment

Choose a reason for hiding this comment

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

Hmm.... it'd be nice to maybe pick more distinct pair of emojis?

I don't know, maybe 🆕 for imported and 🏆 for best? Or anything else that isn't "yellow star-like thing" in both cases. (:

davxy
davxy approved these changes Apr 8, 2024
View reviewed changes
Copy link
Member

@davxy davxy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As we're already outside of ASCII charset, what about using → instead of ->?

I.e.
(0xad53…2a97 → 0xa874…890f)
instead of
(0xad53…2a97 -> 0xa874…890f)

@bkchr
Copy link
Member

bkchr commented Apr 8, 2024

I don't know, maybe 🆕 for imported and 🏆 for best? Or anything else that isn't "yellow star-like thing" in both cases. (:

Looks good :D

bkchr
bkchr approved these changes Apr 8, 2024
View reviewed changes
substrate/client/informant/src/lib.rs Outdated Show resolved Hide resolved
@skunert
Copy link
Contributor Author

skunert commented Apr 8, 2024

Also switched to the arrow as davide suggested.

@skunert skunert enabled auto-merge April 8, 2024 13:07
@skunert skunert added this pull request to the merge queue Apr 8, 2024
Merged via the queue into paritytech:master with commit fdb1dba Apr 8, 2024
107 of 135 checks passed
@skunert skunert deleted the skunert/informant branch April 8, 2024 13:58
Ank4n pushed a commit that referenced this pull request Apr 9, 2024
@skunert @bkchr @Ank4n
Add best block indicator to informant message + print parent block on…
4db2dca 
… import message (#4021)

Sometimes you need to debug some issues just by the logs and reconstruct
what happened.
In these scenarios it would be nice to know if a block was imported as
best block, and what it parent was.
So here I propose to change the output of the informant to this:

```
2024-04-05 20:38:22.004  INFO ⋮substrate: [Parachain] ✨ Imported #18 (0xe7b3…4555 -> 0xbd6f…ced7)    
2024-04-05 20:38:24.005  INFO ⋮substrate: [Parachain] ✨ Imported #19 (0xbd6f…ced7 -> 0x4dd0…d81f)    
2024-04-05 20:38:24.011  INFO ⋮substrate: [jobless-children-5352] 🌟 Imported #42 (0xed2e…27fc -> 0x718f…f30e)    
2024-04-05 20:38:26.005  INFO ⋮substrate: [Parachain] ✨ Imported #20 (0x4dd0…d81f -> 0x6e85…e2b8)    
2024-04-05 20:38:28.004  INFO ⋮substrate: [Parachain] 🌟 Imported #21 (0x6e85…e2b8 -> 0xad53…2a97)    
2024-04-05 20:38:30.004  INFO ⋮substrate: [Parachain] 🌟 Imported #22 (0xad53…2a97 -> 0xa874…890f)    
```

---------

Co-authored-by: Bastian Köcher <[email protected]>
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Apr 9, 2024
@skunert @bkchr @dharjeezy
Add best block indicator to informant message + print parent block on…
e51eb22 
… import message (paritytech#4021)

Sometimes you need to debug some issues just by the logs and reconstruct
what happened.
In these scenarios it would be nice to know if a block was imported as
best block, and what it parent was.
So here I propose to change the output of the informant to this:

```
2024-04-05 20:38:22.004  INFO ⋮substrate: [Parachain] ✨ Imported paritytech#18 (0xe7b3…4555 -> 0xbd6f…ced7)    
2024-04-05 20:38:24.005  INFO ⋮substrate: [Parachain] ✨ Imported paritytech#19 (0xbd6f…ced7 -> 0x4dd0…d81f)    
2024-04-05 20:38:24.011  INFO ⋮substrate: [jobless-children-5352] 🌟 Imported paritytech#42 (0xed2e…27fc -> 0x718f…f30e)    
2024-04-05 20:38:26.005  INFO ⋮substrate: [Parachain] ✨ Imported paritytech#20 (0x4dd0…d81f -> 0x6e85…e2b8)    
2024-04-05 20:38:28.004  INFO ⋮substrate: [Parachain] 🌟 Imported paritytech#21 (0x6e85…e2b8 -> 0xad53…2a97)    
2024-04-05 20:38:30.004  INFO ⋮substrate: [Parachain] 🌟 Imported paritytech#22 (0xad53…2a97 -> 0xa874…890f)    
```

---------

Co-authored-by: Bastian Köcher <[email protected]>
This was referenced Jun 5, 2024
Closed
Closed
Closed
Closed
@ToufeeqP ToufeeqP mentioned this pull request Dec 10, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lexnv lexnv lexnv approved these changes

@sandreim sandreim sandreim approved these changes

@davxy davxy davxy approved these changes

@bkchr bkchr bkchr approved these changes

@michalkucharczyk michalkucharczyk michalkucharczyk approved these changes

@koute koute koute approved these changes

Assignees
No one assigned
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
SDK Node
Status: done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@skunert @bkchr @koute @michalkucharczyk @davxy @sandreim @lexnv