From 1bd6d9854f78805eb7005bb83b84a80d19dee41e Mon Sep 17 00:00:00 2001 From: Kevin Baichoo Date: Tue, 22 Dec 2020 18:48:47 +0000 Subject: [PATCH 1/4] Added ScopeTrackedObject to Network::Connection so we can dump L4 information on crashes during network level processing. Signed-off-by: Kevin Baichoo --- include/envoy/common/BUILD | 5 +++ include/envoy/common/dumpable.h | 28 ++++++++++++ include/envoy/common/scope_tracker.h | 11 +++-- include/envoy/network/BUILD | 1 + include/envoy/network/listen_socket.h | 3 +- source/common/common/dump_state_utils.h | 17 +++++++- source/common/network/BUILD | 3 ++ source/common/network/connection_impl.cc | 24 +++++++++++ source/common/network/connection_impl.h | 11 ++++- source/common/network/listen_socket_impl.h | 10 +++++ test/common/network/connection_impl_test.cc | 47 +++++++++++++++++++++ test/mocks/network/mocks.h | 2 + test/server/filter_chain_benchmark_test.cc | 2 + 13 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 include/envoy/common/dumpable.h diff --git a/include/envoy/common/BUILD b/include/envoy/common/BUILD index d120b7c0cc54..dd0cf42f432a 100644 --- a/include/envoy/common/BUILD +++ b/include/envoy/common/BUILD @@ -90,3 +90,8 @@ envoy_cc_library( name = "scope_tracker_interface", hdrs = ["scope_tracker.h"], ) + +envoy_cc_library( + name = "dumpable_interface", + hdrs = ["dumpable.h"], +) diff --git a/include/envoy/common/dumpable.h b/include/envoy/common/dumpable.h new file mode 100644 index 000000000000..04ace2a6643b --- /dev/null +++ b/include/envoy/common/dumpable.h @@ -0,0 +1,28 @@ +#pragma once + +#include + +#include "envoy/common/pure.h" + +namespace Envoy { + +/* + * Interface for classes that can dump their state. + * This is similar to ScopeTrackedObject interface, but cannot be registered + * for tracking work. + */ +class Dumpable { +public: + virtual ~Dumpable() = default; + /** + * 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; +}; + +} // namespace Envoy diff --git a/include/envoy/common/scope_tracker.h b/include/envoy/common/scope_tracker.h index eb72edf3b7af..4c9d8fbc4219 100644 --- a/include/envoy/common/scope_tracker.h +++ b/include/envoy/common/scope_tracker.h @@ -7,16 +7,19 @@ namespace Envoy { /* - * A class for tracking the scope of work. - * Currently this is only used for best-effort tracking the L7 stream doing - * work if a fatal error occurs. + * 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. + * + * Currently this is only used for the L4 network connection and L7 stream. */ class ScopeTrackedObject { public: virtual ~ScopeTrackedObject() = default; /** - * Dump debug state of the object in question to the provided ostream + * 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. * diff --git a/include/envoy/network/BUILD b/include/envoy/network/BUILD index b845c4d34d94..25ae3ce80f4f 100644 --- a/include/envoy/network/BUILD +++ b/include/envoy/network/BUILD @@ -130,6 +130,7 @@ envoy_cc_library( deps = [ ":io_handle_interface", ":socket_interface", + "//include/envoy/common:dumpable_interface", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/include/envoy/network/listen_socket.h b/include/envoy/network/listen_socket.h index e6f9cef2f20d..b73540b5c5d6 100644 --- a/include/envoy/network/listen_socket.h +++ b/include/envoy/network/listen_socket.h @@ -5,6 +5,7 @@ #include #include +#include "envoy/common/dumpable.h" #include "envoy/common/exception.h" #include "envoy/common/pure.h" #include "envoy/config/core/v3/base.pb.h" @@ -25,7 +26,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 { +class ConnectionSocket : public virtual Socket, public virtual Dumpable { public: ~ConnectionSocket() override = default; diff --git a/source/common/common/dump_state_utils.h b/source/common/common/dump_state_utils.h index 704ea271a5c4..12701eac47b6 100644 --- a/source/common/common/dump_state_utils.h +++ b/source/common/common/dump_state_utils.h @@ -9,11 +9,26 @@ namespace Envoy { // i.e. under the Envoy signal handler if encountering a crash due to OOM, where allocating more // memory would likely lead to the crash handler itself causing a subsequent OOM. -#define DUMP_MEMBER(member) ", " #member ": " << (member) +#define _DUMP_MEMBER(member) ", " #member ": " << (member) +#define _DUMP_MEMBER_VIA_VALUE(member, value) ", " #member ": " << (value) +#define _DUMP_MEMBER_SELECTOR(_1, _2, DUMP_MACRO, ...) DUMP_MACRO + +// This is a workaround for fact that MSVC expands __VA_ARGS__ after passing them into a macro, +// rather than before passing them into a macro. Without this, +// _DUMP_MEMBER_SELECTOR does not work correctly when compiled with MSVC. +#define EXPAND(X) X + +// If DUMP_MEMBER is called with one argument, then _DUMP_MEMBER is called. +// If DUMP_MEMBER is called with two arguments, then _DUMP_MEMBER_VIA_VALUE is called. +#define DUMP_MEMBER(...) \ + EXPAND(_DUMP_MEMBER_SELECTOR(__VA_ARGS__, _DUMP_MEMBER_VIA_VALUE, _DUMP_MEMBER)(__VA_ARGS__)) #define DUMP_OPTIONAL_MEMBER(member) \ ", " #member ": " << ((member).has_value() ? absl::StrCat((member).value()) : "null") +#define DUMP_NULLABLE_MEMBER(member, value) \ + ", " #member ": " << ((member) != nullptr ? value : "null") + // Macro assumes local member variables // os (ostream) // indent_level (int) diff --git a/source/common/network/BUILD b/source/common/network/BUILD index ec11aa935263..f9c4266e8ddd 100644 --- a/source/common/network/BUILD +++ b/source/common/network/BUILD @@ -60,8 +60,10 @@ envoy_cc_library( hdrs = ["connection_impl_base.h"], deps = [ ":filter_manager_lib", + "//include/envoy/common:scope_tracker_interface", "//include/envoy/event:dispatcher_interface", "//source/common/common:assert_lib", + "//source/common/common:dump_state_utils", ], ) @@ -246,6 +248,7 @@ envoy_cc_library( "//include/envoy/network:exception_interface", "//include/envoy/network:listen_socket_interface", "//source/common/common:assert_lib", + "//source/common/common:dump_state_utils", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 53c79cefdd63..b30034d48a50 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -12,8 +12,10 @@ #include "envoy/network/socket.h" #include "common/common/assert.h" +#include "common/common/dump_state_utils.h" #include "common/common/empty_string.h" #include "common/common/enum_to_int.h" +#include "common/common/scope_tracker.h" #include "common/network/address_impl.h" #include "common/network/listen_socket_impl.h" #include "common/network/raw_buffer_socket.h" @@ -530,6 +532,7 @@ void ConnectionImpl::onWriteBufferHighWatermark() { } void ConnectionImpl::onFileEvent(uint32_t events) { + ScopeTrackerScopeState scope(this, this->dispatcher_); ENVOY_CONN_LOG(trace, "socket event: {}", *this, events); if (immediate_error_event_ != ConnectionEvent::Connected) { @@ -753,6 +756,27 @@ void ConnectionImpl::flushWriteBuffer() { } } +std::ostream& operator<<(std::ostream& os, Connection::State connection_state) { + switch (connection_state) { + case Connection::State::Open: + return os << "Open"; + case Connection::State::Closing: + return os << "Closing"; + case Connection::State::Closed: + return os << "Closed"; + } + return os; +} + +// ScopeTrackedObject +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_) + << ", state() :" << state() << DUMP_MEMBER(read_buffer_limit_) << "\n"; + + DUMP_DETAILS(socket_); +} + ServerConnectionImpl::ServerConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket, TransportSocketPtr&& transport_socket, diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index c7b630c4dec7..bb317d720741 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -6,6 +6,7 @@ #include #include +#include "envoy/common/scope_tracker.h" #include "envoy/network/transport_socket.h" #include "common/buffer/watermark_buffer.h" @@ -41,9 +42,12 @@ class ConnectionImplUtility { }; /** - * Implementation of Network::Connection and Network::FilterManagerConnection. + * Implementation of Network::Connection, Network::FilterManagerConnection and + * Envoy::ScopeTrackedObject. */ -class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallbacks { +class ConnectionImpl : public ConnectionImplBase, + public TransportSocketCallbacks, + public ScopeTrackedObject { public: ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket, TransportSocketPtr&& transport_socket, StreamInfo::StreamInfo& stream_info, @@ -126,6 +130,9 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback // Obtain global next connection ID. This should only be used in tests. static uint64_t nextGlobalIdForTest() { return next_global_id_; } + // ScopeTrackedObject + void dumpState(std::ostream& os, int indent_level) const override; + protected: // A convenience function which returns true if // 1) The read disable count is zero or diff --git a/source/common/network/listen_socket_impl.h b/source/common/network/listen_socket_impl.h index 4f2b92f40f1e..9136953fd8e9 100644 --- a/source/common/network/listen_socket_impl.h +++ b/source/common/network/listen_socket_impl.h @@ -11,6 +11,7 @@ #include "envoy/network/socket_interface.h" #include "common/common/assert.h" +#include "common/common/dump_state_utils.h" #include "common/network/socket_impl.h" namespace Envoy { @@ -137,6 +138,15 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket { return ioHandle().lastRoundTripTime(); } + void dumpState(std::ostream& os, int indent_level) const override { + const char* spaces = spacesForLevel(indent_level); + os << spaces << "ListenSocketImpl " << this + << DUMP_NULLABLE_MEMBER(remote_address_, remote_address_->asStringView()) + << DUMP_NULLABLE_MEMBER(direct_remote_address_, direct_remote_address_->asStringView()) + << DUMP_NULLABLE_MEMBER(local_address_, local_address_->asStringView()) + << DUMP_MEMBER(transport_protocol_) << DUMP_MEMBER(server_name_) << "\n"; + } + protected: Address::InstanceConstSharedPtr remote_address_; const Address::InstanceConstSharedPtr direct_remote_address_; diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 279e2f78d9ab..b3a86927c08b 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -4,6 +4,7 @@ #include "envoy/common/platform.h" #include "envoy/config/core/v3/base.pb.h" +#include "envoy/network/address.h" #include "common/api/os_sys_calls_impl.h" #include "common/buffer/buffer_impl.h" @@ -1851,6 +1852,50 @@ TEST_P(ConnectionImplTest, DelayedCloseTimeoutNullStats) { server_connection->close(ConnectionCloseType::NoFlush); } +// Test DumpState methods. +TEST_P(ConnectionImplTest, NetworkSocketDumpsWithoutAllocatingMemory) { + IoHandlePtr io_handle = std::make_unique(0); + Address::InstanceConstSharedPtr server_addr; + Address::InstanceConstSharedPtr local_addr; + if (GetParam() == Network::Address::IpVersion::v4) { + server_addr = Network::Address::InstanceConstSharedPtr{ + new Network::Address::Ipv4Instance("1.1.1.1", 80, nullptr)}; + local_addr = Network::Address::InstanceConstSharedPtr{ + new Network::Address::Ipv4Instance("1.2.3.4", 56789, nullptr)}; + } else { + server_addr = Network::Address::InstanceConstSharedPtr{ + new Network::Address::Ipv6Instance("::1", 80, nullptr)}; + local_addr = Network::Address::InstanceConstSharedPtr{ + new Network::Address::Ipv6Instance("::1:2:3:4", 56789, nullptr)}; + } + + auto connection_socket = + std::make_unique(std::move(io_handle), local_addr, server_addr); + connection_socket->setRequestedServerName("envoyproxy.io"); + + // Start measuring memory and dump state. + Stats::TestUtil::MemoryTest memory_test; + connection_socket->dumpState(std::cout, 0); + EXPECT_EQ(memory_test.consumedBytes(), 0); +} + +TEST_P(ConnectionImplTest, NetworkConnectionDumpsWithoutAllocatingMemory) { + ConnectionMocks mocks = createConnectionMocks(false); + IoHandlePtr io_handle = std::make_unique(0); + + auto server_connection = std::make_unique( + *mocks.dispatcher_, + std::make_unique(std::move(io_handle), nullptr, nullptr), + std::move(mocks.transport_socket_), stream_info_, true); + + // Start measuring memory and dump state. + Stats::TestUtil::MemoryTest memory_test; + server_connection->dumpState(std::cout, 0); + EXPECT_EQ(memory_test.consumedBytes(), 0); + + server_connection->close(ConnectionCloseType::NoFlush); +} + class FakeReadFilter : public Network::ReadFilter { public: FakeReadFilter() = default; @@ -1898,6 +1943,8 @@ class MockTransportConnectionImplTest : public testing::Test { dispatcher_, std::make_unique(std::move(io_handle), nullptr, nullptr), TransportSocketPtr(transport_socket_), stream_info_, true); connection_->addConnectionCallbacks(callbacks_); + // File events will trigger setTrackedObject on the dispatcher. + EXPECT_CALL(dispatcher_, setTrackedObject(_)).WillRepeatedly(Return(nullptr)); } ~MockTransportConnectionImplTest() override { connection_->close(ConnectionCloseType::NoFlush); } diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 27f9782cdfaa..7f9c779f372a 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -305,6 +306,7 @@ class MockConnectionSocket : public ConnectionSocket { MOCK_METHOD(Api::SysCallIntResult, getSocketOption, (int, int, void*, socklen_t*), (const)); MOCK_METHOD(Api::SysCallIntResult, setBlockingForTest, (bool)); MOCK_METHOD(absl::optional, lastRoundTripTime, ()); + MOCK_METHOD(void, dumpState, (std::ostream&, int), (const)); IoHandlePtr io_handle_; Address::InstanceConstSharedPtr local_address_; diff --git a/test/server/filter_chain_benchmark_test.cc b/test/server/filter_chain_benchmark_test.cc index 45b422e9f7f8..cab4612c6869 100644 --- a/test/server/filter_chain_benchmark_test.cc +++ b/test/server/filter_chain_benchmark_test.cc @@ -1,4 +1,5 @@ #include +#include #include "envoy/config/listener/v3/listener.pb.h" #include "envoy/config/listener/v3/listener_components.pb.h" @@ -118,6 +119,7 @@ class MockConnectionSocket : public Network::ConnectionSocket { } Api::SysCallIntResult setBlockingForTest(bool) override { return {0, 0}; } absl::optional lastRoundTripTime() override { return {}; } + void dumpState(std::ostream&, int) const override {} private: Network::IoHandlePtr io_handle_; From 60dc8a69ac2fcf7d3aa407875b1015b421e33074 Mon Sep 17 00:00:00 2001 From: Kevin Baichoo Date: Wed, 23 Dec 2020 15:45:44 +0000 Subject: [PATCH 2/4] Minor cleanup. Signed-off-by: Kevin Baichoo --- source/common/common/dump_state_utils.h | 14 +--------- source/common/network/connection_impl.cc | 2 +- test/common/network/connection_impl_test.cc | 31 +++++++++++++++++++-- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/source/common/common/dump_state_utils.h b/source/common/common/dump_state_utils.h index 12701eac47b6..9fbb8b43a175 100644 --- a/source/common/common/dump_state_utils.h +++ b/source/common/common/dump_state_utils.h @@ -9,19 +9,7 @@ namespace Envoy { // i.e. under the Envoy signal handler if encountering a crash due to OOM, where allocating more // memory would likely lead to the crash handler itself causing a subsequent OOM. -#define _DUMP_MEMBER(member) ", " #member ": " << (member) -#define _DUMP_MEMBER_VIA_VALUE(member, value) ", " #member ": " << (value) -#define _DUMP_MEMBER_SELECTOR(_1, _2, DUMP_MACRO, ...) DUMP_MACRO - -// This is a workaround for fact that MSVC expands __VA_ARGS__ after passing them into a macro, -// rather than before passing them into a macro. Without this, -// _DUMP_MEMBER_SELECTOR does not work correctly when compiled with MSVC. -#define EXPAND(X) X - -// If DUMP_MEMBER is called with one argument, then _DUMP_MEMBER is called. -// If DUMP_MEMBER is called with two arguments, then _DUMP_MEMBER_VIA_VALUE is called. -#define DUMP_MEMBER(...) \ - EXPAND(_DUMP_MEMBER_SELECTOR(__VA_ARGS__, _DUMP_MEMBER_VIA_VALUE, _DUMP_MEMBER)(__VA_ARGS__)) +#define DUMP_MEMBER(member) ", " #member ": " << (member) #define DUMP_OPTIONAL_MEMBER(member) \ ", " #member ": " << ((member).has_value() ? absl::StrCat((member).value()) : "null") diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index b30034d48a50..78dc3db9ed78 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -772,7 +772,7 @@ std::ostream& operator<<(std::ostream& os, Connection::State connection_state) { 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_) - << ", state() :" << state() << DUMP_MEMBER(read_buffer_limit_) << "\n"; + << DUMP_MEMBER(state()) << DUMP_MEMBER(read_buffer_limit_) << "\n"; DUMP_DETAILS(socket_); } diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index b3a86927c08b..755018f35f9c 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -10,6 +10,7 @@ #include "common/buffer/buffer_impl.h" #include "common/common/empty_string.h" #include "common/common/fmt.h" +#include "common/common/utility.h" #include "common/event/dispatcher_impl.h" #include "common/network/address_impl.h" #include "common/network/connection_impl.h" @@ -1854,6 +1855,8 @@ TEST_P(ConnectionImplTest, DelayedCloseTimeoutNullStats) { // Test DumpState methods. TEST_P(ConnectionImplTest, NetworkSocketDumpsWithoutAllocatingMemory) { + std::array buffer; + OutputBufferStream ostream{buffer.data(), buffer.size()}; IoHandlePtr io_handle = std::make_unique(0); Address::InstanceConstSharedPtr server_addr; Address::InstanceConstSharedPtr local_addr; @@ -1875,11 +1878,28 @@ TEST_P(ConnectionImplTest, NetworkSocketDumpsWithoutAllocatingMemory) { // Start measuring memory and dump state. Stats::TestUtil::MemoryTest memory_test; - connection_socket->dumpState(std::cout, 0); + connection_socket->dumpState(ostream, 0); EXPECT_EQ(memory_test.consumedBytes(), 0); + + // Check socket dump + EXPECT_THAT(ostream.contents(), HasSubstr("ListenSocketImpl")); + if (GetParam() == Network::Address::IpVersion::v4) { + EXPECT_THAT( + ostream.contents(), + HasSubstr( + "remote_address_: 1.1.1.1:80, direct_remote_address_: 1.1.1.1:80, local_address_: " + "1.2.3.4:56789, transport_protocol_: , server_name_: envoyproxy.io")); + } else { + EXPECT_THAT( + ostream.contents(), + HasSubstr("remote_address_: [::1]:80, direct_remote_address_: [::1]:80, local_address_: " + "[::1:2:3:4]:56789, transport_protocol_: , server_name_: envoyproxy.io")); + } } TEST_P(ConnectionImplTest, NetworkConnectionDumpsWithoutAllocatingMemory) { + std::array buffer; + OutputBufferStream ostream{buffer.data(), buffer.size()}; ConnectionMocks mocks = createConnectionMocks(false); IoHandlePtr io_handle = std::make_unique(0); @@ -1890,9 +1910,16 @@ TEST_P(ConnectionImplTest, NetworkConnectionDumpsWithoutAllocatingMemory) { // Start measuring memory and dump state. Stats::TestUtil::MemoryTest memory_test; - server_connection->dumpState(std::cout, 0); + server_connection->dumpState(ostream, 0); EXPECT_EQ(memory_test.consumedBytes(), 0); + // Check connection data + EXPECT_THAT(ostream.contents(), HasSubstr("ConnectionImpl")); + EXPECT_THAT(ostream.contents(), + HasSubstr("connecting_: 0, bind_error_: 0, state(): Open, read_buffer_limit_: 0")); + // Check socket starts dumping + EXPECT_THAT(ostream.contents(), HasSubstr("ListenSocketImpl")); + server_connection->close(ConnectionCloseType::NoFlush); } From 056c07a520aad247deddabdbc7730b71a3f71003 Mon Sep 17 00:00:00 2001 From: Kevin Baichoo Date: Tue, 5 Jan 2021 20:11:38 +0000 Subject: [PATCH 3/4] Minor changes. Signed-off-by: Kevin Baichoo --- docs/root/version_history/current.rst | 1 + include/envoy/common/BUILD | 5 ----- include/envoy/common/dumpable.h | 28 ------------------------ include/envoy/network/BUILD | 2 +- include/envoy/network/listen_socket.h | 4 ++-- source/common/network/connection_impl.cc | 25 ++++++++++----------- 6 files changed, 16 insertions(+), 49 deletions(-) delete mode 100644 include/envoy/common/dumpable.h diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 19fb421f6bc9..a2f0c57526cb 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -68,6 +68,7 @@ New Features * compression: the :ref:`compressor ` filter adds support for compressing request payloads. Its configuration is unified with the :ref:`decompressor ` filter with two new fields for different directions - :ref:`requests ` and :ref:`responses `. The latter deprecates the old response-specific fields and, if used, roots the response-specific stats in `.compressor...response.*` instead of `.compressor...*`. * config: added ability to flush stats when the admin's :ref:`/stats endpoint ` is hit instead of on a timer via :ref:`stats_flush_on_admin `. * config: added new runtime feature `envoy.features.enable_all_deprecated_features` that allows the use of all deprecated features. +* crash support: added the ability to dump L4 data on crash. * formatter: added new :ref:`text_format_source ` field to support format strings both inline and from a file. * grpc: implemented header value syntax support when defining :ref:`initial metadata ` for gRPC-based `ext_authz` :ref:`HTTP ` and :ref:`network ` filters, and :ref:`ratelimit ` filters. * grpc-json: added support for configuring :ref:`unescaping behavior ` for path components. diff --git a/include/envoy/common/BUILD b/include/envoy/common/BUILD index dd0cf42f432a..d120b7c0cc54 100644 --- a/include/envoy/common/BUILD +++ b/include/envoy/common/BUILD @@ -90,8 +90,3 @@ envoy_cc_library( name = "scope_tracker_interface", hdrs = ["scope_tracker.h"], ) - -envoy_cc_library( - name = "dumpable_interface", - hdrs = ["dumpable.h"], -) diff --git a/include/envoy/common/dumpable.h b/include/envoy/common/dumpable.h deleted file mode 100644 index 04ace2a6643b..000000000000 --- a/include/envoy/common/dumpable.h +++ /dev/null @@ -1,28 +0,0 @@ -#pragma once - -#include - -#include "envoy/common/pure.h" - -namespace Envoy { - -/* - * Interface for classes that can dump their state. - * This is similar to ScopeTrackedObject interface, but cannot be registered - * for tracking work. - */ -class Dumpable { -public: - virtual ~Dumpable() = default; - /** - * 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; -}; - -} // namespace Envoy diff --git a/include/envoy/network/BUILD b/include/envoy/network/BUILD index 25ae3ce80f4f..64cdece4ab9f 100644 --- a/include/envoy/network/BUILD +++ b/include/envoy/network/BUILD @@ -130,7 +130,7 @@ envoy_cc_library( deps = [ ":io_handle_interface", ":socket_interface", - "//include/envoy/common:dumpable_interface", + "//include/envoy/common:scope_tracker_interface", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/include/envoy/network/listen_socket.h b/include/envoy/network/listen_socket.h index b73540b5c5d6..b3cc169204e1 100644 --- a/include/envoy/network/listen_socket.h +++ b/include/envoy/network/listen_socket.h @@ -5,9 +5,9 @@ #include #include -#include "envoy/common/dumpable.h" #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 +26,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 Dumpable { +class ConnectionSocket : public virtual Socket, public virtual ScopeTrackedObject { public: ~ConnectionSocket() override = default; diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 78dc3db9ed78..3a4e1b2f5c45 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -28,8 +28,20 @@ namespace { constexpr absl::string_view kTransportSocketConnectTimeoutTerminationDetails = "transport socket timeout was reached"; +std::ostream& operator<<(std::ostream& os, Connection::State connection_state) { + switch (connection_state) { + case Connection::State::Open: + return os << "Open"; + case Connection::State::Closing: + return os << "Closing"; + case Connection::State::Closed: + return os << "Closed"; + } + return os; } +} // namespace + void ConnectionImplUtility::updateBufferStats(uint64_t delta, uint64_t new_total, uint64_t& previous_total, Stats::Counter& stat_total, Stats::Gauge& stat_current) { @@ -756,19 +768,6 @@ void ConnectionImpl::flushWriteBuffer() { } } -std::ostream& operator<<(std::ostream& os, Connection::State connection_state) { - switch (connection_state) { - case Connection::State::Open: - return os << "Open"; - case Connection::State::Closing: - return os << "Closing"; - case Connection::State::Closed: - return os << "Closed"; - } - return os; -} - -// ScopeTrackedObject 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_) From a85e8c20eca23fd73c58702c791edaee03dcf879 Mon Sep 17 00:00:00 2001 From: Kevin Baichoo Date: Wed, 6 Jan 2021 13:13:50 +0000 Subject: [PATCH 4/4] Clarified release notes. Signed-off-by: Kevin Baichoo --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index a2f0c57526cb..b8bfd101b09b 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -68,7 +68,7 @@ New Features * compression: the :ref:`compressor ` filter adds support for compressing request payloads. Its configuration is unified with the :ref:`decompressor ` filter with two new fields for different directions - :ref:`requests ` and :ref:`responses `. The latter deprecates the old response-specific fields and, if used, roots the response-specific stats in `.compressor...response.*` instead of `.compressor...*`. * config: added ability to flush stats when the admin's :ref:`/stats endpoint ` is hit instead of on a timer via :ref:`stats_flush_on_admin `. * config: added new runtime feature `envoy.features.enable_all_deprecated_features` that allows the use of all deprecated features. -* crash support: added the ability to dump L4 data on crash. +* crash support: added the ability to dump L4 connection data on crash. * formatter: added new :ref:`text_format_source ` field to support format strings both inline and from a file. * grpc: implemented header value syntax support when defining :ref:`initial metadata ` for gRPC-based `ext_authz` :ref:`HTTP ` and :ref:`network ` filters, and :ref:`ratelimit ` filters. * grpc-json: added support for configuring :ref:`unescaping behavior ` for path components.