feat(trace): host-controlled tracing #92
Conversation
the TRACE level no longer hangs the board!
✅ Deploy Preview for merry-scone-cc7a60 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
In a subsequent branch, I intend to also add a host -> target heartbeat, so that the target can detect when |
| max_level: LevelFilter, | ||
| max_level: AtomicU8, | ||
|
|
||
| /// Tracks whether we are inside of the collector's `send_event` method, so |
|
|
||
| // we probably won't use 256 whole bytes of cobs yet since all the host | ||
| // -> target messages are quite small | ||
| let mut cobs_buf: CobsAccumulator<16> = CobsAccumulator::new(); |
There was a problem hiding this comment.
We're gunna want some kind of config soon.
| return tracing_core_02::Interest::never(); | ||
| } | ||
|
|
||
| // Due to the fact that the collector uses `bbq` internally, we must |
There was a problem hiding this comment.
This feels a little icky, because this is a platform-specific impl detail - melpo doesn't use bbqueue for the tracing collection.
Might be worth instead to::
- Provide bbqueue methods that don't emit traces (and use them in lichee-rv)
- Start an "integration guide" that tells people to use something that doesn't emit traces when implementing tracing
I don't think this is a today blocker, but if we merge this as-is it might be good to throw up an issue to fix this someday.
There was a problem hiding this comment.
Well, this collector does use bbqueue --- so tif we are ever using the collector in this file, we are using bbqueue.
| } | ||
| } | ||
|
|
||
| const fn u8_to_level(level: u8) -> LevelFilter { |
There was a problem hiding this comment.
It might be good to have an invalid level be an error (and ignore the "change log level command" instead), otherwise if we receive a garbage message that HAPPENS to look like a command, we have a 251/256 chance of just shutting off all logging. Tho I guess we don't really need to log at all to a connection that is sending us garbage. Hm.
There was a problem hiding this comment.
these functions are used internally for converting the AtomicU8 to/from a LevelFilter, not for parsing the log level sent to us from the host. that gets deserialized using postcard, so if we get an invalid value, we will error.
| }; | ||
|
|
||
| #[derive(serde::Serialize, serde::Deserialize)] | ||
| pub enum TraceEvent<'a> { | ||
| /// Sent by the target periodically when not actively tracing, to indicate |
There was a problem hiding this comment.
We might want to mark this non-exhaustive and add things to the end instead, to avoid breaking wire format, once we care about that.
There was a problem hiding this comment.
yeah, i figured we didn't currently care about stability for the crate API or for the wire format, but we can do that in the future.
jamesmunns
added
area: tools & build
Currently, the
tracingcollector in the Allwinner D1 platform impl ishardcoded to the DEBUG level. This is unfortunate, as it means that we
will always do the work of collecting and formatting DEBUG traces, even
when no one is listening on the UART tracing port. It would be more
efficient if we only formatted traces when a
crowttyinstance hassubscribed to the UART tracing port. Additionally, because trace
metadata is sent when the callsite is initially registered, a
crowttyinstance that attaches after the board has started running may miss some
trace metadata. Finally, there is no way to change the trace level on
the fly, as it's hard-coded in the kernel.
This branch changes the
mnemos-trace-protowire protocol to include ahost to target message to select a
tracinglevel. Now, the board canstart up with all tracing disabled, and
crowttycan send the desiredtracing level once it connects. This way, we don't collect or format
traces at all when
crowttyisn't connected. In order to ensurecrowttywaits to send the tracing level message only once the board'stracing code has been initialized, we add a periodic "heartbeat" message
sent by the board on the tracing port while tracing is idle. This is
used by the host which is listening for traces to detect the presence of
the tracing collector on the debug target. The same message is used to
ack a successful request to select the tracing level.
When the tracing level is selected, the target rebuilds
tracing'scallsite cache, which means all metadata for callsites which have
previously been hit will be sent to the host. This ensures that
crowttyalways has the metadata for all tracing spans and events thatare enabled, even if the target encountered them before
crowttyconnected.
Making this work required fixing an upstream
tracingbug,tokio-rs/tracing#2634, where calling into code
that includes
tracingdiagnostics from inside aCollect::register_callsitemethod would cause a deadlock.Additionally, I had to add code to the collector for tracking whether
it's inside its own
sendmethod, in order to temporarily disablebbqtracing. This is because the collector's use of
bbqcreates aninfinite loop when the TRACE level is enabled.
Check this out:
