proxy-wasm / proxy-wasm-cpp-sdk

WebAssembly for Proxies (C++ SDK)
Apache License 2.0
139 stars 67 forks source link

Example's root_id #83

Open esnible opened 3 years ago

esnible commented 3 years ago

The file README, line 47 registers the example with no root_id parameter. I assume this picks up the default argument, "", but I couldn't get the example working without a root ID. (I was using Istio EnvoyFilter to configure, perhaps mishandling an empty root_id is an Istio flaw, but it was confusing not to see a root ID in the example.

Consider running the spell checker on the docs (e.g. "from the version of Enovy you are going to deploy")

bianpengyuan commented 3 years ago

Empty root id should also be valid. I think it is something else that makes the example not working.. Most likely tool chain. I have a pending https://github.com/proxy-wasm/proxy-wasm-cpp-sdk/pull/71 to update both tool chain and the example to use bazel. Have not yet got time to finish that.

Meanwhile, you can reference this guide: https://github.com/istio-ecosystem/wasm-extensions/blob/master/doc/write-a-wasm-extension-with-cpp.md, which will be blog posted with 1.9.

esnible commented 3 years ago

I'm not trying to get into an argument with you. I have retested it, and can confirm.

Using myproject.cc with static RegisterContextFactory register_ExampleContext(CONTEXT_FACTORY(ExampleContext), "my_root_id"); works; using it with static RegisterContextFactory register_ExampleContext(CONTEXT_FACTORY(ExampleContext)); doesn't.

When using "my_root_id" my Istio EnvoyFilter contained

          value:
            config:
              root_id: "my_root_id"

when using no string parameter to register_ExampleContext() my EnvoyFilter contained

          value:
            config:
              root_id: ""

The logs only appear when I use a non-empty root id. I will try to troubleshoot this to see if it is a problem with Istio, Envoy, or this SDK.

bianpengyuan commented 3 years ago

@esnible could you skip adding root_id? Not sure if that makes a difference.

esnible commented 3 years ago

Further investigation. If I use root_id: "my_root_id" vs root_id: "" in my EnvoyFilter Pilot generates six nearly identical type.googleapis.com/envoy.extensions.filters.http.wasm.v3.Wasm and they are ACK'd and make it to Envoy's config_dump.

Istio and Envoy seem to have a disconnect between empty and null root_id. I created https://github.com/istio/istio/issues/30473 to discuss. XDS generates a "" root ID, and Envoy ignores it.

In both cases Envoy's logs show

2021-01-28T16:28:03.570764Z trace   envoy wasm  [host->vm] proxy_on_context_create(2, 0)
2021-01-28T16:28:03.570848Z trace   envoy wasm  [vm->host] env.proxy_get_property(2544, 14, 5247632, 5247628)
2021-01-28T16:28:03.571720Z trace   envoy wasm  [vm<-host] env.proxy_get_property return: 0
2021-01-28T16:28:03.571758Z trace   envoy wasm  [host<-vm] proxy_on_context_create return: void

with only the context ID varying.

I will do a further experiment trying to put a null value into the root_id.

esnible commented 3 years ago

Further information:

If I don't include a root_id in my EnvoyFilter it works. So you are correct, there is nothing wrong with the lack of one in your README. You can close this issue, although I would recommend putting a root_id into the example as an enhancement to avoid people falling into the same pit I am falling in.

I learned one more weird thing. I also tried root_id: ~ which SHOULD set root_id to the nil pointer rather than the empty string.

When I did that, Envoy logged some unexpected things.

2021-01-28T17:00:01.308172Z debug   envoy wasm  expr_evaluate value error: No value with name "downstream_peer" found in Activation
2021-01-28T17:00:01.308191Z trace   envoy wasm  wasm log: [extensions/stats/plugin.cc:607]::report() Failed to evaluate expression: <downstream_peer.cluster_id>

I have no clue if that is a problem with Envoy, or this library, or the config Istio generates. If that log info is troubling please follow-up.