wasm: API to add tags to active span from wasm filter #384
Conversation
Signed-off-by: Mohit Agarwal <[email protected]>
mohit-a21
requested review from
jplevyak,
lizan and
PiotrSikora
as code owners
Signed-off-by: Mohit Agarwal <[email protected]>
|
/retest |
Signed-off-by: Mohit Agarwal <[email protected]>
|
I have run the |
|
@mohit-a21 thanks for your contribution! ASan is a bit flaky on the CircleCI lately, I kicked off another run. Having said that, we're doing some cleanup/reorg across the repos, so I'll hold off merging you contribution until those are completed (should be sometime next week). |
…tracing_api Signed-off-by: Mohit Agarwal <[email protected]>
Signed-off-by: Mohit Agarwal <[email protected]>
|
@PiotrSikora Thanks a lot for looking into this. |
|
@PiotrSikora nice work, how's going about this pr? looking forward envoy with rust |
Signed-off-by: Mohit Agarwal <[email protected]>
Signed-off-by: Mohit Agarwal <[email protected]>
Signed-off-by: Mohit Agarwal <[email protected]>
|
@PiotrSikora is the repo open for contribution? Thanks! |
|
@mohit-a21 I think so, I need to confirm one more thing. I'll get back to you tomorrow. |
api/wasm/cpp/proxy_wasm_externs.h
Outdated
| @@ -164,6 +164,9 @@ extern "C" void proxy_on_grpc_trailing_metadata(uint32_t context_id, uint32_t to | |||
| extern "C" void proxy_on_grpc_receive(uint32_t context_id, uint32_t token, uint32_t response_size); | |||
| extern "C" void proxy_on_grpc_close(uint32_t context_id, uint32_t token, uint32_t status_code); | |||
|
|
|||
| extern "C" WasmResult proxy_active_span_set_tag(const char* key_ptr, size_t key_size, const char* value_ptr, | |||
There was a problem hiding this comment.
At the ABI level, this should be called proxy_set_active_span_tag().
Also, you should move it into host exports section above.
Signed-off-by: Mohit Agarwal <[email protected]>
Signed-off-by: Mohit Agarwal <[email protected]>
Signed-off-by: Mohit Agarwal <[email protected]>
Signed-off-by: Mohit Agarwal <[email protected]>
@PiotrSikora Any updates on this? Thanks! |
Signed-off-by: Mohit Agarwal <[email protected]>
|
@PiotrSikora Hey, Are you guys accepting patches now? Thanks! |
|
@mohit-a21 apologies for the delay, could you open a PR against https://github.com/proxy-wasm/spec with the ABI change? Thanks! |
Thanks, @PiotrSikora. Will do. |
|
@PiotrSikora Hey I have created a PR(proxy-wasm/spec#5) in https://github.com/proxy-wasm/spec . Please review. Thanks! |
| if (!decoder_callbacks_) { | ||
| return; | ||
| } | ||
| Tracing::Span& span = decoder_callbacks_->activeSpan(); |
There was a problem hiding this comment.
Have you considered a version of this function that uses encoder_callbacks_, maybe as a backup? I ask because I don't know if decoder_callbacks_ is valid in the context of the HTTP encoder. Do you have.an example of this working within the onResponseBody() of a Context subclass?
Signed-off-by: Mohit Agarwal [email protected]
Description: This change adds an API,
activeSpanSetTagwhich allows a filter to customize span info and help in adding tracing info.Risk Level: Low
Testing: As part of the change, test cases have been added along the lines of test cases for existing APIs. Along with that manual testing has been done to verify the API.
Docs Changes: Docs have been updated regarding the API so added.
Fixes: #383