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

Switch Plugins to use Tracing crate Logging and Forward Plugin Logging to Hipcheck core stderr/stdout #968

Merged
merged 1 commit into from
Mar 13, 2025

Conversation

KirilldogU
Copy link
Contributor

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:

  • Creating a feature in hipcheck_sdk to initialize a tracing_subscriber
  • Calling the init_logger() feature from each plugin
  • When plugins are launched as a child process: pipe their output to a tokio task in manager.rs that deserializes the json logs, parses them, and re-emits them to Hipcheck core output.

It also adds docs in the sdk on how to use the init_logger() feature.

@KirilldogU KirilldogU requested a review from j-lanson February 28, 2025 22:26
@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch from d9bd176 to e4ecc93 Compare March 2, 2025 23:47
@KirilldogU KirilldogU marked this pull request as draft March 3, 2025 18:30
@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch 7 times, most recently from 7a4e4f1 to ebcd05d Compare March 4, 2025 20:43
@KirilldogU KirilldogU self-assigned this Mar 4, 2025
@KirilldogU KirilldogU marked this pull request as ready for review March 4, 2025 20:44
@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch 3 times, most recently from 86db15f to 51dffa6 Compare March 6, 2025 22:08
@KirilldogU KirilldogU marked this pull request as draft March 6, 2025 22:34
@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch 3 times, most recently from 534c726 to 9dec34f Compare March 7, 2025 23:58
@KirilldogU KirilldogU marked this pull request as ready for review March 8, 2025 00:02
@j-lanson
Copy link
Collaborator

@KirilldogU please resolve the merge conflicts. probably a rebase on origin/main, and unless you have a good reason, defer to the existing change on origin/main over your own since its mostly Cargo.toml files

@alilleybrinker alilleybrinker self-requested a review March 10, 2025 15:25
@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch 4 times, most recently from b881b2e to 771d056 Compare March 12, 2025 16:38
@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch from 771d056 to f7e212f Compare March 12, 2025 17:43
@KirilldogU KirilldogU requested a review from j-lanson March 12, 2025 17:49
@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch 4 times, most recently from 8403fd2 to c31859a Compare March 12, 2025 19:28
j-lanson
j-lanson previously approved these changes Mar 12, 2025
Copy link
Collaborator

@j-lanson j-lanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, good work Kirill!

@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch 2 times, most recently from 9e9d9d1 to af8a218 Compare March 12, 2025 20:53
… to hc-core output

Signed-off-by: Kirill Usubyan <kusubyan@mitre.org>
@KirilldogU KirilldogU force-pushed the kusubyan/tracing-logging-upgrade branch from af8a218 to 26e2f48 Compare March 13, 2025 15:40
@KirilldogU KirilldogU dismissed alilleybrinker’s stale review March 13, 2025 15:43

The requested change is implemented. The input is processed with a case/match, and if necessary the error is logged.

@cstepanian cstepanian self-requested a review March 13, 2025 15:46
Copy link
Contributor

@cstepanian cstepanian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me! Thanks Kirill for responding to all the feedback!

@KirilldogU KirilldogU added this pull request to the merge queue Mar 13, 2025
Merged via the queue into main with commit 4503bef Mar 13, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Proper logging for plugins
4 participants