refactor: replace tracing-log with native tracing macros for guest log forwarding#1500
Open
cshung wants to merge 1 commit into
Open
refactor: replace tracing-log with native tracing macros for guest log forwarding#1500cshung wants to merge 1 commit into
cshung wants to merge 1 commit into
Conversation
…g forwarding Remove the tracing-log dependency from hyperlight-host and replace the format_trace/log::Record approach in outb_log with direct tracing event macros (error!, warn!, info!, debug!, trace!). This consolidates guest log emission on the tracing crate, while preserving backward compatibility for consumers using only the log crate (via tracing's built-in log feature). Closes hyperlight-dev#1028 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: cshung <3410332+cshung@users.noreply.github.com>
cshung
requested review from
andreiltd,
danbugs,
dblnz,
devigned,
jprendes,
jsturtevant,
ludfjig,
simongdavies,
squillace and
syntactically
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR migrates guest/host logging paths away from log records + tracing-log formatting and toward emitting tracing events directly, while updating tests and dependencies accordingly.
Changes:
- Emit guest log messages in
outb_logas structuredtracing::*events instead of formatting alog::Recordviatracing_log::format_trace. - Replace some
logmacro usage/imports withtracingin guest and integration tests. - Remove the
tracing-logdependency fromhyperlight_host.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/tests/rust_guests/simpleguest/src/main.rs | Adjust imports and switch one traced log site to tracing::info!. |
| src/hyperlight_host/tests/integration_test.rs | Use tracing::{error, trace} instead of log::{error, trace}. |
| src/hyperlight_host/src/sandbox/outb.rs | Rework outb_log to emit tracing events with guest metadata as fields; update tests accordingly. |
| src/hyperlight_host/Cargo.toml | Drop tracing-log dependency. |
Comment on lines
+69
to
+83
| // Emit guest log data as a tracing event with structured fields. | ||
| // | ||
| // This function is *not* considered part of `tracing`'s public API, and has no | ||
| // stability guarantees. If you use it, and it breaks or disappears entirely, | ||
| // don't say we didn't warn you. | ||
|
|
||
| let should_trace = tracing_core::dispatcher::has_been_set(); | ||
| let source_file = Some(log_data.source_file.as_str()); | ||
| let line = Some(log_data.line); | ||
| let source = Some(log_data.source.as_str()); | ||
|
|
||
| // See https://github.com/rust-lang/rust/issues/42253 for the reason this has to be done this way | ||
|
|
||
| if should_trace { | ||
| // Create a tracing event for the GuestLogData | ||
| // Ideally we would create tracing metadata based on the Guest Log Data | ||
| // but tracing derives the metadata at compile time | ||
| // see https://github.com/tokio-rs/tracing/issues/2419 | ||
| // so we leave it up to the subscriber to figure out that there are logging fields present with this data | ||
| format_trace( | ||
| &Record::builder() | ||
| .args(format_args!("{}", log_data.message)) | ||
| .level(record_level) | ||
| .target("hyperlight_guest") | ||
| .file(source_file) | ||
| .line(line) | ||
| .module_path(source) | ||
| .build(), | ||
| ) | ||
| .map_err(|e| HandleOutbError::TraceFormat(e.to_string()))?; | ||
| } else { | ||
| // Create a log record for the GuestLogData | ||
| log::logger().log( | ||
| &Record::builder() | ||
| .args(format_args!("{}", log_data.message)) | ||
| .level(record_level) | ||
| .target("hyperlight_guest") | ||
| .file(Some(&log_data.source_file)) | ||
| .line(Some(log_data.line)) | ||
| .module_path(Some(&log_data.source)) | ||
| .build(), | ||
| ); | ||
| // We match on the level at runtime because tracing macros determine their | ||
| // level at compile time. Guest file/line/module are passed as structured | ||
| // fields (rather than tracing metadata) because they originate from the | ||
| // guest, not from this call site. | ||
| // | ||
| // Consumers using a `log` logger (without a tracing subscriber) still | ||
| // receive these events thanks to the `tracing` crate's `log` feature, | ||
| // which forwards tracing events to the `log` facade when no subscriber | ||
| // is set. | ||
| let source_file = log_data.source_file.as_str(); | ||
| let line = log_data.line; | ||
| let source = log_data.source.as_str(); | ||
| let message = log_data.message.as_str(); |
Comment on lines
277
to
279
| #[test] | ||
| #[ignore] | ||
| fn test_log_outb_log() { |
ludfjig
added
the
kind/refactor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove the
tracing-logdependency from hyperlight-host and replace theformat_trace/log::Recordapproach inoutb_logwith direct tracing event macros (error!,warn!,info!,debug!,trace!).This consolidates guest log emission on the
tracingcrate, while preserving backward compatibility for consumers using only thelogcrate (via tracing's built-inlogfeature).Changes
tracing-logdependencyformat_trace/log::Recordwith nativetracingmacros matched onLogLeveluse log::{error, trace}touse tracing::{error, trace}log::info!totracing::info!(keptlogimport for backward compat test)Testing
test_log_outb_logverifies backward compatibility withlog-only consumerstest_trace_outb_logverifies tracing subscriber pathCloses #1028