[CORE-5092] Transform SDK: Schema Registry client in C++ #21292

oleiman · 2024-07-08T16:22:18Z

Primarily in service of extending Schema Registry support to the JavaScript SDK, this commit implements the full SR ABI v0 in the C++ SDK.

TODO (exclusive of mundane cleanup):

Type documentation
improve error code forwarding across SDK API boundary
organization of namespces could use a second look
schema_registry_client::new_client() could be a free function
~~Finish building out examples/schema_registry.cc~~ punting on this as the javascript version will test a superset of functionality.
- ~~Avro dependency for deser (ingress)~~
- ~~JSON dependency for ser (egress)~~
- ~~Enable SR integration test~~

Backports Required

Release Notes

Improvements

Add schema registry support to experimental Data Transforms C++ SDK

oleiman · 2024-07-15T23:33:08Z

force push: minor QoL tweak to simple_named_type. Hopefully we don't wind up reimplementing the whole thing 😅

dotnwat · 2024-07-16T04:44:38Z

src/transform-sdk/cpp/include/redpanda/transform_sdk.h

+    schema_version version;
+
+private:
+    friend bool operator==(const reference&, const reference&) = default;


is this to disable comparison?

Nah, this should generate a default overload for the reference struct.

Per the standard, if it's not a member it must be a friend. For some "reason" i mildly prefer the friend formulation to the non-static member 🤷‍♂️

Oh, the privateness? doesn't matter since it's not a member. Private friend feels natural to me for whatever reason

dotnwat · 2024-07-16T04:46:41Z

src/transform-sdk/cpp/include/redpanda/transform_sdk.h

+    [[nodiscard]] const sr::schema& schema() const { return _schema; }
+    [[nodiscard]] const std::string& subject() const { return _subject; }
+    [[nodiscard]] schema_version version() const { return _version; }
+    [[nodiscard]] schema_id id() const { return _id; }


nice. +1 nodiscard

All credit to surrounding code. But yeah, feels good

michael-redpanda

very cool

src/transform-sdk/cpp/src/transform_sdk.cc

michael-redpanda · 2024-07-16T17:15:37Z

src/transform-sdk/cpp/src/transform_sdk.cc

+    be_id.resize(sizeof(schema_id));
+    std::memcpy(be_id.data(), sid.data(), sizeof(schema_id));
+    bytes result = MAGIC_BYTES;
+    result.append_range(be_id | std::views::reverse);


There's also htonl (https://linux.die.net/man/3/htonl) but this is fine

We already require C++23 because of std::expected, so let's use byteswap: https://en.cppreference.com/w/cpp/numeric/byteswap

avoided hton because I think the swap is conditional on host endianness, which shouldn't be an issue in practice but feels like a mild (if extremely common) abuse of the API. might be wrong about that though.

byteswap

killer. wasn't aware

Well if the device is big endian and we receive data in network byte order (which is big endian) hton* is a no-op.

yeah that's kinda my point - the endianness is well-defined on both ends (Wasm vs SR header), so what I want is a byteswap, unconditionally. no functional difference in this case though.

src/transform-sdk/cpp/include/redpanda/transform_sdk.h

rockwotj · 2024-07-16T17:46:21Z

src/transform-sdk/cpp/src/transform_sdk.cc

+    be_id.resize(sizeof(schema_id));
+    std::memcpy(be_id.data(), sid.data(), sizeof(schema_id));
+    bytes result = MAGIC_BYTES;
+    result.append_range(be_id | std::views::reverse);


We already require C++23 because of std::expected, so let's use byteswap: https://en.cppreference.com/w/cpp/numeric/byteswap

rockwotj · 2024-07-16T17:48:12Z

src/transform-sdk/cpp/src/transform_sdk.cc

+    const uint32_t raw_id = (static_cast<uint32_t>(id_bytes[3]) << 0U)
+                            | (static_cast<uint32_t>(id_bytes[2]) << 8U)
+                            | (static_cast<uint32_t>(id_bytes[1]) << 16U)
+                            | (static_cast<uint32_t>(id_bytes[0]) << 24U);


Torn on casting to uint32 and using byteswap... this is probably fine :)

I always get tripped on removing UB in these things: https://justine.lol/endian.html

ya, I think as long as there's no intermediate promotion to signed anything, we're all good. if i can avoid typing memcpy in these situations I'm basically happy.

My only reservation here is that this assumes we're always running on a little endian device.

Wasm is little endian.

See list here: https://webassembly.org/docs/portability/

Straightforward approximation of named_type, intended for the schema registry client interface and providing access to the value of and a non-const pointer to the underlying value.

oleiman · 2024-07-17T18:14:54Z

force push CR comments, various and sundry:

supporting classes can be plain structs
some minor modernizations (views, c++23 features, constants)

oleiman · 2024-07-17T18:16:29Z

and fix SR example

rockwotj

LGTM

src/transform-sdk/cpp/include/redpanda/transform_sdk.h

- enum class schema_format - schema_id (named int32) - schema_version (named int32) - struct reference{name, subject, version} - class schema{raw, format, reference[]} - class subject_schema{schema, subject, version, ID}

With simple tests

We don't have an easy way to support avro serde from c++/wasm, so this isn't suitable for the existing integration test in its current form. However, it's useful for manual verification and as a reference. Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>

oleiman · 2024-07-17T18:38:51Z

force push s/references_c/reference_container/

oleiman self-assigned this Jul 8, 2024

github-actions bot added the area/wasm WASM Data Transforms label Jul 8, 2024

oleiman changed the title ~~Transform SDK: Schema Registry client in C++~~ [CORE-5092] Transform SDK: Schema Registry client in C++ Jul 8, 2024

oleiman force-pushed the xform/sdk/core-5092/sr-c++ branch 8 times, most recently from 0310bde to e78412b Compare July 15, 2024 22:18

oleiman marked this pull request as ready for review July 15, 2024 22:26

oleiman requested review from a team, michael-redpanda and rockwotj and removed request for a team July 15, 2024 22:26

oleiman force-pushed the xform/sdk/core-5092/sr-c++ branch from e78412b to 482fef6 Compare July 15, 2024 23:31

dotnwat reviewed Jul 16, 2024

View reviewed changes

michael-redpanda previously approved these changes Jul 16, 2024

View reviewed changes

rockwotj reviewed Jul 16, 2024

View reviewed changes

xform/sdk/cpp: Introduce simple_named_type

d4c1f8a

Straightforward approximation of named_type, intended for the schema registry client interface and providing access to the value of and a non-const pointer to the underlying value.

oleiman dismissed michael-redpanda’s stale review via 7ec02d3 July 17, 2024 18:13

oleiman force-pushed the xform/sdk/core-5092/sr-c++ branch from 482fef6 to 7ec02d3 Compare July 17, 2024 18:13

oleiman force-pushed the xform/sdk/core-5092/sr-c++ branch from 7ec02d3 to b3e6799 Compare July 17, 2024 18:16

rockwotj previously approved these changes Jul 17, 2024

View reviewed changes

src/transform-sdk/cpp/include/redpanda/transform_sdk.h Outdated Show resolved Hide resolved

oleiman added 2 commits July 17, 2024 11:36

xform/sdk/cpp: SR supporting types

b5fc4d9

- enum class schema_format - schema_id (named int32) - schema_version (named int32) - struct reference{name, subject, version} - class schema{raw, format, reference[]} - class subject_schema{schema, subject, version, ID}

xform/sdk/cpp: Add functions for schema ID codec

0ad8d92

With simple tests

oleiman added 5 commits July 17, 2024 11:36

xform/sdk/cpp: SR ABI (wasm imports and stubs)

540dadc

xform/sdk/cpp: Helpers for reading/writing schemas

03c89c0

xform/sdk/cpp: Custom error code for SR abi boundary

8a2dccb

xform/sdk/cpp: SR client

9fd9baa

oleiman dismissed rockwotj’s stale review via 1d94ebf July 17, 2024 18:38

oleiman force-pushed the xform/sdk/core-5092/sr-c++ branch from b3e6799 to 1d94ebf Compare July 17, 2024 18:38

rockwotj approved these changes Jul 17, 2024

View reviewed changes

michael-redpanda self-requested a review July 17, 2024 18:43

michael-redpanda approved these changes Jul 17, 2024

View reviewed changes

oleiman merged commit 6032904 into redpanda-data:dev Jul 17, 2024
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CORE-5092] Transform SDK: Schema Registry client in C++ #21292

[CORE-5092] Transform SDK: Schema Registry client in C++ #21292

oleiman commented Jul 8, 2024 •

edited

Loading

oleiman commented Jul 15, 2024

dotnwat Jul 16, 2024

oleiman Jul 16, 2024

oleiman Jul 16, 2024

dotnwat Jul 16, 2024

oleiman Jul 16, 2024

michael-redpanda left a comment

michael-redpanda Jul 16, 2024

rockwotj Jul 16, 2024

oleiman Jul 16, 2024 •

edited

Loading

michael-redpanda Jul 17, 2024

oleiman Jul 17, 2024

rockwotj Jul 16, 2024

rockwotj Jul 16, 2024

oleiman Jul 16, 2024

michael-redpanda Jul 17, 2024

rockwotj Jul 17, 2024

oleiman commented Jul 17, 2024

oleiman commented Jul 17, 2024

rockwotj left a comment

oleiman commented Jul 17, 2024

[CORE-5092] Transform SDK: Schema Registry client in C++ #21292

[CORE-5092] Transform SDK: Schema Registry client in C++ #21292

Conversation

oleiman commented Jul 8, 2024 • edited Loading

Backports Required

Release Notes

Improvements

oleiman commented Jul 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael-redpanda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleiman Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleiman commented Jul 17, 2024

oleiman commented Jul 17, 2024

rockwotj left a comment

Choose a reason for hiding this comment

oleiman commented Jul 17, 2024

oleiman commented Jul 8, 2024 •

edited

Loading

oleiman Jul 16, 2024 •

edited

Loading