tracee
tracee copied to clipboard
ebpf: changed to use log package
Initial Checklist
- [x] There is an issue describing the need for this PR.
- [x] Git log contains summary of the change.
- [x] Git log contains motivation and context of the change.
- [ ] If part of an EPIC, PR git log contains EPIC number.
- [ ] If part of an EPIC, PR was added to EPIC description.
Description (git log)
Change prints and error handling to use logging logic instead. This way tracee-epbf can support multiple logging levels without moving context between packages. This is an alternative to #1788, using a known package for logging.
Fixes: #1787
Type of change
- [ ] Bug fix (non-breaking change fixing an issue, preferable).
- [ ] Quick fix (minor non-breaking change requiring no issue, use with care)
- [ ] Code refactor (code improvement and/or code removal)
- [x] New feature (non-breaking change adding functionality).
- [ ] Breaking change (cause existing functionality not to work as expected).
Final Checklist:
Pick "Bug Fix" or "Feature", delete the other and mark appropriate checks.
- [ ] I have made corresponding changes to the documentation.
- [x] My code follows the style guidelines (C and Go) of this project.
- [x] I have performed a self-review of my own code.
- [x] I have commented all functions/methods created explaining what they do.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [x] My changes generate no new warnings.
- [ ] I have added tests that prove my fix, or feature, is effective.
- [x] New and existing unit tests pass locally with my changes.
- [x] Any dependent changes have been merged and published before.
Git Log Checklist:
My commits logs have:
- [x] Subject starts with "subsystem|file: description".
- [x] Do not end the subject line with a period.
- [x] Limit the subject line to 50 characters.
- [x] Separate subject from body with a blank line.
- [x] Use the imperative mood in the subject line.
- [x] Wrap the body at 72 characters.
- [x] Use the body to explain what and why instead of how.
@NDStrahilevitz @rafaeldtinoco WDYT?
I didn't review the code, but to refer to parts of the comment by @NDStrahilevitz, I think that:
- Logger should be in its own package (which can simlpy be named
logger
), where it can wrap the logrus package - A logger instnace can be initialized by main, and set for every package used by it (today we have the main tracee struct in the ebpf package, but this will change in the future)
- Logger can be kept in the tracee instance, like Nadav suggested (
t.logger
)
I didn't review the code, but to refer to parts of the comment by @NDStrahilevitz, I think that:
- Logger should be in its own package (which can simlpy be named
logger
), where it can wrap the logrus package- A logger instnace can be initialized by main, and set for every package used by it (today we have the main tracee struct in the ebpf package, but this will change in the future)
- Logger can be kept in the tracee instance, like Nadav suggested (
t.logger
)
I agree to all this PLUS, let's make sure we configure logger from the cmdline output options as well (out-file, err-file options).
I wonder if we might want to implement the Warning
, Error
, Fatal
and Panic
with error
argument instead of string
.
It will allow tests to use the errors.As
and errors.Is
methods to check the workflows.
WDYT?
I wonder if we might want to implement the
Warning
,Error
,Fatal
andPanic
witherror
argument instead ofstring
. It will allow tests to use theerrors.As
anderrors.Is
methods to check the workflows. WDYT?
That would probably work out fine. You mean integration and e2e tests right? That could work. If it's not too much work you can demonstrate that in this PR.
Regardless using error
as an argument for those methods sounds reasonable.
I believe this is currently being worked by @geyslan now, for 0.9.0, right @AlonZivony ? IF that is so, mind if we close this one ?
Not at all