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

Suggestion: Add CustomDeserializer trait for making debugging raft-rs logs much easier #522

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jopemachine
Copy link
Contributor

@jopemachine jopemachine commented Aug 17, 2023

As I mentioned in the raft-rs Slack channel before, I think it would be very useful if users could see their byte slice information like Entry's context, data.

I think it could be possible by exposing the CustomDeserializer trait in raft-rs.

In this PR, I implement this thinking roughly, and I can see they are working as expected through ratify which uses raft-rs under the hood.

You can see the logged Message includes context, data as deserialized strings, not as byte slices in the below screenshot.

1

I'm just wondering if there are some raft-rs maintainers who agree with this idea and if not, why not.

Although this PR is currently just some sample codes to show ideas and working code, if there's a chance this PR will be merged, I'm willing to work on making improvements.

(Otherwise, I think I have to keep the cloned separate repository for raftify which would be troublesome for me...)

@BusyJay
Copy link
Member

BusyJay commented Aug 17, 2023

Can you describe more details on what's the behavior without this patch and why it's bad?

@jopemachine
Copy link
Contributor Author

jopemachine commented Aug 17, 2023

Can you describe more details on what's the behavior without this patch and why it's bad?

I think nobody wants to see byte slice in their raft-rs logs like the below screenshot.

스크린샷 2023-08-17 오후 11 23 45

Instead, I think users should want to be able to pass their own deserialization method like the below screenshot.

스크린샷 2023-08-17 오후 11 25 51

And I think this PR could accomplish this by exposing the CustomDeserializer trait.

If users don't implement that trait, the byte slices will still be printed as byte slices. (same behavior)

But if users want to see deserialized strings in their raft-rs logs, they can make it through CustomDeserializer of this PR.

If you think more explanation is needed, please feel free to comment here or you can send slack direct message to me directly.

@jopemachine
Copy link
Contributor Author

So, the behavior without the patch is showing raw byte slice in logs, and I think it could be very bad for debugging and figuring out what's happening inside...

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.

None yet

2 participants