Skip to content
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

Envoy's L7 Go extension API #22573

Merged
merged 119 commits into from
Jan 5, 2023
Merged

Conversation

wangfakang
Copy link
Member

This PR is about Envoy's Go extension API , which provides a API for extending Envoy's L7 filter via go. Developers can use the Go to implement filter processing of request or response streams, and the final effect is the same as that of Envoy's native filter, which makes it easier to extend Envoy. The implementation will be pushed after the community agrees on the design and completes the API.

The following describes the interaction between Envoy and the filter implemented with the Go extension API:

moe api

@mattklein123 @htuch @lizan @bkgs @tgraf @jrajahalme
Looking forward to your ideas/comments on this PR!

Commit Message: Add API for Envoy's L7 Go extension
Design doc: https://docs.google.com/document/d/1noApyS0IfmOGmEOHdWk2-BOp0V37zgXMM4MdByr1lQk/edit?usp=sharing
Additional Description: #15152
Risk Level: low
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:]

@wangfakang
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #22573 (comment) was created by @wangfakang.

see: more, trace.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/wait

contrib/golang/filters/http/source/golang_filter.cc Outdated Show resolved Hide resolved
@wangfakang
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #22573 (comment) was created by @wangfakang.

see: more, trace.

@wangfakang
Copy link
Member Author

Thanks this is looking pretty close. 2 other comments:

  1. Add a release note in current.yaml and reference new docs.
  2. Contrib extensions do not have ASAN and TSAN run on them by default. Manually run all the new tests with ASAN and TSAN and post the results. Thank you.

/wait

@mattklein123 Thank you very much for your careful review, which has taught me a lot. I used clang-asan and clang-tsan to successfully run all the test cases of the go extension. Some results are as below.

ASAN

$bazel test -c fastbuild //contrib/golang/filters/http/test/...  --config=clang-asan --test_verbose_timeout_warnings   --test_env=ENVOY_IP_TEST_VERSIONS=v4only  --verbose_failures  --test_arg="-l debug"  --define=wasm=disabled
DEBUG: /home/fakang.wfk/.cache/bazel/_bazel_fakang.wfk/05ab2d40a050525b20b2187bf5222296/external/rules_python/python/pip.bzl:46:10: pip_install is deprecated. Please switch to pip_parse. pip_install will be removed in a future release.
INFO: Analyzed 9 targets (0 packages loaded, 0 targets configured).
INFO: Found 5 targets and 4 test targets...
INFO: From Executing genrule @io_opentracing_cpp//:generate_version_h:
-- The C compiler identification is Clang 11.0.0
-- The CXX compiler identification is Clang 11.0.0
-- Check for working C compiler: /usr/local/bin/clang
-- Check for working C compiler: /usr/local/bin/clang -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/local/bin/clang++
-- Check for working CXX compiler: /usr/local/bin/clang++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- clang-tidy not found.
-- Configuring done
-- Generating done
-- Build files have been written to: /tmp/tmp.9MvmbsVAFz
-- Cache values
BUILD_DYNAMIC_LOADING:BOOL=ON
BUILD_MOCKTRACER:BOOL=OFF
BUILD_SHARED_LIBS:BOOL=ON
BUILD_STATIC_LIBS:BOOL=ON
BUILD_TESTING:BOOL=OFF
CLANG_TIDY_EXE:FILEPATH=CLANG_TIDY_EXE-NOTFOUND
CMAKE_BUILD_TYPE:STRING=
CMAKE_INSTALL_PREFIX:PATH=/usr/local
ENABLE_LINTING:BOOL=ON
INFO: From Generating proto_library @com_google_googleapis//google/devtools/cloudtrace/v2:cloudtrace_proto:
google/devtools/cloudtrace/v2/trace.proto:19:1: warning: Import google/api/annotations.proto is unused.
google/devtools/cloudtrace/v2/tracing.proto:26:1: warning: Import google/protobuf/timestamp.proto is unused.
INFO: From Compiling eval/public/structs/protobuf_descriptor_type_provider.cc:
In file included from external/com_google_cel_cpp/eval/public/structs/protobuf_descriptor_type_provider.cc:15:
external/com_google_cel_cpp/eval/public/structs/protobuf_descriptor_type_provider.h:53:27: warning: private field 'unboxing_option_' is not used [-Wunused-private-field]
  ProtoWrapperTypeOptions unboxing_option_;
                          ^
