Switch Plugins to use Tracing crate Logging and Forward Plugin Logging to Hipcheck core stderr/stdout #968
Conversation
KirilldogU
force-pushed
the
kusubyan/tracing-logging-upgrade
branch
from
d9bd176 to
e4ecc93
Compare
KirilldogU
force-pushed
the
kusubyan/tracing-logging-upgrade
branch
7 times, most recently
from
7a4e4f1 to
ebcd05d
Compare
KirilldogU
force-pushed
the
kusubyan/tracing-logging-upgrade
branch
3 times, most recently
from
86db15f to
51dffa6
Compare
KirilldogU
force-pushed
the
kusubyan/tracing-logging-upgrade
branch
3 times, most recently
from
534c726 to
9dec34f
Compare
|
@KirilldogU please resolve the merge conflicts. probably a rebase on |
| @@ -31,7 +31,12 @@ pub struct PluginServer<P> { | |||
|
|
|||
| impl<P: Plugin> PluginServer<P> { | |||
| /// Create a new plugin server for the provided plugin. | |||
| pub fn register(plugin: P) -> PluginServer<P> { | |||
| pub fn register(plugin: P, log_level: &str) -> PluginServer<P> { | |||
There was a problem hiding this comment.
log_level should be an Enum, and probably should be Option<LogLevel> which is equal to LogLevel::Error when set to None. Similarly, should be this way in the Args {} struct
There was a problem hiding this comment.
You can do log_level: impl Into<Option<LogLevel>> so that people can pass the raw LogLevel or None depending.
There was a problem hiding this comment.
You may want to move LogLevel declaration into the hipcheck-common crate so both the SDK and the core can use it
hipcheck/src/plugin/manager.rs
Outdated
| for setting in settings { | ||
| let parts: Vec<&str> = setting.split('=').collect(); | ||
|
|
||
| if parts.len() == 1 { |
There was a problem hiding this comment.
Would you please comment this block and the parts.len() == 2 block with an explanation of what these cases mean, with at least one example for each? I think that would help a lot, since a reader of this code isn't necessarily familiar with the syntax this code is trying to parse.
There was a problem hiding this comment.
I think a match construction here would also be clearer: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=501b8472e9c6899e95ad773bca5dd687
hipcheck/src/plugin/manager.rs
Outdated
| for setting in settings { | ||
| let parts: Vec<&str> = setting.split('=').collect(); | ||
|
|
||
| if parts.len() == 1 { |
There was a problem hiding this comment.
I think a match construction here would also be clearer: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=501b8472e9c6899e95ad773bca5dd687
| let reader = BufReader::new(reader); | ||
| let mut lines = reader.lines(); | ||
| // parse plugin log json lines to LogEvent objects and output the messages | ||
| while let Some(line) = lines.next_line().await.unwrap() { |
There was a problem hiding this comment.
Add a safety comment for why this unwrap would never be hit, or replace it with error reporting.
KirilldogU
force-pushed
the
kusubyan/tracing-logging-upgrade
branch
2 times, most recently
from
937cec1 to
6e51dbe
Compare
… to hc-core output Signed-off-by: Kirill Usubyan <kusubyan@mitre.org>
KirilldogU
force-pushed
the
kusubyan/tracing-logging-upgrade
branch
from
6e51dbe to
2ac238c
Compare
Resolves #381 and #933.
This PR switches the Hipcheck plugins to use the tracing crate for logging instead of the logging crate (as will be done on Hipcheck core).
It also forwards the logging output of plugins to the stderr/stdout output of Hipcheck core.
It does this through:
It also adds docs in the sdk on how to use the init_logger() feature.