proxy-wasm / spec

WebAssembly for Proxies (ABI specification)
Apache License 2.0
542 stars 28 forks source link

ABI vNEXT: generic vs specialized context functions #9

Open PiotrSikora opened 4 years ago

PiotrSikora commented 4 years ago

The current version of the ABI defines generic proxy_on_context_create() and proxy_on_context_finalize() functions which are used for VM and plugin lifecycle, as well as for per-stream lifecycle, which makes "context" a bit overloaded and confusing.

For VM context:

proxy_on_context_create(VmContext, ...)
proxy_on_vm_start(...)
proxy_on_context_finalize(...)

For plugin context:

proxy_on_context_create(PluginContext, ...)
proxy_on_plugin_start(...)
proxy_on_context_finalize(...)

For HTTP per-stream context:

proxy_on_context_create(HttpContext, ...)
proxy_on_http_request_headers(...)
proxy_on_http_request_body(...)
proxy_on_http_response_headers(...)
proxy_on_http_response_body(...)
proxy_on_context_finalize(...)

Instead, we could have specialized "finalize" functions for each context type, and create context implicitly as part of each "start" function:

For VM context:

proxy_on_vm_start(...)
proxy_on_vm_shutdown(...)

For plugin context:

proxy_on_plugin_start(...)
proxy_on_plugin_shutdown(...)

For HTTP per-stream context:

proxy_on_http_request_headers(...)
proxy_on_http_request_body(...)
proxy_on_http_response_headers(...)
proxy_on_http_response_body(...)
proxy_on_http_finalize(...)

In case proxy_on_http_finalize() returns Action::Pause, the host would wait until proxy_resume() is called before calling proxy_on_http_finalize() again (which matches the behavior of other callbacks that have the ability to pause/resume processing).

@mattklein123 @htuch @yskopets @yuval-k @gbrail @jplevyak @kyessenov any thoughts?

htuch commented 4 years ago

Seems a slam dunk interface improvement.

mattklein123 commented 4 years ago

Huge +1 from me. I think this is one of the things that I initially commented on.

kyessenov commented 4 years ago

+1 as a user. Using the same callback is really problematic when one plugin serves in different roles (which might be common to save on resources by reusing the VM/module).

I think this will need pausable access log extensions in envoy.