1 warning generated.
INFO: From Action external/opencensus_proto/opencensus/proto/agent/trace/v1/trace_service.grpc.pb.h:
bazel-out/k8-fastbuild/bin/external/opencensus_proto/external/opencensus_proto: warning: directory does not exist.
INFO: From Action external/com_google_googleapis/google/devtools/cloudtrace/v2/trace.grpc.pb.h:
bazel-out/k8-fastbuild/bin/external/com_google_googleapis/external/com_google_googleapis: warning: directory does not exist.
google/devtools/cloudtrace/v2/trace.proto:19:1: warning: Import google/api/annotations.proto is unused.
google/devtools/cloudtrace/v2/tracing.proto:26:1: warning: Import google/protobuf/timestamp.proto is unused.
INFO: Elapsed time: 2716.501s, Critical Path: 351.98s
INFO: 4591 processes: 1559 internal, 3032 linux-sandbox.
INFO: Build completed successfully, 4591 total actions

Executed 4 out of 4 tests: 4 tests pass.

TSAN

$bazel test -c fastbuild //contrib/golang/filters/http/test/...   --config=clang-tsan --test_verbose_timeout_warnings   --test_env=ENVOY_IP_TEST_VERSIONS=v4only --verbose_failures  --config=gcc  --test_arg="-l debug"  --define=wasm=disabled --build_tests_only
Starting local Bazel server and connecting to it...
WARNING: option '--build_tag_filters' was expanded to from both option '--config=clang-tsan' (source command line options) and option '--config=clang-tsan' (source command line options)
WARNING: option '--test_tag_filters' was expanded to from both option '--config=clang-tsan' (source command line options) and option '--config=clang-tsan' (source command line options)
DEBUG: /home/fakang.wfk/.cache/bazel/_bazel_fakang.wfk/05ab2d40a050525b20b2187bf5222296/external/rules_python/python/pip.bzl:46:10: pip_install is deprecated. Please switch to pip_parse. pip_install will be removed in a future release.
WARNING: option '--build_tag_filters' was expanded to from both option '--config=clang-tsan' (source command line options) and option '--config=clang-tsan' (source command line options)
WARNING: option '--test_tag_filters' was expanded to from both option '--config=clang-tsan' (source command line options) and option '--config=clang-tsan' (source command line options)
INFO: Analyzed 4 targets (930 packages loaded, 32893 targets configured).
INFO: Found 4 test targets...
INFO: Deleting stale sandbox base /home/fakang.wfk/.cache/bazel/_bazel_fakang.wfk/05ab2d40a050525b20b2187bf5222296/sandbox
INFO: Elapsed time: 566.694s, Critical Path: 471.52s
INFO: 22 processes: 1 internal, 21 linux-sandbox.
INFO: Build completed successfully, 22 total actions

Executed 3 out of 4 tests: 4 tests pass.

@mattklein123 PTAL. Thanks

@wangfakang
Copy link
Member Author

Thanks this is looking pretty close. 2 other comments:

  1. Add a release note in current.yaml and reference new docs.
  2. Contrib extensions do not have ASAN and TSAN run on them by default. Manually run all the new tests with ASAN and TSAN and post the results. Thank you.

/wait

@mattklein123 Thank you very much for your careful review, which has taught me a lot. I used clang-asan and clang-tsan to successfully run all the test cases of the go extension. Some results are as below.

ASAN

$bazel test -c fastbuild //contrib/golang/filters/http/test/...  --config=clang-asan --test_verbose_timeout_warnings   --test_env=ENVOY_IP_TEST_VERSIONS=v4only  --verbose_failures  --test_arg="-l debug"  --define=wasm=disabled
DEBUG: /home/fakang.wfk/.cache/bazel/_bazel_fakang.wfk/05ab2d40a050525b20b2187bf5222296/external/rules_python/python/pip.bzl:46:10: pip_install is deprecated. Please switch to pip_parse. pip_install will be removed in a future release.
INFO: Analyzed 9 targets (0 packages loaded, 0 targets configured).
INFO: Found 5 targets and 4 test targets...
INFO: From Executing genrule @io_opentracing_cpp//:generate_version_h:
-- The C compiler identification is Clang 11.0.0
-- The CXX compiler identification is Clang 11.0.0
-- Check for working C compiler: /usr/local/bin/clang
-- Check for working C compiler: /usr/local/bin/clang -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/local/bin/clang++
-- Check for working CXX compiler: /usr/local/bin/clang++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- clang-tidy not found.
-- Configuring done
-- Generating done
-- Build files have been written to: /tmp/tmp.9MvmbsVAFz
-- Cache values
BUILD_DYNAMIC_LOADING:BOOL=ON
BUILD_MOCKTRACER:BOOL=OFF
BUILD_SHARED_LIBS:BOOL=ON
BUILD_STATIC_LIBS:BOOL=ON
BUILD_TESTING:BOOL=OFF
CLANG_TIDY_EXE:FILEPATH=CLANG_TIDY_EXE-NOTFOUND
CMAKE_BUILD_TYPE:STRING=
CMAKE_INSTALL_PREFIX:PATH=/usr/local
ENABLE_LINTING:BOOL=ON
INFO: From Generating proto_library @com_google_googleapis//google/devtools/cloudtrace/v2:cloudtrace_proto:
google/devtools/cloudtrace/v2/trace.proto:19:1: warning: Import google/api/annotations.proto is unused.
google/devtools/cloudtrace/v2/tracing.proto:26:1: warning: Import google/protobuf/timestamp.proto is unused.
INFO: From Compiling eval/public/structs/protobuf_descriptor_type_provider.cc:
In file included from external/com_google_cel_cpp/eval/public/structs/protobuf_descriptor_type_provider.cc:15:
external/com_google_cel_cpp/eval/public/structs/protobuf_descriptor_type_provider.h:53:27: warning: private field 'unboxing_option_' is not used [-Wunused-private-field]
  ProtoWrapperTypeOptions unboxing_option_;
                          ^
