diff --git a/bazel/BUILD b/bazel/BUILD index adb3fe7905dc..9bfb8f183461 100644 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -517,6 +517,11 @@ config_setting( values = {"define": "perf_annotation=enabled"}, ) +config_setting( + name = "enable_execution_context", + values = {"define": "execution_context=enabled"}, +) + config_setting( name = "enable_perf_tracing", values = {"define": "perf_tracing=enabled"}, diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl index b84d93ef7a5b..015659851c1b 100644 --- a/bazel/envoy_internal.bzl +++ b/bazel/envoy_internal.bzl @@ -125,6 +125,7 @@ def envoy_copts(repository, test = False): envoy_select_static_extension_registration(["-DENVOY_STATIC_EXTENSION_REGISTRATION"], repository) + \ envoy_select_disable_logging(["-DENVOY_DISABLE_LOGGING"], repository) + \ _envoy_select_perf_annotation(["-DENVOY_PERF_ANNOTATION"]) + \ + _envoy_select_execution_context() + \ _envoy_select_perfetto(["-DENVOY_PERFETTO"]) + \ envoy_select_google_grpc(["-DENVOY_GOOGLE_GRPC"], repository) + \ envoy_select_signal_trace(["-DENVOY_HANDLE_SIGNALS"], repository) + \ @@ -190,6 +191,12 @@ def _envoy_select_perf_annotation(xs): "//conditions:default": [], }) +def _envoy_select_execution_context(): + return select({ + "@envoy//bazel:enable_execution_context": ["-DENVOY_ENABLE_EXECUTION_CONTEXT"], + "//conditions:default": [], + }) + def _envoy_select_perfetto(xs): return select({ "@envoy//bazel:enable_perf_tracing": xs, diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 256fd4a98fb8..58c20865ca4e 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -54,6 +54,10 @@ behavior_changes: `. This change can be disabled by setting the runtime guard flag ``envoy.reloadable_features.filter_access_loggers_first`` to ``false``. +- area: monitoring + change: | + Removed runtime feature flag ``envoy.restart_features.enable_execution_context``. The execution context feature + now could be enabled only by setting compile option ``--define=execution_context=enabled``. minor_behavior_changes: # *Changes that may cause incompatibilities for some users, but should not for most* diff --git a/envoy/common/BUILD b/envoy/common/BUILD index 3c8680576e4b..12c71b554bbf 100644 --- a/envoy/common/BUILD +++ b/envoy/common/BUILD @@ -124,15 +124,19 @@ envoy_cc_library( envoy_cc_library( name = "execution_context", hdrs = ["execution_context.h"], - deps = [":pure_lib"], + deps = [ + ":pure_lib", + ":scope_tracker_interface", + ], ) envoy_cc_library( name = "scope_tracker_interface", hdrs = ["scope_tracker.h"], deps = [ - ":execution_context", + ":optref_lib", ":pure_lib", + "//envoy/stream_info:stream_info_interface", ], ) diff --git a/envoy/common/execution_context.h b/envoy/common/execution_context.h index fb227a05af41..b723a221bcae 100644 --- a/envoy/common/execution_context.h +++ b/envoy/common/execution_context.h @@ -3,22 +3,25 @@ #include #include "envoy/common/pure.h" +#include "envoy/common/scope_tracker.h" +#include "envoy/stream_info/stream_info.h" #include "source/common/common/non_copyable.h" namespace Envoy { +#ifdef ENVOY_ENABLE_EXECUTION_CONTEXT + +static constexpr absl::string_view kConnectionExecutionContextFilterStateName = + "envoy.network.connection_execution_context"; + class ScopedExecutionContext; // ExecutionContext can be inherited by subclasses to represent arbitrary information associated // with the execution of a piece of code. activate/deactivate are called when the said execution // starts/ends. For an example usage, please see // https://github.com/envoyproxy/envoy/issues/32012. -class ExecutionContext : NonCopyable { -public: - ExecutionContext() = default; - virtual ~ExecutionContext() = default; - +class ExecutionContext : public StreamInfo::FilterState::Object, NonCopyable { protected: // Called when the current thread starts to run code on behalf of the owner of this object. // protected because it should only be called by ScopedExecutionContext. @@ -43,7 +46,8 @@ class ExecutionContext : NonCopyable { class ScopedExecutionContext : NonCopyable { public: ScopedExecutionContext() : ScopedExecutionContext(nullptr) {} - ScopedExecutionContext(ExecutionContext* context) : context_(context) { + ScopedExecutionContext(const ScopeTrackedObject* object) + : context_(object != nullptr ? getExecutionContext(object->trackedStream()) : nullptr) { if (context_ != nullptr) { context_->activate(); } @@ -62,7 +66,18 @@ class ScopedExecutionContext : NonCopyable { bool isNull() const { return context_ == nullptr; } private: + ExecutionContext* getExecutionContext(OptRef info) { + if (!info.has_value()) { + return nullptr; + } + const auto* const_context = info->filterState().getDataReadOnly( + kConnectionExecutionContextFilterStateName); + return const_cast(const_context); + } + ExecutionContext* context_; }; +#endif + } // namespace Envoy diff --git a/envoy/common/scope_tracker.h b/envoy/common/scope_tracker.h index f96425541513..e846f284e412 100644 --- a/envoy/common/scope_tracker.h +++ b/envoy/common/scope_tracker.h @@ -2,16 +2,17 @@ #include -#include "envoy/common/execution_context.h" +#include "envoy/common/optref.h" #include "envoy/common/pure.h" +#include "envoy/stream_info/stream_info.h" namespace Envoy { /* * An interface for tracking the scope of work. Implementors of this interface * can be registered to the dispatcher when they're active on the stack. If a - * fatal error occurs while they were active, the dumpState method will be - * called. + * fatal error occurs while they were active, the dumpState() method will be + * called to output the active state. * * Currently this is only used for the L4 network connection and L7 stream. */ @@ -20,9 +21,11 @@ class ScopeTrackedObject { virtual ~ScopeTrackedObject() = default; /** - * If the tracked object has a ExecutionContext, returns it. Returns nullptr otherwise. + * Return the tracked stream info that related to the scope tracked object (L4 + * network connection or L7 stream). + * @return optional reference to stream info of stream (L4 connection or L7 stream). */ - virtual ExecutionContext* executionContext() const { return nullptr; } + virtual OptRef trackedStream() const { return {}; } /** * Dump debug state of the object in question to the provided ostream. diff --git a/envoy/network/BUILD b/envoy/network/BUILD index f40b97d53073..95ab4e4677e5 100644 --- a/envoy/network/BUILD +++ b/envoy/network/BUILD @@ -25,6 +25,7 @@ envoy_cc_library( ":filter_interface", ":listen_socket_interface", "//envoy/buffer:buffer_interface", + "//envoy/common:scope_tracker_interface", "//envoy/event:deferred_deletable", "//envoy/ssl:connection_interface", "//envoy/stream_info:stream_info_interface", @@ -174,7 +175,6 @@ envoy_cc_library( deps = [ ":io_handle_interface", ":socket_interface", - "//envoy/common:scope_tracker_interface", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/envoy/network/listen_socket.h b/envoy/network/listen_socket.h index e600d2d0e42c..08f4a4d5a672 100644 --- a/envoy/network/listen_socket.h +++ b/envoy/network/listen_socket.h @@ -7,7 +7,6 @@ #include "envoy/common/exception.h" #include "envoy/common/pure.h" -#include "envoy/common/scope_tracker.h" #include "envoy/config/core/v3/base.pb.h" #include "envoy/network/address.h" #include "envoy/network/io_handle.h" @@ -26,7 +25,7 @@ namespace Network { * TODO(jrajahalme): Hide internals (e.g., fd) from listener filters by providing callbacks filters * may need (set/getsockopt(), peek(), recv(), etc.) */ -class ConnectionSocket : public virtual Socket, public virtual ScopeTrackedObject { +class ConnectionSocket : public virtual Socket { public: /** * Set detected transport protocol (e.g. RAW_BUFFER, TLS). @@ -83,6 +82,16 @@ class ConnectionSocket : public virtual Socket, public virtual ScopeTrackedObjec * return value is cwnd(in packets) times the connection's MSS. */ virtual absl::optional congestionWindowInBytes() const PURE; + + /** + * Dump debug state of the object in question to the provided ostream. + * + * This is called on Envoy fatal errors, so should do minimal memory allocation. + * + * @param os the ostream to output to. + * @param indent_level how far to indent, for pretty-printed classes and subclasses. + */ + virtual void dumpState(std::ostream& os, int indent_level = 0) const PURE; }; using ConnectionSocketPtr = std::unique_ptr; diff --git a/source/common/common/scope_tracker.h b/source/common/common/scope_tracker.h index dfe994704245..9deb53381d78 100644 --- a/source/common/common/scope_tracker.h +++ b/source/common/common/scope_tracker.h @@ -18,9 +18,7 @@ namespace Envoy { class ScopeTrackerScopeState { public: ScopeTrackerScopeState(const ScopeTrackedObject* object, Event::ScopeTracker& tracker) - : registered_object_(object), - scoped_execution_context_(executionContextEnabled() ? object->executionContext() : nullptr), - tracker_(tracker) { + : registered_object_(object), tracker_(tracker) { tracker_.pushTrackedObject(registered_object_); } @@ -36,14 +34,13 @@ class ScopeTrackerScopeState { private: friend class ScopeTrackerScopeStateTest; - static bool& executionContextEnabled() { - static bool enabled = - Runtime::runtimeFeatureEnabled("envoy.restart_features.enable_execution_context"); - return enabled; - } + const ScopeTrackedObject* registered_object_; - ScopedExecutionContext scoped_execution_context_; Event::ScopeTracker& tracker_; + +#ifdef ENVOY_ENABLE_EXECUTION_CONTEXT + ScopedExecutionContext scoped_execution_context_{registered_object_}; +#endif }; } // namespace Envoy diff --git a/source/common/http/BUILD b/source/common/http/BUILD index f0dc9190b85c..cf716abd8789 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -392,7 +392,6 @@ envoy_cc_library( "//source/common/config:utility_lib", "//source/common/http/http1:codec_lib", "//source/common/http/http2:codec_lib", - "//source/common/network:common_connection_filter_states_lib", "//source/common/network:proxy_protocol_filter_state_lib", "//source/common/network:utility_lib", "//source/common/quic:quic_server_factory_stub_lib", diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index aef542307ff1..d4f73ccef45d 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -45,7 +45,6 @@ #include "source/common/http/user_agent.h" #include "source/common/http/utility.h" #include "source/common/local_reply/local_reply.h" -#include "source/common/network/common_connection_filter_states.h" #include "source/common/network/proxy_protocol_filter_state.h" #include "source/common/stream_info/stream_info_impl.h" #include "source/common/tracing/http_tracer_impl.h" @@ -218,10 +217,9 @@ class ConnectionManagerImpl : Logger::Loggable, } // ScopeTrackedObject - ExecutionContext* executionContext() const override { - return getConnectionExecutionContext(connection_manager_.read_callbacks_->connection()); + OptRef trackedStream() const override { + return filter_manager_.trackedStream(); } - void dumpState(std::ostream& os, int indent_level = 0) const override { const char* spaces = spacesForLevel(indent_level); os << spaces << "ActiveStream " << this << DUMP_MEMBER(stream_id_); diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index f1698144b390..1798106f864a 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -651,6 +651,7 @@ class FilterManager : public ScopeTrackedObject, } // ScopeTrackedObject + OptRef trackedStream() const override { return streamInfo(); } void dumpState(std::ostream& os, int indent_level = 0) const override { const char* spaces = spacesForLevel(indent_level); os << spaces << "FilterManager " << this << DUMP_MEMBER(state_.has_1xx_headers_) diff --git a/source/common/http/http1/BUILD b/source/common/http/http1/BUILD index 0e31c65919c8..564dfd31f354 100644 --- a/source/common/http/http1/BUILD +++ b/source/common/http/http1/BUILD @@ -60,7 +60,6 @@ envoy_cc_library( "//source/common/http:headers_lib", "//source/common/http:status_lib", "//source/common/http:utility_lib", - "//source/common/network:common_connection_filter_states_lib", "//source/common/runtime:runtime_features_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 5e054058a996..795fc40b72f3 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -24,7 +24,6 @@ #include "source/common/http/http1/header_formatter.h" #include "source/common/http/http1/legacy_parser_impl.h" #include "source/common/http/utility.h" -#include "source/common/network/common_connection_filter_states.h" #include "source/common/runtime/runtime_features.h" #include "absl/container/fixed_array.h" @@ -976,8 +975,8 @@ void ConnectionImpl::onResetStreamBase(StreamResetReason reason) { onResetStream(reason); } -ExecutionContext* ConnectionImpl::executionContext() const { - return getConnectionExecutionContext(connection_); +OptRef ConnectionImpl::trackedStream() const { + return connection_.trackedStream(); } void ConnectionImpl::dumpState(std::ostream& os, int indent_level) const { diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index 26e3bfe82d9f..8520e7354ee3 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -272,7 +272,7 @@ class ConnectionImpl : public virtual Connection, Envoy::Http::Status codec_status_; // ScopeTrackedObject - ExecutionContext* executionContext() const override; + OptRef trackedStream() const override; void dumpState(std::ostream& os, int indent_level) const override; protected: diff --git a/source/common/http/http2/BUILD b/source/common/http/http2/BUILD index 281377db831e..34b9dadd3868 100644 --- a/source/common/http/http2/BUILD +++ b/source/common/http/http2/BUILD @@ -56,7 +56,6 @@ envoy_cc_library( "//source/common/http:headers_lib", "//source/common/http:status_lib", "//source/common/http:utility_lib", - "//source/common/network:common_connection_filter_states_lib", "//source/common/runtime:runtime_features_lib", "@com_github_google_quiche//:http2_adapter", "@com_google_absl//absl/algorithm", diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index 92ce3565409d..d707a6072590 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -25,7 +25,6 @@ #include "source/common/http/headers.h" #include "source/common/http/http2/codec_stats.h" #include "source/common/http/utility.h" -#include "source/common/network/common_connection_filter_states.h" #include "source/common/runtime/runtime_features.h" #include "absl/cleanup/cleanup.h" @@ -2066,8 +2065,8 @@ ConnectionImpl::ClientHttp2Options::ClientHttp2Options( #endif } -ExecutionContext* ConnectionImpl::executionContext() const { - return getConnectionExecutionContext(connection_); +OptRef ConnectionImpl::trackedStream() const { + return connection_.trackedStream(); } void ConnectionImpl::dumpState(std::ostream& os, int indent_level) const { diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index e7603e24d690..f93f33477348 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -179,7 +179,7 @@ class ConnectionImpl : public virtual Connection, } // ScopeTrackedObject - ExecutionContext* executionContext() const override; + OptRef trackedStream() const override; void dumpState(std::ostream& os, int indent_level) const override; protected: diff --git a/source/common/network/BUILD b/source/common/network/BUILD index 7eefab3fa379..bb9b174fff67 100644 --- a/source/common/network/BUILD +++ b/source/common/network/BUILD @@ -80,7 +80,6 @@ envoy_cc_library( srcs = ["connection_impl_base.cc"], hdrs = ["connection_impl_base.h"], deps = [ - ":common_connection_filter_states_lib", ":connection_socket_lib", ":filter_manager_lib", "//envoy/common:scope_tracker_interface", @@ -624,14 +623,3 @@ envoy_cc_library( "//envoy/network:filter_interface", ], ) - -envoy_cc_library( - name = "common_connection_filter_states_lib", - srcs = ["common_connection_filter_states.cc"], - hdrs = ["common_connection_filter_states.h"], - deps = [ - "//envoy/common:execution_context", - "//envoy/network:connection_interface", - "//envoy/stream_info:filter_state_interface", - ], -) diff --git a/source/common/network/common_connection_filter_states.cc b/source/common/network/common_connection_filter_states.cc deleted file mode 100644 index 6aef205ac184..000000000000 --- a/source/common/network/common_connection_filter_states.cc +++ /dev/null @@ -1,14 +0,0 @@ -#include "source/common/network/common_connection_filter_states.h" - -namespace Envoy { -namespace Network { - -ExecutionContext* getConnectionExecutionContext(const Network::Connection& connection) { - const ConnectionExecutionContextFilterState* filter_state = - connection.streamInfo().filterState().getDataReadOnly( - kConnectionExecutionContextFilterStateName); - return filter_state == nullptr ? nullptr : filter_state->executionContext(); -} - -} // namespace Network -} // namespace Envoy diff --git a/source/common/network/common_connection_filter_states.h b/source/common/network/common_connection_filter_states.h deleted file mode 100644 index 4de4e97647d1..000000000000 --- a/source/common/network/common_connection_filter_states.h +++ /dev/null @@ -1,34 +0,0 @@ -#pragma once - -#include "envoy/common/execution_context.h" -#include "envoy/network/connection.h" -#include "envoy/stream_info/filter_state.h" - -namespace Envoy { -namespace Network { - -static constexpr absl::string_view kConnectionExecutionContextFilterStateName = - "envoy.network.connection_execution_context"; - -// ConnectionExecutionContextFilterState is an optional connection-level filter state that goes by -// the name kConnectionExecutionContextFilterStateName. It owns a ExecutionContext, whose -// activate/deactivate methods will be called when a thread starts/finishes running code on behalf -// of the corresponding connection. -class ConnectionExecutionContextFilterState : public Envoy::StreamInfo::FilterState::Object { -public: - // It is safe, although useless, to set execution_context to nullptr. - explicit ConnectionExecutionContextFilterState( - std::unique_ptr execution_context) - : execution_context_(std::move(execution_context)) {} - - ExecutionContext* executionContext() const { return execution_context_.get(); } - -private: - std::unique_ptr execution_context_; -}; - -// Returns the ExecutionContext of a connection, if any. Or nullptr if not found. -ExecutionContext* getConnectionExecutionContext(const Network::Connection& connection); - -} // namespace Network -} // namespace Envoy diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 39cc345382a9..606300fbc23e 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -872,6 +872,8 @@ void ConnectionImpl::flushWriteBuffer() { } } +OptRef ConnectionImpl::trackedStream() const { return streamInfo(); } + void ConnectionImpl::dumpState(std::ostream& os, int indent_level) const { const char* spaces = spacesForLevel(indent_level); os << spaces << "ConnectionImpl " << this << DUMP_MEMBER(connecting_) << DUMP_MEMBER(bind_error_) diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index a66ffeb1866c..1c8ba08f384d 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -148,6 +148,8 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback // ScopeTrackedObject void dumpState(std::ostream& os, int indent_level) const override; + OptRef trackedStream() const override; + DetectedCloseType detectedCloseType() const override { return detected_close_type_; } protected: diff --git a/source/common/network/connection_impl_base.cc b/source/common/network/connection_impl_base.cc index e0586db6a832..7c3647c2d44b 100644 --- a/source/common/network/connection_impl_base.cc +++ b/source/common/network/connection_impl_base.cc @@ -1,7 +1,5 @@ #include "source/common/network/connection_impl_base.h" -#include "source/common/network/common_connection_filter_states.h" - namespace Envoy { namespace Network { @@ -42,10 +40,6 @@ void ConnectionImplBase::setDelayedCloseTimeout(std::chrono::milliseconds timeou delayed_close_timeout_ = timeout; } -ExecutionContext* ConnectionImplBase::executionContext() const { - return getConnectionExecutionContext(*this); -} - void ConnectionImplBase::initializeDelayedCloseTimer() { const auto timeout = delayed_close_timeout_.count(); ASSERT(delayed_close_timer_ == nullptr && timeout > 0); diff --git a/source/common/network/connection_impl_base.h b/source/common/network/connection_impl_base.h index 4b0104adbecd..21ee7530a45d 100644 --- a/source/common/network/connection_impl_base.h +++ b/source/common/network/connection_impl_base.h @@ -29,8 +29,6 @@ class ConnectionImplBase : public FilterManagerConnection, void setConnectionStats(const ConnectionStats& stats) override; void setDelayedCloseTimeout(std::chrono::milliseconds timeout) override; - ExecutionContext* executionContext() const override; - protected: void initializeDelayedCloseTimer(); diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 7b28eebbb21b..ab913d4a0210 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -116,9 +116,6 @@ RUNTIME_GUARD(envoy_restart_features_use_fast_protobuf_hash); // Begin false flags. Most of them should come with a TODO to flip true. -// Execution context is optional and must be enabled explicitly. -// See https://github.com/envoyproxy/envoy/issues/32012. -FALSE_RUNTIME_GUARD(envoy_restart_features_enable_execution_context); // Sentinel and test flag. FALSE_RUNTIME_GUARD(envoy_reloadable_features_test_feature_false); // TODO(paul-r-gall) Make this enabled by default after additional soak time. diff --git a/test/common/common/BUILD b/test/common/common/BUILD index 96ad095dab9a..be2281c69cab 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -541,7 +541,13 @@ envoy_benchmark_test( envoy_cc_test( name = "execution_context_test", srcs = ["execution_context_test.cc"], + copts = [ + "-DENVOY_ENABLE_EXECUTION_CONTEXT", + ], deps = [ "//envoy/common:execution_context", + "//source/common/api:api_lib", + "//test/mocks:common_lib", + "//test/mocks/stream_info:stream_info_mocks", ], ) diff --git a/test/common/common/execution_context_test.cc b/test/common/common/execution_context_test.cc index 25d886314e1f..8aed2f00eaf8 100644 --- a/test/common/common/execution_context_test.cc +++ b/test/common/common/execution_context_test.cc @@ -1,5 +1,15 @@ +#include + #include "envoy/common/execution_context.h" +#include "source/common/api/api_impl.h" +#include "source/common/common/scope_tracker.h" + +#include "test/mocks/common.h" +#include "test/mocks/stream_info/mocks.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" #include "gtest/gtest.h" namespace Envoy { @@ -27,45 +37,104 @@ class TestExecutionContext : public ExecutionContext { int activation_generations_ = 0; }; -TEST(ExecutionContextTest, NullContext) { - ScopedExecutionContext scoped_context(nullptr); - EXPECT_TRUE(scoped_context.isNull()); +class ExecutionContextTest : public testing::Test { +public: + ExecutionContextTest() { + ON_CALL(tracked_object_, trackedStream()) + .WillByDefault(testing::Return(OptRef(stream_info_))); + } - ScopedExecutionContext scoped_context2; - EXPECT_TRUE(scoped_context2.isNull()); + void setWithoutContext() { + context_ = nullptr; + stream_info_.filter_state_ = std::make_shared( + StreamInfo::FilterState::LifeSpan::Connection); + } + void setWithContext() { + context_ = std::make_shared(); + stream_info_.filter_state_ = std::make_shared( + StreamInfo::FilterState::LifeSpan::Connection); + stream_info_.filter_state_->setData(kConnectionExecutionContextFilterStateName, context_, + StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::LifeSpan::Connection); + } + + testing::NiceMock stream_info_; + testing::NiceMock tracked_object_; + std::shared_ptr context_{}; +}; + +TEST_F(ExecutionContextTest, NullContext) { + { + ScopedExecutionContext scoped_context(nullptr); + EXPECT_TRUE(scoped_context.isNull()); + } + { + ScopedExecutionContext scoped_context; + EXPECT_TRUE(scoped_context.isNull()); + } + { + setWithoutContext(); + ScopedExecutionContext scoped_context(&tracked_object_); + EXPECT_TRUE(scoped_context.isNull()); + } } -TEST(ExecutionContextTest, NestedScopes) { - TestExecutionContext context; - EXPECT_EQ(context.activationDepth(), 0); - EXPECT_EQ(context.activationGenerations(), 0); +TEST_F(ExecutionContextTest, NestedScopes) { + setWithContext(); + + EXPECT_EQ(context_->activationDepth(), 0); + EXPECT_EQ(context_->activationGenerations(), 0); { - ScopedExecutionContext scoped_context(&context); - EXPECT_EQ(context.activationDepth(), 1); - EXPECT_EQ(context.activationGenerations(), 1); + ScopedExecutionContext scoped_context(&tracked_object_); + EXPECT_EQ(context_->activationDepth(), 1); + EXPECT_EQ(context_->activationGenerations(), 1); { - ScopedExecutionContext nested_scoped_context(&context); - EXPECT_EQ(context.activationDepth(), 2); - EXPECT_EQ(context.activationGenerations(), 1); + ScopedExecutionContext nested_scoped_context(&tracked_object_); + EXPECT_EQ(context_->activationDepth(), 2); + EXPECT_EQ(context_->activationGenerations(), 1); } - EXPECT_EQ(context.activationDepth(), 1); - EXPECT_EQ(context.activationGenerations(), 1); + EXPECT_EQ(context_->activationDepth(), 1); + EXPECT_EQ(context_->activationGenerations(), 1); } - EXPECT_EQ(context.activationDepth(), 0); - EXPECT_EQ(context.activationGenerations(), 1); + EXPECT_EQ(context_->activationDepth(), 0); + EXPECT_EQ(context_->activationGenerations(), 1); } -TEST(ExecutionContextTest, DisjointScopes) { - TestExecutionContext context; +TEST_F(ExecutionContextTest, DisjointScopes) { + setWithContext(); for (int i = 1; i < 5; i++) { - ScopedExecutionContext scoped_context(&context); - EXPECT_EQ(context.activationDepth(), 1); - EXPECT_EQ(context.activationGenerations(), i); + ScopedExecutionContext scoped_context(&tracked_object_); + EXPECT_EQ(context_->activationDepth(), 1); + EXPECT_EQ(context_->activationGenerations(), i); + } + + EXPECT_EQ(context_->activationDepth(), 0); +} + +TEST_F(ExecutionContextTest, InScopeTrackerScopeState) { + + Api::ApiPtr api(Api::createApiForTest()); + Event::DispatcherPtr dispatcher(api->allocateDispatcher("test_thread")); + EXPECT_CALL(tracked_object_, trackedStream()) + .Times(2) + .WillRepeatedly(testing::Return(OptRef(stream_info_))); + + setWithContext(); + EXPECT_EQ(context_->activationDepth(), 0); + EXPECT_EQ(context_->activationGenerations(), 0); + { + ScopeTrackerScopeState scope(&tracked_object_, *dispatcher); + EXPECT_EQ(context_->activationDepth(), 1); + EXPECT_EQ(context_->activationGenerations(), 1); } - EXPECT_EQ(context.activationDepth(), 0); + EXPECT_EQ(context_->activationDepth(), 0); + EXPECT_EQ(context_->activationGenerations(), 1); + + setWithoutContext(); + { ScopeTrackerScopeState scope(&tracked_object_, *dispatcher); } } } // namespace Envoy diff --git a/test/common/common/scope_tracker_test.cc b/test/common/common/scope_tracker_test.cc index 7d928b18019e..76722b08de81 100644 --- a/test/common/common/scope_tracker_test.cc +++ b/test/common/common/scope_tracker_test.cc @@ -14,15 +14,7 @@ namespace Envoy { using testing::_; -class ScopeTrackerScopeStateTest : public testing::Test { -protected: - void setExecutionContextEnabled(bool enabled) { - ScopeTrackerScopeState::executionContextEnabled() = enabled; - } -}; - -TEST_F(ScopeTrackerScopeStateTest, ShouldManageTrackedObjectOnDispatcherStack) { - setExecutionContextEnabled(false); +TEST(ScopeTrackerScopeStateTest, ShouldManageTrackedObjectOnDispatcherStack) { Api::ApiPtr api(Api::createApiForTest()); Event::DispatcherPtr dispatcher(api->allocateDispatcher("test_thread")); MockScopeTrackedObject tracked_object; @@ -40,13 +32,4 @@ TEST_F(ScopeTrackerScopeStateTest, ShouldManageTrackedObjectOnDispatcherStack) { static_cast(dispatcher.get())->onFatalError(std::cerr); } -TEST_F(ScopeTrackerScopeStateTest, ExecutionContextEnabled) { - setExecutionContextEnabled(true); - Api::ApiPtr api(Api::createApiForTest()); - Event::DispatcherPtr dispatcher(api->allocateDispatcher("test_thread")); - MockScopeTrackedObject tracked_object; - EXPECT_CALL(tracked_object, executionContext()); - ScopeTrackerScopeState scope(&tracked_object, *dispatcher); -} - } // namespace Envoy diff --git a/test/common/network/BUILD b/test/common/network/BUILD index 036c23c15dcf..4c39e1fa3621 100644 --- a/test/common/network/BUILD +++ b/test/common/network/BUILD @@ -78,7 +78,6 @@ envoy_cc_test( "//source/common/common:empty_string", "//source/common/event:dispatcher_includes", "//source/common/event:dispatcher_lib", - "//source/common/network:common_connection_filter_states_lib", "//source/common/network:connection_lib", "//source/common/network:listen_socket_lib", "//source/common/network:utility_lib", diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 11c769ead5a7..f5cc602f07a6 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -15,7 +15,6 @@ #include "source/common/common/utility.h" #include "source/common/event/dispatcher_impl.h" #include "source/common/network/address_impl.h" -#include "source/common/network/common_connection_filter_states.h" #include "source/common/network/connection_impl.h" #include "source/common/network/io_socket_handle_impl.h" #include "source/common/network/listen_socket_impl.h" @@ -68,16 +67,6 @@ class MockInternalListenerManager : public InternalListenerManager { MOCK_METHOD(InternalListenerOptRef, findByAddress, (const Address::InstanceConstSharedPtr&), ()); }; -class NoopConnectionExecutionContext : public ExecutionContext { -public: - NoopConnectionExecutionContext() = default; - ~NoopConnectionExecutionContext() override = default; - -protected: - void activate() override {} - void deactivate() override {} -}; - TEST(RawBufferSocket, TestBasics) { TransportSocketPtr raw_buffer_socket(Network::Test::createRawBufferSocket()); EXPECT_FALSE(raw_buffer_socket->ssl()); @@ -348,27 +337,6 @@ TEST_P(ConnectionImplTest, SetSslConnection) { disconnect(false); } -TEST_P(ConnectionImplTest, SetGetExecutionContextFilterState) { - setUpBasicConnection(); - connect(); - - EXPECT_EQ(getConnectionExecutionContext(*client_connection_), nullptr); - - const StreamInfo::FilterStateSharedPtr& filter_state = - client_connection_->streamInfo().filterState(); - auto connection_execution_context = std::make_unique(); - const NoopConnectionExecutionContext* context_pointer = - connection_execution_context.get(); // Not owned. - auto filter_state_object = std::make_shared( - std::move(connection_execution_context)); - filter_state->setData(kConnectionExecutionContextFilterStateName, filter_state_object, - StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::LifeSpan::Connection); - - EXPECT_EQ(getConnectionExecutionContext(*client_connection_), context_pointer); - disconnect(true); -} - TEST_P(ConnectionImplTest, GetCongestionWindow) { setUpBasicConnection(); connect(); diff --git a/test/mocks/common.h b/test/mocks/common.h index 246aec0f0068..d5ee5c66078d 100644 --- a/test/mocks/common.h +++ b/test/mocks/common.h @@ -103,7 +103,7 @@ inline bool operator==(const StringViewSaver& saver, const char* str) { class MockScopeTrackedObject : public ScopeTrackedObject { public: MOCK_METHOD(void, dumpState, (std::ostream&, int), (const)); - MOCK_METHOD(ExecutionContext*, executionContext, (), (const)); + MOCK_METHOD(OptRef, trackedStream, (), (const)); }; namespace ConnectionPool { diff --git a/test/mocks/network/connection.h b/test/mocks/network/connection.h index 9c8b7bcbfe44..ba776f5f35c9 100644 --- a/test/mocks/network/connection.h +++ b/test/mocks/network/connection.h @@ -96,7 +96,7 @@ class MockConnectionBase { (uint64_t bandwidth_bits_per_sec, std::chrono::microseconds rtt), ()); \ MOCK_METHOD(absl::optional, congestionWindowInBytes, (), (const)); \ MOCK_METHOD(void, dumpState, (std::ostream&, int), (const)); \ - MOCK_METHOD(ExecutionContext*, executionContext, (), (const)); + MOCK_METHOD(OptRef, trackedStream, (), (const)); class MockConnection : public Connection, public MockConnectionBase { public: diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 483ba8404f28..7174b21667f6 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -411,7 +411,6 @@ class MockConnectionSocket : public ConnectionSocket { MOCK_METHOD(absl::optional, lastRoundTripTime, ()); MOCK_METHOD(absl::optional, congestionWindowInBytes, (), (const)); MOCK_METHOD(void, dumpState, (std::ostream&, int), (const)); - MOCK_METHOD(ExecutionContext*, executionContext, (), (const)); IoHandlePtr io_handle_; std::shared_ptr connection_info_provider_;