-
Notifications
You must be signed in to change notification settings - Fork 11
[Feat] Use standard Elixir logger #64
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
Conversation
- Implement Logger.debug/info/warn/error translation to bpf_printk - Add automatic log level prefixes ([DEBUG], [INFO], [WARN], [ERROR]) - Refactor bpf_printk logic to support Logger calls - Add comprehensive test suite for Logger functionality - Create example demonstrating Logger usage - Add documentation for Logger support This allows developers to use familiar Elixir Logger syntax instead of raw bpf_printk calls, improving code readability and organization.
- Add support for Logger.warning alongside Logger.warn - Both functions produce [WARN] prefix for compatibility - Update tests to verify both warn and warning work identically - Update documentation and examples to prefer Logger.warning - Add comprehensive compatibility documentation - Maintain backward compatibility with existing Logger.warn usage This ensures compatibility across Elixir versions while following modern deprecation practices.
- Correct documentation to show current limitations with string interpolation - Update examples to use currently supported syntax (simple strings) - Add clear status indicators (✅ supported,⚠️ planned) - Remove misleading printf-style format string examples - Add test case documenting interpolation limitation - Clarify that interpolation support is planned for future releases This ensures users have accurate expectations about current Logger capabilities while maintaining the vision for future enhancements.
|
|
||
| - `Logger.debug/1` and `Logger.debug/2` | ||
| - `Logger.info/1` and `Logger.info/2` | ||
| - `Logger.warn/1` and `Logger.warn/2` (deprecated, use `warning`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have yet to merge this into main even once, so we can remove the deprecated versions before they even get to exist! (take this opportunity now or it might be a bother later)
I tend to run Honey Potion on Arch, and often I tend to see the deprecated failures and the random issues from using non-standard applications of Elixir's tools and it happens quite frequently (Elixir has quite fast development!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is done this way to comply with the Elixir API; otherwise, the behavior would be different from what is expected in the Elixir code. The goal of this PR is to be 100% compatible with the Elixir API, even for deprecated features. When it's removed from Elixir, we'll remove it here.
|
I really like the idea! As long as we don't deprecate/remove the bpf_printk for those that want/need it I find this to be a very welcome addition. I'll be checking if it runs fine and more when I get some more free time! ps: One of the example files is in the wrong folder! (one is inside and the other outside lib, if you want to keep it out to show it's not supported make a new folder for future projects yet to be supported.) |
Good.
Interpolation is supported; the file was accidentally in the wrong folder. |
|
@Dwctor can we proceed with the merge? Have you finished the reviews? |
|
Hi @sleipnir . I'm sorry, but I don't quite have the bandwidth to check Honey Potion in my day to day currently (just moved into a new place). From what I remember the branch had no issues and should be fine! Feel free to merge it. |
|
No problem @Dwctor, I wish you well in your new place. |
Still in line with my original intention of bringing Honey Potion closer to Elixir, this PR aims to introduce Logger functionality in the style of Elixir instead of having our users use the bpf_printk helper function directly.
So now we will do this to print: