You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Depends on #2399.
While working on #2399, I noticed that there was an emerging boilerplate
pattern for tasks which report ereports, where I had declared a struct
that bundles together a buffer of a size calculated using
`microcbor::max_cbor_len_for!` with the `Packrat` API client and some
methods for reporting ereports. For example, in the Cosmo sequencer, I
wrote this:
https://github.com/oxidecomputer/hubris/blob/9e18d0bd9cb69f76534ab0539f9114aeb398ac8c/drv/cosmo-seq-server/src/main.rs#L1237-L1263
And then an identical copy of the same code in the Gimlet sequencer:
https://github.com/oxidecomputer/hubris/blob/9e18d0bd9cb69f76534ab0539f9114aeb398ac8c/drv/gimlet-seq-server/src/main.rs#L1620-L1647
I started to wonder whether it would be possible to factor out this
boilerplate. While we could easily just have a
```rust
pub struct Ereporter<const BUF_LEN: usize> {
// ...
}
```
and some methods in the `ereports` crate, and let the user provide the
buffer size from a separate invocation of the `microcbor` macro, it
occurred to me that there was a way to ... feed ... two birds with
one...loaf of bread?[^1] and also solve some of the long running issues
with the present ereport-recording APIs.
In particular, a long-standing thorn in my side is the fact that, while
we presently have a way to _calculate_ the required buffer length for a
list of ereports, we **don't** currently have anything that actually
**stops** you from attempting to to actually use that buffer to encode
an ereport that **wasn't** involved in the length calculation. I had
hoped that there would be a generic way to do this using const generics,
but unfortunately, Rust's const generics remain only sort of half
finished, as I discussed in detail in
#2246 (comment).
So, this means that it's possible to add a new ereport type, forget to
add it to the list of ereports used for the length calculation, and end
up with an ereport that never fits in the buffer without really
noticing. So that's not great.
Another less important but similarly bothersome thing is that I have
generally tried to add ringbuf entries to record when an ereport is
submitted, and more importantly, when it _isn't_, either because the
ereport was too big for the encoding buffer due to issues like the one I
described above, or due to the `packrat` ereport aggregation buffer
being full. Ideally, these ringbuf entries/counters would also record
_which_ ereport class was or was not reported, but doing this requires
even *more* boilerplate which must be specific to the individual task
that records ereports, as the list of ereports depends on the task.
So, I've come up with a way to feed all of the aforementioned birds, by
introducing a new `declare_ereporter!` macro in the `ereports` crate.
This macro is invoked with the name of a struct, the name of a _trait_,
and a list of ereports, and generates an ereporter struct, a trait
implemented by all of the ereports in the provided list, and a method on
the ereporter struct to record an ereport *where the ereport type
implements the generated trait*. This way, we can have a way to ensure
that only ereports whose lengths were used in the encoding buffer size
calculation are reported using that buffer. The macro also generates
ringbuf entries/counters for each ereport that's recorded using the
generated ereporter type. For more details on how the new thing is used,
see [the RustDoc I added for the macro][1].
[1]:
https://github.com/oxidecomputer/hubris/blob/d4444b38458ae4d6fb72df7cec8fb38519b7f040/lib/ereports/src/lib.rs#L14-L104
[^1]: Attempting to use non-violent language here... :)
0 commit comments