diff --git a/envoy/network/socket.h b/envoy/network/socket.h index ed37195de7088..79940b26f9226 100644 --- a/envoy/network/socket.h +++ b/envoy/network/socket.h @@ -80,6 +80,11 @@ class SocketAddressProvider { */ virtual absl::string_view requestedServerName() const PURE; + /** + * @return Connection ID of the downstream connection, or unset if not available. + **/ + virtual absl::optional connectionID() const PURE; + /** * Dumps the state of the SocketAddressProvider to the given ostream. * @@ -121,6 +126,11 @@ class SocketAddressSetter : public SocketAddressProvider { * @param SNI value requested. */ virtual void setRequestedServerName(const absl::string_view requested_server_name) PURE; + + /** + * @param id Connection ID of the downstream connection. + **/ + virtual void setConnectionID(uint64_t id) PURE; }; using SocketAddressSetterSharedPtr = std::shared_ptr; diff --git a/envoy/stream_info/stream_info.h b/envoy/stream_info/stream_info.h index 002f0ce8bdf63..7a67ff946f180 100644 --- a/envoy/stream_info/stream_info.h +++ b/envoy/stream_info/stream_info.h @@ -584,16 +584,6 @@ class StreamInfo { */ virtual Tracing::Reason traceReason() const PURE; - /** - * @return Connection ID of the downstream connection, or unset if not available. - **/ - virtual absl::optional connectionID() const PURE; - - /** - * @param id Connection ID of the downstream connection. - **/ - virtual void setConnectionID(uint64_t id) PURE; - /** * @param filter_chain_name Network filter chain name of the downstream connection. */ diff --git a/source/common/formatter/substitution_formatter.cc b/source/common/formatter/substitution_formatter.cc index c8419263f367c..e1001871f038f 100644 --- a/source/common/formatter/substitution_formatter.cc +++ b/source/common/formatter/substitution_formatter.cc @@ -814,7 +814,7 @@ StreamInfoFormatter::StreamInfoFormatter(const std::string& field_name) { } else if (field_name == "CONNECTION_ID") { field_extractor_ = std::make_unique( [](const StreamInfo::StreamInfo& stream_info) { - return stream_info.connectionID().value_or(0); + return stream_info.downstreamAddressProvider().connectionID().value_or(0); }); } else if (field_name == "REQUESTED_SERVER_NAME") { field_extractor_ = std::make_unique( diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index e792b2735d269..e70d5f0c91089 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -651,9 +651,6 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect filter_manager_.streamInfo().setDownstreamSslConnection( connection_manager_.read_callbacks_->connection().ssl()); - filter_manager_.streamInfo().setConnectionID( - connection_manager_.read_callbacks_->connection().id()); - if (connection_manager_.config_.streamIdleTimeout().count()) { idle_timeout_ms_ = connection_manager_.config_.streamIdleTimeout(); stream_idle_timer_ = diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 008baa50bcd81..f8e3e1326e025 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -621,6 +621,9 @@ class OverridableRemoteSocketAddressSetterStreamInfo : public StreamInfo::Stream absl::string_view requestedServerName() const override { return StreamInfoImpl::downstreamAddressProvider().requestedServerName(); } + absl::optional connectionID() const override { + return StreamInfoImpl::downstreamAddressProvider().connectionID(); + } void dumpState(std::ostream& os, int indent_level) const override { StreamInfoImpl::dumpState(os, indent_level); diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index ec8748241e0c1..765df00afa73a 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -95,6 +95,10 @@ ConnectionImpl::ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPt Event::FileReadyType::Read | Event::FileReadyType::Write); transport_socket_->setTransportSocketCallbacks(*this); + + // TODO(soulxu): generate the connection id inside the addressProvider directly, + // then we don't need a setter or any of the optional stuff. + socket_->addressProvider().setConnectionID(id()); } ConnectionImpl::~ConnectionImpl() { diff --git a/source/common/network/socket_impl.h b/source/common/network/socket_impl.h index d277f536c3349..e04fae266b411 100644 --- a/source/common/network/socket_impl.h +++ b/source/common/network/socket_impl.h @@ -49,6 +49,8 @@ class SocketAddressSetterImpl : public SocketAddressSetter { void setRequestedServerName(const absl::string_view requested_server_name) override { server_name_ = std::string(requested_server_name); } + absl::optional connectionID() const override { return connection_id_; } + void setConnectionID(uint64_t id) override { connection_id_ = id; } private: Address::InstanceConstSharedPtr local_address_; @@ -56,6 +58,7 @@ class SocketAddressSetterImpl : public SocketAddressSetter { Address::InstanceConstSharedPtr remote_address_; Address::InstanceConstSharedPtr direct_remote_address_; std::string server_name_; + absl::optional connection_id_; }; class SocketImpl : public virtual Socket { diff --git a/source/common/stream_info/stream_info_impl.h b/source/common/stream_info/stream_info_impl.h index 1a41097e51420..53d4351749c10 100644 --- a/source/common/stream_info/stream_info_impl.h +++ b/source/common/stream_info/stream_info_impl.h @@ -274,10 +274,6 @@ struct StreamInfoImpl : public StreamInfo { return upstream_cluster_info_; } - void setConnectionID(uint64_t id) override { connection_id_ = id; } - - absl::optional connectionID() const override { return connection_id_; } - void setFilterChainName(absl::string_view filter_chain_name) override { filter_chain_name_ = std::string(filter_chain_name); } @@ -336,7 +332,6 @@ struct StreamInfoImpl : public StreamInfo { UpstreamTiming upstream_timing_; std::string upstream_transport_failure_reason_; absl::optional upstream_cluster_info_; - absl::optional connection_id_; std::string filter_chain_name_; Tracing::Reason trace_reason_; }; diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc index d073b3226494f..934f78728bcd4 100644 --- a/source/extensions/filters/common/expr/context.cc +++ b/source/extensions/filters/common/expr/context.cc @@ -186,7 +186,7 @@ absl::optional ConnectionWrapper::operator[](CelValue key) const { } else if (value == RequestedServerName) { return CelValue::CreateStringView(info_.downstreamAddressProvider().requestedServerName()); } else if (value == ID) { - auto id = info_.connectionID(); + auto id = info_.downstreamAddressProvider().connectionID(); if (id.has_value()) { return CelValue::CreateUint64(id.value()); } diff --git a/source/server/active_tcp_listener.cc b/source/server/active_tcp_listener.cc index a8a2120ba2a08..b798e302e4d67 100644 --- a/source/server/active_tcp_listener.cc +++ b/source/server/active_tcp_listener.cc @@ -409,7 +409,6 @@ ActiveTcpConnection::ActiveTcpConnection(ActiveConnections& active_connections, listener.stats_.downstream_cx_active_.inc(); listener.per_worker_stats_.downstream_cx_total_.inc(); listener.per_worker_stats_.downstream_cx_active_.inc(); - stream_info_->setConnectionID(connection_->id()); // Active connections on the handler (not listener). The per listener connections have already // been incremented at this point either via the connection balancer or in the socket accept diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index 72fdcafdfef38..00d0ce6a3a4d5 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -663,7 +663,7 @@ TEST(SubstitutionFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("CONNECTION_ID"); uint64_t id = 123; - EXPECT_CALL(stream_info, connectionID()).WillRepeatedly(Return(id)); + stream_info.downstream_address_provider_->setConnectionID(id); EXPECT_EQ("123", upstream_format.format(request_headers, response_headers, response_trailers, stream_info, body)); EXPECT_THAT(upstream_format.formatValue(request_headers, response_headers, response_trailers, diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 164f7d46f947d..db5762711336b 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -293,7 +293,6 @@ TEST_F(HttpConnectionManagerImplTest, 100ContinueResponseWithDecoderPause) { // When create new stream, the stream info will be populated from the connection. TEST_F(HttpConnectionManagerImplTest, PopulateStreamInfo) { setup(true, "", false); - EXPECT_CALL(filter_callbacks_.connection_, id()).WillRepeatedly(Return(1234)); // Set up the codec. Buffer::OwnedImpl fake_input("input"); @@ -303,7 +302,8 @@ TEST_F(HttpConnectionManagerImplTest, PopulateStreamInfo) { EXPECT_EQ(requestIDExtension().get(), decoder_->streamInfo().getRequestIDProvider()); EXPECT_EQ(ssl_connection_, decoder_->streamInfo().downstreamSslConnection()); - EXPECT_EQ(1234U, decoder_->streamInfo().connectionID()); + EXPECT_EQ(filter_callbacks_.connection_.id_, + decoder_->streamInfo().downstreamAddressProvider().connectionID()); EXPECT_EQ(server_name_, decoder_->streamInfo().downstreamAddressProvider().requestedServerName()); // Clean up. diff --git a/test/common/stream_info/stream_info_impl_test.cc b/test/common/stream_info/stream_info_impl_test.cc index 482490ca7874d..6ff19887f4fc7 100644 --- a/test/common/stream_info/stream_info_impl_test.cc +++ b/test/common/stream_info/stream_info_impl_test.cc @@ -252,14 +252,6 @@ TEST_F(StreamInfoImplTest, DefaultRequestIDExtensionTest) { EXPECT_EQ(nullptr, stream_info.getRequestIDProvider()); } -TEST_F(StreamInfoImplTest, ConnectionID) { - StreamInfoImpl stream_info(test_time_.timeSystem(), nullptr); - EXPECT_FALSE(stream_info.connectionID().has_value()); - uint64_t id = 123; - stream_info.setConnectionID(id); - EXPECT_EQ(id, stream_info.connectionID()); -} - TEST_F(StreamInfoImplTest, Details) { StreamInfoImpl stream_info(test_time_.timeSystem(), nullptr); EXPECT_FALSE(stream_info.responseCodeDetails().has_value()); diff --git a/test/common/stream_info/test_util.h b/test/common/stream_info/test_util.h index 39b95e2b76549..a4ab278e8db3e 100644 --- a/test/common/stream_info/test_util.h +++ b/test/common/stream_info/test_util.h @@ -213,10 +213,6 @@ class TestStreamInfo : public StreamInfo::StreamInfo { return upstream_cluster_info_; } - void setConnectionID(uint64_t id) override { connection_id_ = id; } - - absl::optional connectionID() const override { return connection_id_; } - void setFilterChainName(absl::string_view filter_chain_name) override { filter_chain_name_ = std::string(filter_chain_name); } diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index 79475ef353c70..f7b1931195e46 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -449,6 +449,7 @@ TEST(Context, ConnectionAttributes) { EXPECT_CALL(info, upstreamTransportFailureReason()) .WillRepeatedly(ReturnRef(upstream_transport_failure_reason)); EXPECT_CALL(info, connectionID()).WillRepeatedly(Return(123)); + info.downstream_address_provider_->setConnectionID(123); const absl::optional connection_termination_details = "unauthorized"; EXPECT_CALL(info, connectionTerminationDetails()) .WillRepeatedly(ReturnRef(connection_termination_details)); diff --git a/test/mocks/network/connection.cc b/test/mocks/network/connection.cc index 54b318559ee46..adcb9c971dd18 100644 --- a/test/mocks/network/connection.cc +++ b/test/mocks/network/connection.cc @@ -81,6 +81,7 @@ template static void initializeMockConnection(T& connection) { connection.raiseEvent(Network::ConnectionEvent::LocalClose); })); ON_CALL(connection, id()).WillByDefault(Return(connection.next_id_)); + connection.stream_info_.downstream_address_provider_->setConnectionID(connection.id_); ON_CALL(connection, state()).WillByDefault(ReturnPointee(&connection.state_)); // The real implementation will move the buffer data into the socket. diff --git a/test/mocks/stream_info/mocks.cc b/test/mocks/stream_info/mocks.cc index a2b7c768162d3..28194ab7ab700 100644 --- a/test/mocks/stream_info/mocks.cc +++ b/test/mocks/stream_info/mocks.cc @@ -118,7 +118,6 @@ MockStreamInfo::MockStreamInfo() ON_CALL(*this, getRouteName()).WillByDefault(ReturnRef(route_name_)); ON_CALL(*this, upstreamTransportFailureReason()) .WillByDefault(ReturnRef(upstream_transport_failure_reason_)); - ON_CALL(*this, connectionID()).WillByDefault(Return(connection_id_)); ON_CALL(*this, setConnectionID(_)).WillByDefault(Invoke([this](uint64_t id) { connection_id_ = id; }));