fix: use prettyPrintIndices for logging #8298
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @maishivamhoo123, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request improves the consistency and readability of log outputs by standardizing how subnet indices are presented. The change ensures that all relevant log fields use a dedicated utility function for formatting, making debugging and monitoring more efficient.
Highlights
- Logging Consistency: Standardized the logging of mySampleSubnets by replacing sampleSubnets.join(" ") with prettyPrintIndices(sampleSubnets) to ensure uniform and readable output across log fields.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request updates the logging of mySampleSubnets to use the prettyPrintIndices helper function. This is a good change as it improves the readability of the log output by grouping sequential subnet indices, and it also makes the logging format consistent with other fields like dataColumns and custodyGroups in the same log statement. The change is correct and well-scoped. No issues found.
|
that was the only log you found where this is applicable? |
|
@nflaig I noticed that join isn’t only used in one place — it’s also present in other parts of the code, like neededColumns.join(" ") and a few others. The problem is, I’m not able to reproduce the error right now, so I’m unsure whether those spots also need fixing. If you could guide me on how to reproduce the error, I’d be able to verify it and handle the other cases more confidently. |
the best way to tackle this issue is by running a node on the devnet, then observe debug logs and find logs that need to be updated you can do that via this command relevant configs can be downloaded here https://fusaka-devnet-3.ethpandaops.io/#consensus-layer-clients and here https://config.fusaka-devnet-3.ethpandaops.io/cl/bootstrap_nodes.txt |
wemeetagain
changed the title
| blockInputs: blockInputs.map((bInpt) => `${bInpt.block.message.slot}=${bInpt.type}`).join(" "), | ||
| slots: prettyPrintIndices(blockInputs.map((b) => Number(b.block.message.slot))), | ||
| types: blockInputs.map((b) => b.type).join(" "), |
There was a problem hiding this comment.
not sure it's good to split this up, makes it harder to see which type belongs to which slot
Sep-03 17:36:55.419[sync] debug: matched BlockWithDataColumns peerClient=Lighthouse, blockInputs=303008=availableData 303010=availableData 303011=availableData 303012=availableData 303013=availableData 303015=availableData 303016=availableData 303018=availableData 303019=availableData 303020=availableData 303021=availableData 303023=availableData 303024=availableData 303025=availableData 303026=availableData 303027=availableData 303028=availableData 303030=availableData 303032=availableData
Sep-03 17:36:56.160[sync] debug: matched BlockWithDataColumns peerClient=Grandine, blockInputs=302944=dataPromise 302945=dataPromise 302946=dataPromise 302948=dataPromise 302950=dataPromise 302951=dataPromise 302952=dataPromise 302955=dataPromise 302956=dataPromise 302958=dataPromise 302959=dataPromise 302960=dataPromise 302961=dataPromise 302962=dataPromise 302963=dataPromise 302964=dataPromise 302965=dataPromise 302966=dataPromise 302967=dataPromise 302968=availableData 302969=dataPromise 302973=availableData
Sep-03 17:36:56.354[sync] debug: matched BlockWithDataColumns peerClient=Grandine, blockInputs=302976=dataPromise 302977=dataPromise 302979=dataPromise 302980=dataPromise 302982=dataPromise 302984=dataPromise 302985=dataPromise 302986=dataPromise 302988=dataPromise 302993=dataPromise 302996=dataPromise 303000=dataPromise 303001=dataPromise 303002=dataPromise 303003=dataPromise 303004=dataPromise 303005=dataPromise 303006=dataPromise 303007=dataPromise
Sep-03 17:36:58.685[sync] debug: matched BlockWithDataColumns peerClient=Lighthouse, blockInputs=302976=availableData 302977=availableData 302979=availableData 302980=availableData 302982=availableData 302984=availableData 302985=availableData 302986=availableData 302988=availableData 302993=availableData 302996=availableData 303000=availableData 303001=availableData 303002=availableData 303003=availableData 303004=availableData 303005=availableData 303006=availableData 303007=availableData
Sep-03 17:36:59.974[sync] debug: matched BlockWithDataColumns peerClient=Lighthouse, blockInputs=302944=availableData 302945=availableData 302946=availableData 302948=availableData 302950=availableData 302951=availableData 302952=availableData 302955=availableData 302956=availableData 302958=availableData 302959=availableData 302960=availableData 302961=availableData 302962=availableData 302963=availableData 302964=availableData 302965=availableData 302966=availableData 302967=availableData 302968=availableData 302969=availableData 302973=availableData
in general this log is not great though and hard to read already
There was a problem hiding this comment.
also going away with the block input refactor
There was a problem hiding this comment.
should probably avoid editing this line to avoid conflicts
There was a problem hiding this comment.
the whole file is deleted in the refactor, should be fine either way
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## unstable #8298 +/- ##
============================================
- Coverage 54.25% 54.11% -0.14%
============================================
Files 847 849 +2
Lines 63899 64045 +146
Branches 4848 4853 +5
============================================
- Hits 34668 34660 -8
- Misses 29154 29308 +154
Partials 77 77 🚀 New features to boost your workflow:
|
|
🎉 This PR is included in v1.34.0 🎉 |
3 participants
Motivation
We want to use
prettyPrintIndicesconsistently for logging subnet indices.Description
Replaced
sampleSubnets.join(" ")withprettyPrintIndices(sampleSubnets)to improve readability and consistency with other log fields.
Closes #8292
Steps to test or reproduce
git checkout maishivamhoo123/prettyPrintIndices-fresh npm run test