golang extension: implement GetAllHeaders in the cluster_specifier plugin. #39094
Conversation
|
Hi @oconnorkyle, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
|
/retest |
|
ping @doujiang24 |
|
pinging @doujiang24 again. |
| @@ -25,6 +25,16 @@ absl::string_view referGoString(void* str) { | |||
| extern "C" { | |||
| #endif | |||
|
|
|||
| // Returns the number of headers in the request. | |||
| uint64_t envoyGoClusterSpecifierGetNumHeaders(unsigned long long header_ptr) { | |||
There was a problem hiding this comment.
we can combine envoyGoClusterSpecifierGetNumHeaders and envoyGoClusterSpecifierGetHeadersByteSize into a single API, so that we can reduce one Go->C call.
There was a problem hiding this comment.
Done, they are now combined into a single function call.
|
@oconnorkyle Thanks a lot and sorry for the late, just a small comment. |
oconnorkyle
requested review from
RyanTheOptimist,
abeyad,
fredyw,
mattklein123,
mathetake,
botengyao,
yanavlasov,
nezdolik,
ravenblackx,
tyxia,
ggreenway,
wbpcode and
kyessenov
as code owners
repokitteh-read-only
bot
added
api
deps
|
CC @envoyproxy/coverage-shephards: FYI only for changes made to |
oconnorkyle
force-pushed
the
cluster-specifier-all-headers
branch
from
8331b9d to
090b9e2
Compare
|
Sorry for the review spam, I messed up merging main into this branch and it assigned you all. I'm not sure if I'm able to un-assign reviews. |
|
/unassign RyanTheOptimist |
|
oconnorkyle is not allowed to unassign users. |
1 similar comment
|
oconnorkyle is not allowed to unassign users. |
oconnorkyle
force-pushed
the
cluster-specifier-all-headers
branch
from
1e0bab6 to
090b9e2
Compare
Signed-off-by: Kyle O'Connor <oconnorkyle@google.com>
…erSpecifierGetHeadersByteSize` functions into a single API.` Signed-off-by: Kyle O'Connor <oconnorkyle@google.com>
oconnorkyle
force-pushed
the
cluster-specifier-all-headers
branch
from
090b9e2 to
67bf879
Compare
|
/retest |
ravenblackx
removed request for
fredyw,
abeyad,
ravenblackx,
mattklein123,
yanavlasov,
ggreenway,
kyessenov,
wbpcode,
mathetake,
nezdolik,
RyanTheOptimist,
botengyao and
tyxia
|
/retest |
|
Thank you @oconnorkyle ! |
Commit Message:
golang extension: implement GetAllHeaders in the cluster_specifier plugin.
Fixes: #38644
Additional Description:
Risk Level: Low
Testing: Integration testing - I would prefer to add some unit testing as well for the Go -> Cgo code, but I didn't see existing examples of this so I wasn't sure if it's common practice.
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A