Add a minimal msgpack serialiser for ccf - #7866
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a minimal, header-only MessagePack encoder under src/msgpack/ (including fluentd in_forward EventTime ext type support) to enable more efficient log export as part of the broader observability/log-export work (#7858).
Changes:
- Added a C++20 header-only msgpack encoder (
write_*primitives, container headers, andFluentdEventTime). - Added unit tests (boundary tables + property tests) and differential tests using
nlohmann::json::from_msgpackas an oracle. - Added a libFuzzer harness and integrated the new unit/fuzz tests into the build.
Custom instructions used:
.github/copilot-instructions.md.github/instructions/reviewing.instructions.md
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/msgpack/encode.h |
New header-only msgpack encoder API, error model, and Fluentd EventTime support |
src/msgpack/endian.h |
Big-endian writing helper for msgpack wire format |
src/msgpack/test/encode_test.cpp |
Property/boundary tests for smallest-format-wins encoding and EventTime layout |
src/msgpack/test/differential_test.cpp |
Differential encode-vs-nlohmann decode tests + fluentd message-mode pinned vector |
src/msgpack/test/gen.h |
Shared generator + script driver used by tests and fuzz harness |
src/msgpack/test/fuzz_script_test.cpp |
Deterministic/canned script tests pinning specific composite byte layouts |
src/msgpack/test/format_introspect.h |
Test helper to classify msgpack first-byte format families |
src/msgpack/test/msgpack_fuzz.cpp |
libFuzzer harness for encoder round-trip validation via nlohmann oracle |
CMakeLists.txt |
Registers msgpack_test unit test and msgpack_fuzz_test fuzz target |
| } | ||
| else | ||
| { | ||
| // std::byteswap is C++23-only; this hand-rolled swap keeps the |
There was a problem hiding this comment.
are we able to switch to C++23 on Azure Linux 3 already?
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38e26ecc-d8a3-452a-8f63-de338955dffb
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38e26ecc-d8a3-452a-8f63-de338955dffb
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38e26ecc-d8a3-452a-8f63-de338955dffb
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38e26ecc-d8a3-452a-8f63-de338955dffb
| } | ||
| } | ||
|
|
||
| // ===== FluentdEventTime ===== |
There was a problem hiding this comment.
I'd prefer to have the FluentdEventTime type defined here, next to the only function that actually needs it? And probably-maybe in a separate header, so it's clearly an opt-in for the places that want it rather than a core part of the generic msgpack API.
For #7858 we want to export more logs more efficiently.
I proposed that we should support msgpack export, and this is a minimal msgpack serialiser.
Here is a benchmark of the serialiser against nlohmann (json dump) and rapidjson (as an 'optimal baseline')