Skip to content

Update go filter #5

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

Merged
merged 59 commits into from
Mar 25, 2025
Merged

Conversation

johnlanni
Copy link

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

antJack and others added 30 commits March 24, 2025 14:41
* golang filter: add OnLog

TODO: pass AccessLogType & allow logging in the middle
Signed-off-by: spacewander <[email protected]>

* address review

Signed-off-by: spacewander <[email protected]>
`go.sum` is useless according to:
envoyproxy#28595 (comment)
Add it to the gitignore so it won't be committed
Signed-off-by: spacewander <[email protected]>

Signed-off-by: spacewander <[email protected]>
* fix go network extension close without reason

Signed-off-by: wangkai19 <[email protected]>

* fix ut

Signed-off-by: wangkai19 <[email protected]>

---------

Signed-off-by: wangkai19 <[email protected]>
* go http extension support metric api

Signed-off-by: wangkai19 <[email protected]>

---------

Signed-off-by: wangkai19 <[email protected]>
* go http extension add record metric api

Signed-off-by: wangkai19 <[email protected]>

* fix format

Signed-off-by: wangkai19 <[email protected]>

* move metric test to a new filter

Signed-off-by: wangkai19 <[email protected]>

* fix dependabot

Signed-off-by: wangkai19 <[email protected]>

* fix review

Signed-off-by: wangkai19 <[email protected]>

---------

Signed-off-by: wangkai19 <[email protected]>
* golang filter: add GetProperty (envoyproxy#28595)

As we already support getting/setting FilterState, we don't need to implement
GetProperty with non-attribute name & SetProperty which actually work on FilterState

* remove assertion which is no longer true

The assertion is no longer true after
envoyproxy@a420821

Solve envoyproxy#28595 (comment)


Signed-off-by: spacewander <[email protected]>
…ves for C functions. (envoyproxy#29396)

* golang extension: optimization: added noescape and nocallback directives for C functions.

they could prevent the Go compiler from allocating function parameters on the heap.

Signed-off-by: doujiang24 <[email protected]>

* add comment for the magic number.

Signed-off-by: doujiang24 <[email protected]>

---------

Signed-off-by: doujiang24 <[email protected]>
envoyproxy#29533)

* golang filter: support DownstreamPeriodic & DownstreamStart access log

Signed-off-by: spacewander <[email protected]>

* pass test

Signed-off-by: spacewander <[email protected]>

* tweak

Signed-off-by: spacewander <[email protected]>

---------

Signed-off-by: spacewander <[email protected]>
Since we can't provide Peek feature in the callback
(the body data is already read and we can't peek unread data),
here I remove the Peek method. It's fine as this method is not
implemented and can be replaced with Bytes.

Signed-off-by: spacewander <[email protected]>
* golang filter: fetch body data as []byte

Most of the Go libraries consider using []byte is more
efficient. Therefore, they only provide api with []byte.
By returning []byte instead of string, we can avoid an
extra `string -> []byte` copy.

This change also align the behavior with the proxy-wasm-go-sdk,
make it easier to port Wasm plugin to Go plugin.

Signed-off-by: spacewander <[email protected]>

* for write op

Signed-off-by: spacewander <[email protected]>

---------

Signed-off-by: spacewander <[email protected]>
…ng it (envoyproxy#29984)

* bugfix: fix a race when using the cached merged_config_id_ and updating it.

Signed-off-by: doujiang24 <[email protected]>

* fix missing p1.

Signed-off-by: doujiang24 <[email protected]>

* comment in c++.

Signed-off-by: doujiang24 <[email protected]>

* fix comment style.

Signed-off-by: doujiang24 <[email protected]>

* fix comment.

Signed-off-by: doujiang24 <[email protected]>

---------

Signed-off-by: doujiang24 <[email protected]>
…0099)

Also update the comment of this method.

Also remove a stale TODO, as Go doesn't allow to change the cgocheck via compile-time GODEBUG var in Go 1.21. See https://github.com/golang/go/blob/dacf1f1e10a6b1ed02b6b935e502ddf8585b3748/src/internal/godebugs/table.go#L27

Commit Message: golang filter: export the error returned by GetProperty
Additional Description:
Risk Level: Low
Testing: Integration test
Docs Changes:
Release Notes:
Platform Specific Features:

Signed-off-by: spacewander <[email protected]>
…yproxy#30264)

* golang filter: should be able to mock logger
* move cgo to a separate pkg so the common/go/api is cgo=free

Signed-off-by: spacewander <[email protected]>
spacewander and others added 27 commits March 25, 2025 21:13
…ader name (envoyproxy#31392)

* Golang filter: SendLocalReply support multiple header values for a header name

Signed-off-by: Michael Sauter <[email protected]>
Signed-off-by: doujiang24 <[email protected]>
…roxy#32081)

fix envoyproxy#31654 and envoyproxy#29496

Thanks @spacewander for founding this race:
envoyGoRequestSemaDec may be invoked eariler than r.sema.Wait, then, there is no one to resume the r.sema.Wait.

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
Fixes: envoyproxy#31654, envoyproxy#29496


Signed-off-by: doujiang24 <[email protected]>
The current HTTP Go filter utilizes a sync.Map to manage the lifecycle of the requests memory between Go and C++. Envoy instances that have a larger number of workers (>=32), the sync.Map causes contention on the underlying lock and reduces performance.

Before this fix, a direct reply benchmark would get about 210,000 req/sec which is considerably lower than 400,000 req/sec (Envoy that uses a direct reply within a route with no Go HTTP filter). The same benchmark with this fix included gets 350,000 req/sec, a 67% increase in performance.

The sync.Map is replaced with a []map[*C.httpRequest]*httpRequest which allows each worker to get its own map. This slice is initialized envoyGoFilterNewHttpPluginConfig which now passes along the concurrency value that Envoy was started with which controls the number of workers. The httpRequest that is shared between Envoy and Go Plugin has been updated to pass along the worker_id that is responsible for the request. Since each worker is single threaded, we no longer need a mutex to control access to the map.

Fixes envoyproxy#31916

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
Fixes: envoyproxy#31916

Signed-off-by: Braden Bassingthwaite <[email protected]>
…lic API (envoyproxy#32107)

Adds new functions to the Go SDK EnvoyConcurrency() and StreamInfo.WorkerID(). This information will allow Go HTTP plugins to optimize their performance in certain use cases. Follow up from envoyproxy#31987

cc @doujiang24

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:

Signed-off-by: Braden Bassingthwaite <[email protected]>
…roxy#32183)

We followed the C++ style to register configFactory, but it's not a proper choice for Golang filter.
Here is the Reasons:

Golang introduced the ConfigParser interface to Parse/validate a config, configFactory does not need to parse config.
better performance. By using fiterFactory, we could ommit generate a closure per http request, that may take ~1ns.
Yep, this is a breaking change, people need to change the register API.
So, I suggest we fix it earlier.
Signed-off-by: doujiang24 <[email protected]>
envoyproxy#33229)

* golang filter: do not clear route cache in HeaderMap.Set by default.

introduce a new API `ClearRouteCache` to clear route cache.
fix envoyproxy#33082

Signed-off-by: doujiang24 <[email protected]>
…nvoyproxy#34196)

This PR adds support for enableHalfClose for upstream connection in the go network filter. If the user sets
enableHalfClose to true, reading a remote upstream half-close will not fully close the connection.

Future Plan
Support for go network filter can be configured together with HCM, so user can specify upstream conn
enableHalfClose on L4 and manage traffic on L7.

Signed-off-by: duxin <[email protected]>
…proxy#35742)

As discussed from
envoyproxy#35595 (comment),
the `AccessLogHandler::log` is not designed for mutating the StreamInfo.
It's recommended to use `OnStreamComplete` to do the final metadata
management.

Signed-off-by: spacewander <[email protected]>
…e is empty (envoyproxy#35930)

Otherwise, Golang may panic when it found this invalid pointer.
It's not easy to add a test case for it, since Golang only panic during
GC.

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

Signed-off-by: doujiang24 <[email protected]>
…6023)

Commit Message:
Additional Description:
In my local benchmark(requires some hack for benchmark), it makes header
map init faster 10%+.
from 2656ns to 2269ns for per header map init.

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

Signed-off-by: doujiang24 <[email protected]>
<!--
!!!ATTENTION!!!

If you are fixing *any* crash or *any* potential security issue, *do
not*
open a pull request in this repo. Please report the issue via emailing
[email protected] where the issue will be triaged
appropriately.
Thank you in advance for helping to keep Envoy secure.

!!!ATTENTION!!!

For an explanation of how to fill out the fields, please see the
relevant section
in
[PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md)
-->

Commit Message: golang: provide method to refresh route cache
Additional Description:
Risk Level: Low, add new method
Testing: Integration
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
Fixes envoyproxy#36848
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: spacewander <[email protected]>
During Golang GC there will be
envoyGoFilterHttpFinalize -> deferredDeleteRequest -> getDispatcher.
Before this fix, the getDispatcher will use the occasional deleted
decoder callback, which will cause a crash. After this fix, we won't
touch the deleted callback anymore.
Risk Level: Low
Testing: after 3 hours load testing, the Envoy with this fix is still
running well.

Signed-off-by: spacewander <[email protected]>
<!--
!!!ATTENTION!!!

If you are fixing *any* crash or *any* potential security issue, *do
not*
open a pull request in this repo. Please report the issue via emailing
[email protected] where the issue will be triaged
appropriately.
Thank you in advance for helping to keep Envoy secure.

!!!ATTENTION!!!

For an explanation of how to fill out the fields, please see the
relevant section
in
[PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md)
-->

Commit Message: golang: allow injecting extra data
Additional Description: This PR adds a feature that allows users to
flush the data immediately when processing the data asynchronously.
Risk Level: Low
Testing: Integration test
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

Signed-off-by: spacewander <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Mar 25, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 10 committers have signed the CLA.

✅ spacewander
✅ johnlanni
❌ antJack
❌ doujiang24
❌ StarryVae
❌ willemveerman
❌ phlax
❌ bbassingthwaite
❌ duxin40
❌ Meshkati
You have signed the CLA already but the status is still pending? Let us recheck it.

@johnlanni johnlanni merged commit 6d786e2 into higress-group:envoy-1.27 Mar 25, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.