1 warning generated.
INFO: From Action external/opencensus_proto/opencensus/proto/agent/trace/v1/trace_service.grpc.pb.h:
bazel-out/k8-fastbuild/bin/external/opencensus_proto/external/opencensus_proto: warning: directory does not exist.
INFO: From Action external/com_google_googleapis/google/devtools/cloudtrace/v2/trace.grpc.pb.h:
bazel-out/k8-fastbuild/bin/external/com_google_googleapis/external/com_google_googleapis: warning: directory does not exist.
google/devtools/cloudtrace/v2/trace.proto:19:1: warning: Import google/api/annotations.proto is unused.
google/devtools/cloudtrace/v2/tracing.proto:26:1: warning: Import google/protobuf/timestamp.proto is unused.
INFO: Elapsed time: 2716.501s, Critical Path: 351.98s
INFO: 4591 processes: 1559 internal, 3032 linux-sandbox.
INFO: Build completed successfully, 4591 total actions

Executed 4 out of 4 tests: 4 tests pass.

TSAN

$bazel test -c fastbuild //contrib/golang/filters/http/test/...   --config=clang-tsan --test_verbose_timeout_warnings   --test_env=ENVOY_IP_TEST_VERSIONS=v4only --verbose_failures  --config=gcc  --test_arg="-l debug"  --define=wasm=disabled --build_tests_only
Starting local Bazel server and connecting to it...
WARNING: option '--build_tag_filters' was expanded to from both option '--config=clang-tsan' (source command line options) and option '--config=clang-tsan' (source command line options)
WARNING: option '--test_tag_filters' was expanded to from both option '--config=clang-tsan' (source command line options) and option '--config=clang-tsan' (source command line options)
DEBUG: /home/fakang.wfk/.cache/bazel/_bazel_fakang.wfk/05ab2d40a050525b20b2187bf5222296/external/rules_python/python/pip.bzl:46:10: pip_install is deprecated. Please switch to pip_parse. pip_install will be removed in a future release.
WARNING: option '--build_tag_filters' was expanded to from both option '--config=clang-tsan' (source command line options) and option '--config=clang-tsan' (source command line options)
WARNING: option '--test_tag_filters' was expanded to from both option '--config=clang-tsan' (source command line options) and option '--config=clang-tsan' (source command line options)
INFO: Analyzed 4 targets (930 packages loaded, 32893 targets configured).
INFO: Found 4 test targets...
INFO: Deleting stale sandbox base /home/fakang.wfk/.cache/bazel/_bazel_fakang.wfk/05ab2d40a050525b20b2187bf5222296/sandbox
INFO: Elapsed time: 566.694s, Critical Path: 471.52s
INFO: 22 processes: 1 internal, 21 linux-sandbox.
INFO: Build completed successfully, 22 total actions

Executed 3 out of 4 tests: 4 tests pass.

@mattklein123 Thanks for your comments, all of them are addressed. Thanks

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/wait

contrib/golang/filters/http/source/common/dso/dso.h Outdated Show resolved Hide resolved
contrib/golang/filters/http/source/golang_filter.h Outdated Show resolved Hide resolved
Signed-off-by: wangfakang <fakangwang@gmail.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM to ship and iterate! Cool stuff.

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Jan 5, 2023
@mattklein123 mattklein123 merged commit c256b94 into envoyproxy:main Jan 5, 2023
VishalDamgude pushed a commit to freshworks/envoy that referenced this pull request Feb 1, 2023
Signed-off-by: wangfakang <fakangwang@gmail.com>
Signed-off-by: VishalDamgude <vishal.damgude@freshworks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants