add humility counters, show counts in humility ringbuf
#449
Conversation
This file contains 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
2 tasks
hawkw
added a commit
to oxidecomputer/hubris
that referenced
this pull request
Feb 26, 2024
Ringbufs provide a fixed-size buffer for storing diagnostic events, which may include data. This is very useful for debugging. However, because the ring buffer has a fixed size, only the last `N` events are visible at any given time. If a ringbuf has a capacity of, say, 16 events, and a given event has occurred 10,000 times over the lifespan of the task, there's currently no way to know that the event has happened more than 16 times. Furthermore, there's no way to use the ringbuf to determine whether a given event has _ever_ occurred during the task's lifetime: if a given variant is not _currently_ present in the ringbuf, it may still have occurred previously and been "drowned out" by subsequent events. Finally, some targets disable the ringbuf entirely, for space reasons, and on those targets, it may still be desirable to be able to get some less granular diagnostic data out of the ringbuf events a task records. Therefore, this commit extends the `ringbuf` crate to support generating counts of the number of times a particular event variant has been recorded, in addition to a fixed-size buffer of events. These counters provide less detailed data than inspecting the ringbuf contents, because they don't record the values of any _fields_ on that event. However, they allow recording very large numbers of events in a fixed amount of space, and provide historical data for events that may have occurred in the past and then "fallen off" the end of the ringbuf as new events were recorded. By inspecting these counters, we can determine if a given variant has *ever* been recorded, even if it isn't currently in the buffer, and we can see the total number of times it has been recorded over the task's entire lifetime. Event counters are implemented using a new `CountedRingbuf` type, which marries a ring buffer with a type which can count occurances of entry variants. `CountedRingbuf`s may be declared using the `counted_ringbuf!` macro, and entries can be recorded using the existing `ringbuf_entry!` macro. `CountedRingbuf` requires that the entry type implement a new `Count` trait, which defines the counter type, an initializer for creating new instances of the counter type, and a method to increment the counter type with a given instance of the entry type. An implementation of `ringbuf::Count` can be generated for an entry type using the `#[derive(ringbuf::Count)]` attribute, which generates a `struct` with an `AtomicU32` field for each of the deriving `enum`'s variants. This `struct` can then be loaded by Humility to provide a view of the event counters. Because recording event counters requires only `4 * <N_VARIANTS>` bytes, counters are currently recorded even when the `ringbuf` crate has the `disabled` feature set. This way, we can still record some diagnostic data on targets that don't have the space to store the ringbuf itself. If it becomes necessary to also disable counters, we could add a separate `disable-counters` feature as well, so that a target can pick and choose between recording ringbuf entries, counts, or both. As a proof of concept, I've also updated `gimlet-seq-server` to use the new event counters for its `Trace` ringbuf. Subsequent commits will roll it out elsewhere. Future work will include: - [ ] Actually adopting event counters in existing ringbufs - [ ] Updating `humility ringbuf` to also dump ringbuf event counts (see oxidecomputer/humility#449). Depends on #1624, both to reduce merge conflicts and to mitigate an increase in size due to the changes in this branch.
This way, we can add a separate `humility counters` subcommand for printing counters that may not be attached to a ringbuf.
This adds a command for displaying all counters, regardless of whether or not they are associated with a ringbuf. Given that we now have this command, I've removed the `-t`/`--totals-only` flag from `humility ringbuf`, as it no longer seems necessary
hawkw
force-pushed
the
eliza/ringbuf-counts
branch
from
1bf4c9a
to
1e0ddb7
Compare
hawkw
changed the title
humility ringbufhumility counters, show counts in humility ringbuf
mkeeter
reviewed
Feb 28, 2024
mkeeter
reviewed
Feb 28, 2024
bcantrill
requested changes
Feb 29, 2024
There was a problem hiding this comment.
This looks great -- please take a dump from a Gimlet system (or another that has counters) and add it to the test suite. Also, please add a counters test to the test suite -- and then be sure to bump the version number! (And considering that this is a breaking change in that new Hubris needs new Humility to properly display some ringbufs, I think we should bump the minor and not just the patch -- taking us to 0.11.0.)
Ah, thanks — I had totally missed that we had a test suite with representative dumps, very nice! I’ll add some tests shortly. |
bcantrill
approved these changes
Feb 29, 2024
hawkw
force-pushed
the
eliza/ringbuf-counts
branch
from
7b31eb3
to
c012260
Compare
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.
Hubris PR oxidecomputer/hubris#1621 added the ability for ring buffers
whose entry type is an
enumto generate a set of counters that trackthe total number of times each entry enum variant has been added to the
ring buffer. PR oxidecomputer/hubris#1630 generalizes this to also allow
counters to be generated for any
enumwithout requiring acorresponding ring buffer.
These counters are designed to be consumed by Humility. This PR updates
Humility to understand these counters. In particular, it:
humility ringbufcommand to understandCountedRingbufsas well as un-counted
RingbufsCountedRingbuf, thehumility ringbufcommandwill now display the total counts for that ringbuf's entry variants
(unless explicitly asked not to)
humility counterssubcommand for displaying all counters,whether or not they are attached to a ringbuf.
The
humility counterscommand behaves similarly tohumility ringbuf,and allows filtering which counters are displayed based on the name of
the variable and task, and can also be used to list all counters in the
attached Hubris instance or dump. Both commands support a CLI flag to
display counters with zero values.
Examples
Listing all counters in a dump:
Displaying all counters in a dump:
Filtering by task:
Sorting counters by their value:
Sorting counters alphabetically:
Displaying all counters, including those with zero values:
Displaying variant total counts when running `humility ringbuf`:
Displaying totals with zero values when running `humility ringbuf`:
Opting out of totals when displaying a counted ringbuf: