-
Notifications
You must be signed in to change notification settings - Fork 31
Add support for host features. #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # Host features | ||
|
|
||
| | Identifier | Name | Values | Deprecated | | ||
| |:----------:|:-------------------------|:---------------------------------------------------------------------------------------------------|:----------:| | ||
| | 0x0001 | `HAS_CORE` | When `1`, all baseline functions considered essential are supported. | No | | ||
| | 0x0002 | `HAS_LOGGING` | When `1`, all baseline functions for logging are supported. | No | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are the SDKs expected to track which host features are supported and avoid invoking their hostcalls, or is it up to plugin authors to avoid calling such functions?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They could (to avoid crossing Wasm<>Host boundary), but for any unsupported hostcalls, they'll receive The main goal is for plugins to verify support for the features they required during configuration phase, and fail to start if some are missing, instead of starting successfully and failing at runtime due to |
||
| | 0x0003 | `HAS_HTTP_HEADERS` | When `1`, only baseline functions related to HTTP headers are supported. | No | | ||
| | 0x0004 | `HAS_HTTP_WITH_BODY` | When `1`, all baseline functions for HTTP are supported. | No | | ||
|
Comment on lines
+7
to
+8
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we will still need HTTP headers without body. |
||
| | 0x0005 | `HAS_HTTP_CALLS` | When `1`, all baseline functions for HTTP calls are supported. | No | | ||
| | 0x0006 | `HAS_GRPC_CALLS` | When `1`, all baseline functions for gRPC calls are supported. | No | | ||
| | 0x0007 | `HAS_GRPC_STREAMS` | When `1`, all baseline functions for gRPC streams are supported. | No | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if it's worth it to separate streaming from unary gRPC, it's hard for me to imagine a host supporting only non-streaming gRPC.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree. Take Envoy for example, it currently supports only fully buffered (i.e. unary) HTTP callouts, so if for some reason we decided to rebuild gRPC support directly on top of HTTP callouts in SDKs, instead of relying on hosts to support gRPC, then we'd lose gRPC streams for callouts. This is a hypothetical situation, and in such case we wouldn't have those features advertised here, but I'm just trying to show that some host features are not as well supported as they should be, and for example we don't have streaming HTTP callout because they're not supported in Envoy. |
||
| | 0x0008 | `HAS_TCP_FILTER` | When `1`, only baseline functions related to new TCP connections are supported. | No | | ||
| | 0x0009 | `HAS_TCP_WITH_PAYLOAD` | When `1`, all baseline functions for TCP are supported. | No | | ||
| | 0x000A | `HAS_KEY_VALUE_STORES` | When `1`, all baseline functions for key-value stores are supported. | No | | ||
| | 0x000B | `HAS_SHARED_QUEUES` | When `1`, all baseline functions for shared queues are supported. | No | | ||
| | 0x000C | `HAS_TIMERS` | When `1`, all baseline functions for timers are supported. | No | | ||
| | 0x000D | `HAS_METRICS` | When `1`, all baseline functions for metrics are supported. | No | | ||
| | 0x000E | `HAS_PROPERTIES` | When `1`, all baseline functions for properties are supported. | No | | ||
| | 0x000F | `HAS_CUSTOM_FUNCTIONS` | When `1`, all baseline functions for custom functions are supported. | No | | ||
| | 0x0F01 | `HAS_WASI_PREVIEW1_CORE` | When `1`, all baseline functions considered essential from WASI Preview1 are supported. | No | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With any of the features here, host is advertising a working implementations. Stubs are always expected for |
||
|
|
||
|
|
||
| Identifiers below 0x2000 are reserved for standardized features and options. Random numbers above that range should be used for private extensions. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you expect that when a new feature intended for eventual standardization is added, it would start off with an ID above 0x2000 while still in experimental state, and then later switch to a standard ID?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. This is the same mechanism that's used by IETF and it works quite well during development, since backwards compatibility can be ignored when people are still working on implementations, because it's easy to switch to another random ID without any consensus. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it's best to be explicit, but just out of curiosity, is there ever a case where this bit wouldn't be set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping the value part leaves the doors open for building upon this mechanism, instead of adding another one in the future, and I think we'll use it sooner rather than later.
Some things that come to mind would be:
MAXIMUM_BUFFER_SIZE, 16384,proxy_set_host_features(HAS_CUSTOM_FUNCTIONS, 1).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another use case that we might want to use is to define
BASELINE_VERSIONwith random (or monotonic?) values while we're actively working on v0.3 implementations, before we freeze it as finalBASELINE_VERSION=3000or similar.