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.

Sign up for GitHub

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

Upstreaming of proxy-wasm Part 1. #10262

Closed
wants to merge 20 commits into from
Closed

Upstreaming of proxy-wasm Part 1. #10262

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
972166c
Upstreaming of proxy-wasm Part 1.
jplevyak Mar 5, 2020
8b5efdc
Format.
jplevyak Mar 6, 2020
65bd8ae
Address comments.
jplevyak Mar 6, 2020
b21914e
Address comments.
jplevyak Mar 6, 2020
3204e82
Address comments.
jplevyak Mar 6, 2020
3a25b41
Address comments.
jplevyak Mar 6, 2020
7548daf
Format.
jplevyak Mar 7, 2020
3518e10
Format.
jplevyak Mar 7, 2020
c9bfb2e
Format.
jplevyak Mar 9, 2020
b6f1ce6
Fix formatting.
jplevyak Mar 9, 2020
7832462
Remove local repo reference.
jplevyak Mar 9, 2020
489f8f8
Remove WasmState which is not needed in this PR.
jplevyak Mar 9, 2020
b21857a
Address comments.
jplevyak Mar 9, 2020
bbc7922
Format.
jplevyak Mar 10, 2020
6f29c2a
Move pointer using statement to core.
jplevyak Mar 10, 2020
b3a3072
Merge remote-tracking branch 'envoyproxy/master' into wasm-upstream-base
jplevyak Mar 10, 2020
e85d273
Add Random as required by RemoteAsyncDataProvider.
jplevyak Mar 11, 2020
167c344
Attempt to fix the @envoy dependency problem.
jplevyak Mar 12, 2020
06581c7
Sort alphabetically because that was sooooo important.
jplevyak Mar 12, 2020
4119e6b
Address some coverage.
jplevyak Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions source/common/stats/symbol_table_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,7 @@ class StatNameSet {
using StringStatNameMap = absl::flat_hash_map<std::string, Stats::StatName>;
StringStatNameMap builtin_stat_names_;
};
using StatNameSetSharedPtr = std::shared_ptr<Stats::StatNameSet>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect this is not a great pattern for using StatNameSet; I'm inclined not to institutionalize it by centralizing a shortcut for it.

But it's worth exploring what your data model is. Is the StatNameSet shared across all threads, all VMs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The VMs are running the same code and there are many of them (e.g 60 on a large system), so the replication would be large without sharing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

envoyproxy/envoy-wasm#452

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed on envoyproxy/envoy-wasm#452 we have a new solution and are testing this now. I will upstream that change assuming it works out. Let's not let that hold up this review.


} // namespace Stats
} // namespace Envoy
19 changes: 4 additions & 15 deletions source/extensions/common/wasm/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ envoy_cc_library(
srcs = ["wasm_vm.cc"],
deps = [
":wasm_hdr",
"@proxy_wasm_cpp_host//:lib",
"@proxy_wasm_cpp_sdk//:common_lib",
"//source/common/common:assert_lib",
"//source/common/stats:stats_lib",
"@proxy_wasm_cpp_host//:lib",
"@proxy_wasm_cpp_sdk//:common_lib",
],
)

Expand All @@ -37,23 +37,13 @@ envoy_cc_library(
"wasm_vm.h",
],
deps = [
"@proxy_wasm_cpp_host//:include",
"@proxy_wasm_cpp_sdk//:common_lib",
":well_known_names",
"//include/envoy/upstream:cluster_manager_interface",
"//source/common/config:datasource_lib",
"//source/common/stats:stats_lib",
"@envoy_api//envoy/extensions/wasm/v3:pkg_cc_proto",
],
)

envoy_cc_library(
name = "wasm_interoperation_lib",
srcs = ["wasm_state.cc"],
hdrs = ["wasm_state.h"],
deps = [
"//include/envoy/stream_info:filter_state_interface",
"//source/common/protobuf",
"@proxy_wasm_cpp_host//:include",
"@proxy_wasm_cpp_sdk//:common_lib",
],
)

