ereports: add declare_ereporter! macro

[hawkw]

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:

hubris/drv/cosmo-seq-server/src/main.rs

Lines 1237 to 1263 in 9e18d0b

	/// This is just the Packrat API handle and the ereport buffer bundled together
	/// in one thing so that it can be passed into various places as a single
	/// argument.
	pub(crate) struct Ereporter {
	packrat: task_packrat_api::Packrat,
	buf: &'static mut [u8; EREPORT_BUF_LEN],
	}
	impl Ereporter {
	pub(crate) fn try_send_ereport(
	&mut self,
	ereport: &impl microcbor::StaticCborLen,
	) {
	let eresult = self
	.packrat
	.deliver_microcbor_ereport(&ereport, &mut self.buf[..]);
	match eresult {
	Ok(len) => ringbuf_entry!(Trace::EreportSent(len)),
	Err(task_packrat_api::EreportEncodeError::Packrat { len, err }) => {
	ringbuf_entry!(Trace::EreportLost(len, err))
	}
	Err(task_packrat_api::EreportEncodeError::Encoder(_)) => {
	ringbuf_entry!(Trace::EreportTooBig)
	}
	}
	}
	}

And then an identical copy of the same code in the Gimlet sequencer:

hubris/drv/gimlet-seq-server/src/main.rs

Lines 1620 to 1647 in 9e18d0b

	/// This is just the Packrat API handle and the ereport buffer bundled together
	/// in one thing so that it can be passed into various places as a single
	/// argument.
	pub(crate) struct Ereporter {
	packrat: task_packrat_api::Packrat,
	buf: &'static mut [u8; EREPORT_BUF_LEN],
	}
	impl Ereporter {
	pub(crate) fn try_send_ereport(
	&mut self,
	ereport: &impl microcbor::StaticCborLen,
	) {
	let eresult = self
	.packrat
	.deliver_microcbor_ereport(&ereport, &mut self.buf[..]);
	match eresult {
	Ok(len) => ringbuf_entry!(Trace::EreportSent(len)),
	Err(task_packrat_api::EreportEncodeError::Packrat { len, err }) => {
	ringbuf_entry!(Trace::EreportLost(len, err))
	}
	Err(task_packrat_api::EreportEncodeError::Encoder(_)) => {
	ringbuf_entry!(Trace::EreportTooBig)
	}
	}
	}
	}

I started to wonder whether it would be possible to factor out this boilerplate. While we could easily just have a

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?¹ 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.

Footnotes

1. Attempting to use non-violent language here... :) ↩

[mkeeter]

boy I sure do love Rust macros