Expand C++ SDK documentation #158
Conversation
mpwarres
commented
Aug 1, 2023
mpwarres
commented
- Add docs/api_overview.md file
- Add doc comments to proxy_wasm_api.h and other header files
- Simplify top-level README.md and move build instructions into docs/building.md file
A subsequent commit will add a short top-level README.md that references the build instructions as well as API documentation. Signed-off-by: Michael Warres <mpw@google.com>
Signed-off-by: Michael Warres <mpw@google.com>
Signed-off-by: Michael Warres <mpw@google.com>
Signed-off-by: Michael Warres <mpw@google.com>
Signed-off-by: Michael Warres <mpw@google.com>
| Abseil (optionally) is built in /root/abseil and can be used. Note that the | ||
| Abseil containers (e.g. `absl::flat_hash_set`) exercise many syscalls which are | ||
| not supported. Consequentially individual files should be pulled in which are | ||
| relatively self contained (e.g. `strings`). Example customized Makefile: |
There was a problem hiding this comment.
[I realize this is just a moved doc]
Picking apart Abseil is kind of a crappy experience. AIUI Abseil support for standalone Wasm is improving, though still in progress. We should revisit this soon.
| ```c++ | ||
| static RegisterContextFactory register_ExampleContext( | ||
| CONTEXT_FACTORY(ExampleContext), ROOT_FACTORY(ExampleRootContext), | ||
| "my_root_id"); |
There was a problem hiding this comment.
We don't explain the purpose of the root ID in these docs, and it's not required unless you're making multiple roots. Should we elide this from the example?
| * `onCreate`: called when handling of a new stream starts. | ||
| * `onDone`: called when the host is done processing the stream. | ||
| * `onLog`: called after the host is done processing the stream, if the plugin is | ||
| being used for access logging. |
There was a problem hiding this comment.
I'm not sure what this means. How does a plugin sign up for access logging? Clarify that this is not related to logging described below?
| * `onUpstreamData`: called when a new chunk of data is received from upstream | ||
| over a connection. | ||
|
|
||
| For API details, see doc comments for the `Context` class in [proxy_wasm_api.h]. |
There was a problem hiding this comment.
You might say something here about return values and immediate responses.
| * `onDelete`: called after the plugin has completed all processing related to | ||
| the stream, as indication to release resources. | ||
|
|
||
| `Context` instances also receive callbacks corresponding to stream events: |
There was a problem hiding this comment.
I'd split callbacks for HTTP and TCP into separate sections, otherwise it reads as both are called for the same stream.
| `Context::onUpstreamData` method. | ||
|
|
||
| To access the actual data being proxied, plugin code would use the | ||
| buffer-related hostcalls described in [Buffers], specifying |
There was a problem hiding this comment.
The link to [Buffers] is missing.
| * `LOG_ERROR` | ||
| * `LOG_CRITICAL` | ||
|
|
||
| Additionally, there is a LOG macro that accepts log level as a |
There was a problem hiding this comment.
I know the LOG macro is exported, but I believe it was defined as a helper function for LOG_* macros, so perhaps it would be best if it's not included in the docs.
Also, it's conflicting with LOG macro in Abseil(?).
| cp protobuf-wasm/src/.libs/libprotobuf.a ${CPP_API}/libprotobuf.a | ||
| ``` | ||
|
|
||
| ### WAVM binaries |
There was a problem hiding this comment.
WAVM isn't the default engine (and should probably be removed at this point), so installing WAVM binaries as part of SDK (to precompile .wasm binary) is pointless and this should be removed.
| @@ -148,6 +168,7 @@ template <typename Pairs> size_t pairsSize(const Pairs &result) { | |||
| return size; | |||
| } | |||
|
|
|||
| // Marshals `result` to the memory buffer `buffer`. | |||
There was a problem hiding this comment.
This should say something about the type that's marshalled.
Also, I'd rename result to pairs, since the former doesn't make much sense.