Expand All @@ -65,7 +55,6 @@ envoy_cc_library(
],
deps = [
":wasm_hdr",
":wasm_interoperation_lib",
":wasm_vm_lib",
"//external:abseil_base",
"//external:abseil_node_hash_map",
Expand Down
1 change: 0 additions & 1 deletion source/extensions/common/wasm/context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "common/common/logger.h"

#include "extensions/common/wasm/wasm.h"
#include "extensions/common/wasm/wasm_state.h"
#include "extensions/common/wasm/well_known_names.h"

#include "absl/base/casts.h"
Expand Down
9 changes: 6 additions & 3 deletions source/extensions/common/wasm/context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
#include <map>
#include <memory>

#include "include/proxy-wasm/context.h"

#include "envoy/access_log/access_log.h"
#include "envoy/buffer/buffer.h"
#include "envoy/http/filter.h"
Expand All @@ -14,6 +12,9 @@
#include "common/common/assert.h"
#include "common/common/logger.h"

#include "include/proxy-wasm/context.h"
#include "include/proxy-wasm/wasm.h"

namespace Envoy {
namespace Extensions {
namespace Common {
Expand Down Expand Up @@ -64,6 +65,8 @@ struct Plugin : public PluginBase {
: PluginBase(name, root_id, vm_id, plugin_configuration), direction_(direction),
local_info_(local_info), listener_metadata_(listener_metadata) {}

// Information specific to HTTP Filters.
// TODO: consider using a variant record or filter-type specific sub-objects via unique_ptr.
const envoy::config::core::v3::TrafficDirection direction_;
const LocalInfo::LocalInfo& local_info_;
const envoy::config::core::v3::Metadata* listener_metadata_;
Expand Down Expand Up @@ -116,7 +119,7 @@ class Context : public proxy_wasm::ContextBase,

const LocalInfo::LocalInfo* root_local_info_{nullptr}; // set only for root_context.

// State use for calls into the VM.
// Temporary state used for and valid only during calls into the VM.
Buffer buffer_;
jplevyak marked this conversation as resolved.
Show resolved Hide resolved
};
using ContextSharedPtr = std::shared_ptr<Context>;
Expand Down
3 changes: 1 addition & 2 deletions source/extensions/common/wasm/wasm.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@
#include "common/common/enum_to_int.h"
#include "common/common/logger.h"

#include "extensions/common/wasm/wasm_state.h"
#include "extensions/common/wasm/well_known_names.h"
#include "extensions/common/wasm/wasm_vm.h"
#include "extensions/common/wasm/well_known_names.h"

#include "absl/base/casts.h"
#include "absl/container/flat_hash_map.h"
Expand Down
12 changes: 3 additions & 9 deletions source/extensions/common/wasm/wasm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
#include <map>
#include <memory>

#include "include/proxy-wasm/exports.h"
#include "include/proxy-wasm/wasm.h"

#include "envoy/common/exception.h"
#include "envoy/extensions/wasm/v3/wasm.pb.validate.h"
#include "envoy/stats/scope.h"
Expand All @@ -23,13 +20,10 @@
#include "extensions/common/wasm/wasm_vm.h"
#include "extensions/common/wasm/well_known_names.h"

namespace Envoy {

// TODO: move to source/common/stats/symbol_table_impl.h when upstreaming.
namespace Stats {
using StatNameSetSharedPtr = std::shared_ptr<Stats::StatNameSet>;
} // namespace Stats
#include "include/proxy-wasm/exports.h"
#include "include/proxy-wasm/wasm.h"

namespace Envoy {
namespace Extensions {
namespace Common {
namespace Wasm {
Expand Down
30 changes: 0 additions & 30 deletions source/extensions/common/wasm/wasm_state.cc
View file
Open in desktop

This file was deleted.

37 changes: 0 additions & 37 deletions source/extensions/common/wasm/wasm_state.h
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion source/extensions/common/wasm/wasm_vm.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

#include <memory>

#include "extensions/common/wasm/well_known_names.h"

#include "include/proxy-wasm/null.h"
#include "include/proxy-wasm/v8.h"
#include "extensions/common/wasm/well_known_names.h"

namespace Envoy {
namespace Extensions {
Expand Down
3 changes: 1 addition & 2 deletions source/extensions/common/wasm/wasm_vm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@

#include <memory>

#include "absl/strings/str_cat.h"

#include "envoy/common/exception.h"
#include "envoy/stats/scope.h"
#include "envoy/stats/stats.h"
#include "envoy/stats/stats_macros.h"

#include "common/common/logger.h"

#include "absl/strings/str_cat.h"
#include "include/proxy-wasm/wasm_vm.h"
#include "include/proxy-wasm/word.h"

Expand Down
12 changes: 7 additions & 5 deletions test/extensions/common/wasm/test_data/test_cpp_null_plugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// NOLINT(namespace-envoy)
#include "include/proxy-wasm/null_plugin.h"

namespace proxy_wasm {
namespace null_plugin {
namespace CommonWasmTestCpp {
NullPluginRegistry *context_registry_;
} // CommonWasmtestCpp
NullPluginRegistry* context_registry_;
} // namespace CommonWasmTestCpp

RegisterNullVmPluginFactory register_common_wasm_test_cpp_plugin("CommonWasmTestCpp",
[]() { return std::make_unique<NullPlugin>(CommonWasmTestCpp::context_registry_); });
RegisterNullVmPluginFactory register_common_wasm_test_cpp_plugin("CommonWasmTestCpp", []() {
return std::make_unique<NullPlugin>(CommonWasmTestCpp::context_registry_);
});

} // namespace null_plugin;
} // namespace null_plugin
} // namespace proxy_wasm
2 changes: 1 addition & 1 deletion test/extensions/common/wasm/wasm_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ TEST_P(WasmCommonTest, Logging) {
wasm_weak.lock()->configure(root_context, plugin);
wasm_handler.reset();
dispatcher->run(Event::Dispatcher::RunType::NonBlock);
// This will SEGV on nullptr if wasm has been deleted.
// This will fault on nullptr if wasm has been deleted.
plugin->plugin_configuration_ = "done";
wasm_weak.lock()->configure(root_context, plugin);
dispatcher->run(Event::Dispatcher::RunType::NonBlock);
Expand Down
2 changes: 1 addition & 1 deletion test/extensions/common/wasm/wasm_vm_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

#include "common/stats/isolated_store_impl.h"

#include "include/proxy-wasm/null_vm_plugin.h"
#include "extensions/common/wasm/wasm_vm.h"

#include "test/test_common/environment.h"
#include "test/test_common/utility.h"

#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "include/proxy-wasm/null_vm_plugin.h"

using proxy_wasm::WasmCallVoid;
using proxy_wasm::WasmCallWord;
Expand Down
1 change: 1 addition & 0 deletions tools/spelling/spelling_dictionary.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ dynamodb
emplace
emplaced
emscripten
emsdk
enablement
encodings
endian
Expand Down