diff --git a/envoy/stream_info/stream_info.h b/envoy/stream_info/stream_info.h index 9836510f861a6..50c25e10fbec3 100644 --- a/envoy/stream_info/stream_info.h +++ b/envoy/stream_info/stream_info.h @@ -358,13 +358,6 @@ class UpstreamInfo { */ virtual Ssl::ConnectionInfoConstSharedPtr upstreamSslConnection() const PURE; - /** - * Sets the upstream timing information for this stream. This is useful for - * when multiple upstream requests are issued and we want to save timing - * information for the one that "wins". - */ - virtual void setUpstreamTiming(const UpstreamTiming& upstream_timing) PURE; - /* * @return the upstream timing for this stream * */ @@ -451,11 +444,6 @@ class StreamInfo { */ virtual bool intersectResponseFlags(uint64_t response_flags) const PURE; - /** - * @param host the selected upstream host for the request. - */ - virtual void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) PURE; - /** * @param std::string name denotes the name of the route. */ @@ -518,13 +506,6 @@ class StreamInfo { */ virtual absl::optional lastDownstreamRxByteReceived() const PURE; - /** - * Sets the upstream timing information for this stream. This is useful for - * when multiple upstream requests are issued and we want to save timing - * information for the one that "wins". - */ - virtual void setUpstreamTiming(const UpstreamTiming& upstream_timing) PURE; - /** * Sets the upstream information for this stream. */ @@ -627,13 +608,6 @@ class StreamInfo { */ virtual Upstream::HostDescriptionConstSharedPtr upstreamHost() const PURE; - /** - * @param upstream_local_address sets the local address of the upstream connection. Note that it - * can be different than the local address of the downstream connection. - */ - virtual void setUpstreamLocalAddress( - const Network::Address::InstanceConstSharedPtr& upstream_local_address) PURE; - /** * @return the upstream local address. */ @@ -654,12 +628,6 @@ class StreamInfo { */ virtual const Network::ConnectionInfoProvider& downstreamAddressProvider() const PURE; - /** - * @param connection_info sets the upstream ssl connection. - */ - virtual void - setUpstreamSslConnection(const Ssl::ConnectionInfoConstSharedPtr& ssl_connection_info) PURE; - /** * @return the upstream SSL connection. This will be nullptr if the upstream * connection does not use SSL. @@ -701,12 +669,6 @@ class StreamInfo { * @return pointer to filter state to be used by upstream connections. */ virtual const FilterStateSharedPtr& upstreamFilterState() const PURE; - virtual void setUpstreamFilterState(const FilterStateSharedPtr& filter_state) PURE; - - /** - * @param failure_reason the upstream transport failure reason. - */ - virtual void setUpstreamTransportFailureReason(absl::string_view failure_reason) PURE; /** * @return const std::string& the upstream transport failure reason, e.g. certificate validation @@ -768,11 +730,6 @@ class StreamInfo { */ virtual const std::string& filterChainName() const PURE; - /** - * @param connection ID of the upstream connection. - */ - virtual void setUpstreamConnectionId(uint64_t id) PURE; - /** * @return the ID of the upstream connection, or absl::nullopt if not available. */ diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 59336369ab11d..849e4ae31ba35 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -895,12 +895,6 @@ void Filter::onDestroy() { void Filter::onResponseTimeout() { ENVOY_STREAM_LOG(debug, "upstream timeout", *callbacks_); - // If we had an upstream request that got a "good" response, save its - // upstream timing information into the downstream stream info. - if (final_upstream_request_) { - callbacks_->streamInfo().setUpstreamTiming(final_upstream_request_->upstreamTiming()); - } - // Reset any upstream requests that are still in flight. while (!upstream_requests_.empty()) { UpstreamRequestPtr upstream_request = @@ -1164,7 +1158,6 @@ void Filter::onUpstreamReset(Http::StreamResetReason reset_reason, ? ", transport failure reason: " : "", transport_failure_reason); - callbacks_->streamInfo().setUpstreamTransportFailureReason(transport_failure_reason); const std::string& basic_details = downstream_response_started_ ? StreamInfo::ResponseCodeDetails::get().LateUpstreamReset : StreamInfo::ResponseCodeDetails::get().EarlyUpstreamReset; @@ -1407,14 +1400,8 @@ void Filter::onUpstreamHeaders(uint64_t response_code, Http::ResponseHeaderMapPt downstream_response_started_ = true; final_upstream_request_ = &upstream_request; - // In upstream request hedging scenarios the upstream connection ID set in onPoolReady might not - // be the connection ID of the upstream connection that ended up receiving upstream headers. Thus - // reset the upstream connection ID here with the ID of the connection that ultimately was the - // transport for the final upstream request. - if (final_upstream_request_->streamInfo().upstreamConnectionId().has_value()) { - callbacks_->streamInfo().setUpstreamConnectionId( - final_upstream_request_->streamInfo().upstreamConnectionId().value()); - } + // Make sure that for request hedging, we end up with the correct final upstream info. + callbacks_->streamInfo().setUpstreamInfo(final_upstream_request_->streamInfo().upstreamInfo()); resetOtherUpstreams(upstream_request); if (end_stream) { onUpstreamComplete(upstream_request); @@ -1471,8 +1458,6 @@ void Filter::onUpstreamComplete(UpstreamRequest& upstream_request) { if (!downstream_end_stream_) { upstream_request.resetStream(); } - callbacks_->streamInfo().setUpstreamTiming(final_upstream_request_->upstreamTiming()); - Event::Dispatcher& dispatcher = callbacks_->dispatcher(); std::chrono::milliseconds response_time = std::chrono::duration_cast( dispatcher.timeSource().monotonicTime() - downstream_request_complete_time_); diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 8b9baba46cfde..3640d6b0dbf2d 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -60,6 +60,8 @@ UpstreamRequest::UpstreamRequest(RouterFilterInterface& parent, span_->setTag(Tracing::Tags::get().RetryCount, std::to_string(parent.attemptCount() - 1)); } } + stream_info_.setUpstreamInfo(std::make_shared()); + parent_.callbacks()->streamInfo().setUpstreamInfo(stream_info_.upstreamInfo()); stream_info_.healthCheck(parent_.callbacks()->streamInfo().healthCheck()); absl::optional cluster_info = @@ -115,7 +117,6 @@ UpstreamRequest::~UpstreamRequest() { } } - stream_info_.setUpstreamTiming(upstream_timing_); stream_info_.onRequestComplete(); for (const auto& upstream_log : parent_.config().upstream_logs_) { upstream_log->log(parent_.downstreamHeaders(), upstream_headers_.get(), @@ -162,7 +163,7 @@ void UpstreamRequest::decodeHeaders(Http::ResponseHeaderMapPtr&& headers, bool e // TODO(rodaine): This is actually measuring after the headers are parsed and not the first // byte. - upstream_timing_.onFirstUpstreamRxByteReceived(parent_.callbacks()->dispatcher().timeSource()); + upstreamTiming().onFirstUpstreamRxByteReceived(parent_.callbacks()->dispatcher().timeSource()); maybeEndDecode(end_stream); awaiting_headers_ = false; @@ -219,15 +220,15 @@ void UpstreamRequest::decodeMetadata(Http::MetadataMapPtr&& metadata_map) { void UpstreamRequest::maybeEndDecode(bool end_stream) { if (end_stream) { - upstream_timing_.onLastUpstreamRxByteReceived(parent_.callbacks()->dispatcher().timeSource()); + upstreamTiming().onLastUpstreamRxByteReceived(parent_.callbacks()->dispatcher().timeSource()); decode_complete_ = true; } } void UpstreamRequest::onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) { - stream_info_.onUpstreamHostSelected(host); + StreamInfo::UpstreamInfo& upstream_info = *streamInfo().upstreamInfo(); + upstream_info.setUpstreamHost(host); upstream_host_ = host; - parent_.callbacks()->streamInfo().onUpstreamHostSelected(host); parent_.onUpstreamHostSelected(host); } @@ -260,7 +261,7 @@ void UpstreamRequest::encodeData(Buffer::Instance& data, bool end_stream) { stream_info_.addBytesSent(data.length()); upstream_->encodeData(data, end_stream); if (end_stream) { - upstream_timing_.onLastUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); + upstreamTiming().onLastUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); } } } @@ -277,7 +278,7 @@ void UpstreamRequest::encodeTrailers(const Http::RequestTrailerMap& trailers) { ENVOY_STREAM_LOG(trace, "proxying trailers", *parent_.callbacks()); upstream_->encodeTrailers(trailers); - upstream_timing_.onLastUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); + upstreamTiming().onLastUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); } } @@ -427,29 +428,22 @@ void UpstreamRequest::onPoolReady( stream_info_.protocol(protocol.value()); } + parent_.callbacks()->streamInfo().setUpstreamInfo(stream_info_.upstreamInfo()); if (info.upstreamInfo().has_value()) { auto& upstream_timing = info.upstreamInfo().value().get().upstreamTiming(); - upstream_timing_.upstream_connect_start_ = upstream_timing.upstream_connect_start_; - upstream_timing_.upstream_connect_complete_ = upstream_timing.upstream_connect_complete_; - upstream_timing_.upstream_handshake_complete_ = upstream_timing.upstream_handshake_complete_; + upstreamTiming().upstream_connect_start_ = upstream_timing.upstream_connect_start_; + upstreamTiming().upstream_connect_complete_ = upstream_timing.upstream_connect_complete_; + upstreamTiming().upstream_handshake_complete_ = upstream_timing.upstream_handshake_complete_; } - stream_info_.setUpstreamFilterState(std::make_shared( + StreamInfo::UpstreamInfo& upstream_info = *stream_info_.upstreamInfo(); + upstream_info.setUpstreamFilterState(std::make_shared( info.filterState().parent()->parent(), StreamInfo::FilterState::LifeSpan::Request)); - parent_.callbacks()->streamInfo().setUpstreamFilterState( - std::make_shared(info.filterState().parent()->parent(), - StreamInfo::FilterState::LifeSpan::Request)); - stream_info_.setUpstreamLocalAddress(upstream_local_address); - parent_.callbacks()->streamInfo().setUpstreamLocalAddress(upstream_local_address); - - stream_info_.setUpstreamSslConnection(info.downstreamAddressProvider().sslConnection()); - parent_.callbacks()->streamInfo().setUpstreamSslConnection( - info.downstreamAddressProvider().sslConnection()); + upstream_info.setUpstreamLocalAddress(upstream_local_address); + upstream_info.setUpstreamSslConnection(info.downstreamAddressProvider().sslConnection()); if (info.downstreamAddressProvider().connectionID().has_value()) { - stream_info_.setUpstreamConnectionId(info.downstreamAddressProvider().connectionID().value()); - parent_.callbacks()->streamInfo().setUpstreamConnectionId( - info.downstreamAddressProvider().connectionID().value()); + upstream_info.setUpstreamConnectionId(info.downstreamAddressProvider().connectionID().value()); } stream_info_.setUpstreamBytesMeter(upstream_->bytesMeter()); @@ -478,7 +472,7 @@ void UpstreamRequest::onPoolReady( span_->injectContext(*parent_.downstreamHeaders()); } - upstream_timing_.onFirstUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); + upstreamTiming().onFirstUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); // Make sure that when we are forwarding CONNECT payload we do not do so until // the upstream has accepted the CONNECT request. @@ -549,7 +543,7 @@ void UpstreamRequest::encodeBodyAndTrailers() { } if (encode_complete_) { - upstream_timing_.onLastUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); + upstreamTiming().onLastUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); } } } diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h index c5a6272f470e6..de02565a67586 100644 --- a/source/common/router/upstream_request.h +++ b/source/common/router/upstream_request.h @@ -104,7 +104,6 @@ class UpstreamRequest : public Logger::Loggable, outlier_detection_timeout_recorded_ = recorded; } bool outlierDetectionTimeoutRecorded() { return outlier_detection_timeout_recorded_; } - const StreamInfo::UpstreamTiming& upstreamTiming() { return upstream_timing_; } void retried(bool value) { retried_ = value; } bool retried() { return retried_; } bool grpcRqSuccessDeferred() { return grpc_rq_success_deferred_; } @@ -122,6 +121,9 @@ class UpstreamRequest : public Logger::Loggable, StreamInfo::StreamInfo& streamInfo() { return stream_info_; } private: + StreamInfo::UpstreamTiming& upstreamTiming() { + return stream_info_.upstreamInfo()->upstreamTiming(); + } bool shouldSendEndStream() { // Only encode end stream if the full request has been received, the body // has been sent, and any trailers or metadata have also been sent. @@ -147,7 +149,6 @@ class UpstreamRequest : public Logger::Loggable, DownstreamWatermarkManager downstream_watermark_manager_{*this}; Tracing::SpanPtr span_; StreamInfo::StreamInfoImpl stream_info_; - StreamInfo::UpstreamTiming upstream_timing_; const MonotonicTime start_time_; // This is wrapped in an optional, since we want to avoid computing zero size headers when in // reality we just didn't get a response back. diff --git a/source/common/stream_info/stream_info_impl.h b/source/common/stream_info/stream_info_impl.h index bbf1bef50e4b8..88306f2de4c10 100644 --- a/source/common/stream_info/stream_info_impl.h +++ b/source/common/stream_info/stream_info_impl.h @@ -47,9 +47,6 @@ struct UpstreamInfoImpl : public UpstreamInfo { Ssl::ConnectionInfoConstSharedPtr upstreamSslConnection() const override { return upstream_ssl_info_; } - void setUpstreamTiming(const UpstreamTiming& upstream_timing) override { - upstream_timing_ = upstream_timing; - } UpstreamTiming& upstreamTiming() override { return upstream_timing_; } const UpstreamTiming& upstreamTiming() const override { return upstream_timing_; } const Network::Address::InstanceConstSharedPtr& upstreamLocalAddress() const override { @@ -135,11 +132,6 @@ struct StreamInfoImpl : public StreamInfo { } } - void setUpstreamConnectionId(uint64_t id) override { - maybeCreateUpstreamInfo(); - upstream_info_->setUpstreamConnectionId(id); - } - absl::optional upstreamConnectionId() const override { if (!upstream_info_) { return absl::nullopt; @@ -154,13 +146,10 @@ struct StreamInfoImpl : public StreamInfo { return duration(downstream_timing_.value().lastDownstreamRxByteReceived()); } - void setUpstreamTiming(const UpstreamTiming& upstream_timing) override { - maybeCreateUpstreamInfo(); - upstream_info_->setUpstreamTiming(upstream_timing); - } - void setUpstreamInfo(std::shared_ptr info) override { upstream_info_ = info; } + std::shared_ptr upstreamInfo() override { return upstream_info_; } + OptRef upstreamInfo() const override { if (!upstream_info_) { return {}; @@ -274,11 +263,6 @@ struct StreamInfoImpl : public StreamInfo { uint64_t responseFlags() const override { return response_flags_; } - void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) override { - maybeCreateUpstreamInfo(); - upstream_info_->setUpstreamHost(host); - } - Upstream::HostDescriptionConstSharedPtr upstreamHost() const override { if (!upstream_info_) { return nullptr; @@ -292,12 +276,6 @@ struct StreamInfoImpl : public StreamInfo { const std::string& getRouteName() const override { return route_name_; } - void setUpstreamLocalAddress( - const Network::Address::InstanceConstSharedPtr& upstream_local_address) override { - maybeCreateUpstreamInfo(); - upstream_info_->setUpstreamLocalAddress(upstream_local_address); - } - const Network::Address::InstanceConstSharedPtr& upstreamLocalAddress() const override { if (!upstream_info_) { return legacy_upstream_local_address_; @@ -313,11 +291,6 @@ struct StreamInfoImpl : public StreamInfo { return *downstream_connection_info_provider_; } - void setUpstreamSslConnection(const Ssl::ConnectionInfoConstSharedPtr& connection_info) override { - maybeCreateUpstreamInfo(); - upstream_info_->setUpstreamSslConnection(connection_info); - } - Ssl::ConnectionInfoConstSharedPtr upstreamSslConnection() const override { return upstream_info_ ? upstream_info_->upstreamSslConnection() : nullptr; } @@ -340,15 +313,6 @@ struct StreamInfoImpl : public StreamInfo { } return upstream_info_->upstreamFilterState(); } - void setUpstreamFilterState(const FilterStateSharedPtr& filter_state) override { - maybeCreateUpstreamInfo(); - return upstream_info_->setUpstreamFilterState(filter_state); - } - - void setUpstreamTransportFailureReason(absl::string_view failure_reason) override { - maybeCreateUpstreamInfo(); - upstream_info_->setUpstreamTransportFailureReason(failure_reason); - } const std::string& upstreamTransportFailureReason() const override { if (!upstream_info_) { diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 4dd925c34cdc0..7f0e231fa1829 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -88,7 +88,6 @@ Config::Config(const envoy::extensions::filters::network::tcp_proxy::v3::TcpProx upstream_drain_manager_slot_(context.threadLocal().allocateSlot()), shared_config_(std::make_shared(config, context)), random_generator_(context.api().randomGenerator()) { - upstream_drain_manager_slot_->set([](Event::Dispatcher&) { ThreadLocal::ThreadLocalObjectSharedPtr drain_manager = std::make_shared(); @@ -196,7 +195,7 @@ void Filter::initialize(Network::ReadFilterCallbacks& callbacks, bool set_connec // in onData(). This will get re-enabled when the upstream connection is // established. read_callbacks_->connection().readDisable(true); - + getStreamInfo().setUpstreamInfo(std::make_shared()); config_->stats().downstream_cx_total_.inc(); if (set_connection_stats) { read_callbacks_->connection().setConnectionStats( @@ -439,7 +438,7 @@ void Filter::onGenericPoolFailure(ConnectionPool::PoolFailureReason reason, Upstream::HostDescriptionConstSharedPtr host) { generic_conn_pool_.reset(); read_callbacks_->upstreamHost(host); - getStreamInfo().onUpstreamHostSelected(host); + getStreamInfo().upstreamInfo()->setUpstreamHost(host); switch (reason) { case ConnectionPool::PoolFailureReason::Overflow: @@ -468,13 +467,14 @@ void Filter::onGenericPoolReady(StreamInfo::StreamInfo* info, upstream_ = std::move(upstream); generic_conn_pool_.reset(); read_callbacks_->upstreamHost(host); - getStreamInfo().onUpstreamHostSelected(host); - getStreamInfo().setUpstreamLocalAddress(local_address); - getStreamInfo().setUpstreamSslConnection(ssl_info); + StreamInfo::UpstreamInfo& upstream_info = *getStreamInfo().upstreamInfo(); + upstream_info.setUpstreamHost(host); + upstream_info.setUpstreamLocalAddress(local_address); + upstream_info.setUpstreamSslConnection(ssl_info); onUpstreamConnection(); read_callbacks_->continueReading(); if (info) { - read_callbacks_->connection().streamInfo().setUpstreamFilterState(info->filterState()); + upstream_info.setUpstreamFilterState(info->filterState()); } } diff --git a/source/common/upstream/health_checker_impl.cc b/source/common/upstream/health_checker_impl.cc index 6bb52b6b90ca0..ebd9b4c6e4c7d 100644 --- a/source/common/upstream/health_checker_impl.cc +++ b/source/common/upstream/health_checker_impl.cc @@ -298,7 +298,8 @@ void HttpHealthCheckerImpl::HttpActiveHealthCheckSession::onInterval() { host_->transportSocketFactory().implementsSecureTransport()); StreamInfo::StreamInfoImpl stream_info(protocol_, parent_.dispatcher_.timeSource(), local_connection_info_provider_); - stream_info.onUpstreamHostSelected(host_); + stream_info.setUpstreamInfo(std::make_shared()); + stream_info.upstreamInfo()->setUpstreamHost(host_); parent_.request_headers_parser_->evaluateHeaders(*request_headers, stream_info); auto status = request_encoder->encodeHeaders(*request_headers, true); // Encoding will only fail if required request headers are missing. diff --git a/test/common/access_log/access_log_impl_test.cc b/test/common/access_log/access_log_impl_test.cc index d2d4c291f110d..1c693a0a653ef 100644 --- a/test/common/access_log/access_log_impl_test.cc +++ b/test/common/access_log/access_log_impl_test.cc @@ -106,7 +106,7 @@ name: accesslog EXPECT_CALL(*file_, write(_)); auto cluster = std::make_shared>(); - stream_info_.onUpstreamHostSelected( + stream_info_.upstreamInfo()->setUpstreamHost( Upstream::makeTestHostDescription(cluster, "tcp://10.0.0.5:1234", simTime())); stream_info_.setResponseFlag(StreamInfo::ResponseFlag::DownstreamConnectionTermination); @@ -217,7 +217,7 @@ name: accesslog TEST_F(AccessLogImplTest, UpstreamHost) { auto cluster = std::make_shared>(); - stream_info_.onUpstreamHostSelected( + stream_info_.upstreamInfo()->setUpstreamHost( Upstream::makeTestHostDescription(cluster, "tcp://10.0.0.5:1234", simTime())); const std::string yaml = R"EOF( diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index 1970b16b2a4e3..21a83b184c765 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -106,7 +106,7 @@ TEST_F(StreamInfoHeaderFormatterTest, TestformatWithUpstreamRemoteAddressVariabl testFormatting("UPSTREAM_REMOTE_ADDRESS", "10.0.0.1:443"); NiceMock stream_info; - stream_info.host_.reset(); + stream_info.upstreamInfo()->setUpstreamHost(nullptr); testFormatting(stream_info, "UPSTREAM_REMOTE_ADDRESS", ""); } diff --git a/test/common/router/router_2_test.cc b/test/common/router/router_2_test.cc index 4d37b31004f29..5b0d6a22634b8 100644 --- a/test/common/router/router_2_test.cc +++ b/test/common/router/router_2_test.cc @@ -280,10 +280,6 @@ TEST_F(WatermarkTest, RetryRequestNotComplete) { })); EXPECT_CALL(callbacks_.stream_info_, setResponseFlag(StreamInfo::ResponseFlag::UpstreamRemoteReset)); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillRepeatedly(Invoke([&](const Upstream::HostDescriptionConstSharedPtr& host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); @@ -728,10 +724,6 @@ TEST_F(RouterTestSupressGRPCStatsEnabled, ExcludeTimeoutHttpStats) { upstream_stream_info_, Http::Protocol::Http10); return nullptr; })); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); expectResponseTimerCreate(); @@ -793,10 +785,6 @@ TEST_F(RouterTestSupressGRPCStatsDisabled, IncludeHttpTimeoutStats) { upstream_stream_info_, Http::Protocol::Http10); return nullptr; })); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); expectResponseTimerCreate(); diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 46506b71174a4..9515fc7ce261f 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -361,10 +361,6 @@ TEST_F(RouterTest, PoolFailureWithPriority) { EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(callbacks_.stream_info_, setResponseFlag(StreamInfo::ResponseFlag::UpstreamConnectionFailure)); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); @@ -396,10 +392,6 @@ TEST_F(RouterTest, PoolFailureDueToConnectTimeout) { EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(callbacks_.stream_info_, setResponseFlag(StreamInfo::ResponseFlag::UpstreamConnectionFailure)); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); @@ -1006,10 +998,6 @@ TEST_F(RouterTest, EnvoyAttemptCountInResponsePresentWithLocalReply) { EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(callbacks_.stream_info_, setResponseFlag(StreamInfo::ResponseFlag::UpstreamConnectionFailure)); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); @@ -1265,10 +1253,6 @@ TEST_F(RouterTest, UpstreamTimeout) { upstream_stream_info_, Http::Protocol::Http10); return nullptr; })); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); expectResponseTimerCreate(); @@ -1902,10 +1886,6 @@ TEST_F(RouterTest, UpstreamTimeoutWithAltResponse) { upstream_stream_info_, Http::Protocol::Http10); return nullptr; })); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); expectResponseTimerCreate(); @@ -1974,11 +1954,6 @@ TEST_F(RouterTest, UpstreamPerTryIdleTimeout) { Buffer::OwnedImpl data; router_.decodeData(data, true); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); - per_try_idle_timeout_ = new Event::MockTimer(&callbacks_.dispatcher_); EXPECT_CALL(*per_try_idle_timeout_, enableTimer(std::chrono::milliseconds(3000), _)); EXPECT_EQ(0U, @@ -2041,11 +2016,6 @@ TEST_F(RouterTest, UpstreamPerTryIdleTimeoutSuccess) { Buffer::OwnedImpl data; router_.decodeData(data, true); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); - per_try_idle_timeout_ = new Event::MockTimer(&callbacks_.dispatcher_); EXPECT_CALL(*per_try_idle_timeout_, enableTimer(std::chrono::milliseconds(3000), _)); EXPECT_EQ(0U, @@ -2083,11 +2053,6 @@ TEST_F(RouterTest, UpstreamPerTryTimeout) { upstream_stream_info_, Http::Protocol::Http10); return nullptr; })); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); - Http::TestRequestHeaderMapImpl headers{{"x-envoy-internal", "true"}, {"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); @@ -2149,11 +2114,6 @@ TEST_F(RouterTest, UpstreamPerTryTimeoutDelayedPoolReady) { // Per try timeout starts when onPoolReady is called. expectPerTryTimerCreate(); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); - EXPECT_EQ(0U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); pool_callbacks->onPoolReady(encoder, cm_.thread_local_cluster_.conn_pool_.host_, @@ -2198,11 +2158,6 @@ TEST_F(RouterTest, UpstreamPerTryTimeoutExcludesNewStream) { response_timeout_ = new Event::MockTimer(&callbacks_.dispatcher_); EXPECT_CALL(*response_timeout_, enableTimer(_, _)); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); - Http::TestRequestHeaderMapImpl headers{{"x-envoy-internal", "true"}, {"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); @@ -2548,7 +2503,7 @@ TEST_F(RouterTest, HedgedPerTryTimeoutThirdRequestSucceeds) { response_decoder3->decodeHeaders(std::move(response_headers2), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 1)); - EXPECT_EQ(333U, callbacks_.stream_info_.upstream_connection_id_); + EXPECT_EQ(333U, callbacks_.stream_info_.upstreamConnectionId()); // TODO: Verify hedge stats here once they are implemented. } @@ -3430,10 +3385,6 @@ TEST_F(RouterTest, RetryNoneHealthy) { })); expectResponseTimerCreate(); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); @@ -5162,16 +5113,15 @@ TEST_F(RouterTest, UpstreamTimingSingleRequest) { response_decoder->decodeHeaders(std::move(response_headers), false); test_time_.advanceTimeWait(std::chrono::milliseconds(43)); - // Confirm we still have no upstream timing data. It won't be set until after the - // stream has ended. - EXPECT_FALSE(stream_info.firstUpstreamTxByteSent().has_value()); - EXPECT_FALSE(stream_info.lastUpstreamTxByteSent().has_value()); - EXPECT_FALSE(stream_info.firstUpstreamRxByteReceived().has_value()); + // Upstream timing data is now available live. + EXPECT_TRUE(stream_info.firstUpstreamTxByteSent().has_value()); + EXPECT_TRUE(stream_info.lastUpstreamTxByteSent().has_value()); + EXPECT_TRUE(stream_info.firstUpstreamRxByteReceived().has_value()); EXPECT_FALSE(stream_info.lastUpstreamRxByteReceived().has_value()); response_decoder->decodeData(data, true); - // Now these should be set. + // Now all these should be set. EXPECT_TRUE(stream_info.firstUpstreamTxByteSent().has_value()); EXPECT_TRUE(stream_info.lastUpstreamTxByteSent().has_value()); EXPECT_TRUE(stream_info.firstUpstreamRxByteReceived().has_value()); @@ -5230,14 +5180,9 @@ TEST_F(RouterTest, UpstreamTimingRetry) { return nullptr; })); - // Check that upstream timing is not set when a retry will occur. Http::ResponseHeaderMapPtr bad_response_headers( new Http::TestResponseHeaderMapImpl{{":status", "503"}}); response_decoder->decodeHeaders(std::move(bad_response_headers), true); - EXPECT_FALSE(stream_info.firstUpstreamTxByteSent().has_value()); - EXPECT_FALSE(stream_info.lastUpstreamTxByteSent().has_value()); - EXPECT_FALSE(stream_info.firstUpstreamRxByteReceived().has_value()); - EXPECT_FALSE(stream_info.lastUpstreamRxByteReceived().has_value()); router_.retry_state_->callback_(); EXPECT_CALL(*router_.retry_state_, shouldRetryHeaders(_, _)).WillOnce(Return(RetryStatus::No)); @@ -5998,10 +5943,6 @@ TEST_F(RouterTest, AutoHostRewriteEnabled) { return Http::okStatus(); })); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); EXPECT_CALL(callbacks_.route_->route_entry_, autoHostRewrite()).WillOnce(Return(true)); EXPECT_CALL(callbacks_.route_->route_entry_, appendXfh()).WillOnce(Return(true)); router_.decodeHeaders(incoming_headers, true); @@ -6038,10 +5979,6 @@ TEST_F(RouterTest, AutoHostRewriteDisabled) { return Http::okStatus(); })); - EXPECT_CALL(callbacks_.stream_info_, onUpstreamHostSelected(_)) - .WillOnce(Invoke([&](const Upstream::HostDescriptionConstSharedPtr host) -> void { - EXPECT_EQ(host_address_, host->address()); - })); EXPECT_CALL(callbacks_.route_->route_entry_, autoHostRewrite()).WillOnce(Return(false)); router_.decodeHeaders(incoming_headers, true); EXPECT_EQ(1U, diff --git a/test/common/stream_info/stream_info_impl_test.cc b/test/common/stream_info/stream_info_impl_test.cc index 74ad2c1dcbabe..2f8498eeca390 100644 --- a/test/common/stream_info/stream_info_impl_test.cc +++ b/test/common/stream_info/stream_info_impl_test.cc @@ -39,7 +39,8 @@ class StreamInfoImplTest : public testing::Test { TEST_F(StreamInfoImplTest, TimingTest) { MonotonicTime pre_start = test_time_.timeSystem().monotonicTime(); StreamInfoImpl info(Http::Protocol::Http2, test_time_.timeSystem(), nullptr); - Envoy::StreamInfo::UpstreamTiming upstream_timing; + info.setUpstreamInfo(std::make_shared()); + UpstreamTiming& upstream_timing = info.upstreamInfo()->upstreamTiming(); MonotonicTime post_start = test_time_.timeSystem().monotonicTime(); const MonotonicTime& start = info.startTimeMonotonic(); @@ -54,22 +55,18 @@ TEST_F(StreamInfoImplTest, TimingTest) { EXPECT_FALSE(info.firstUpstreamTxByteSent()); upstream_timing.onFirstUpstreamTxByteSent(test_time_.timeSystem()); - info.setUpstreamTiming(upstream_timing); dur = checkDuration(dur, info.firstUpstreamTxByteSent()); EXPECT_FALSE(info.lastUpstreamTxByteSent()); upstream_timing.onLastUpstreamTxByteSent(test_time_.timeSystem()); - info.setUpstreamTiming(upstream_timing); dur = checkDuration(dur, info.lastUpstreamTxByteSent()); EXPECT_FALSE(info.firstUpstreamRxByteReceived()); upstream_timing.onFirstUpstreamRxByteReceived(test_time_.timeSystem()); - info.setUpstreamTiming(upstream_timing); dur = checkDuration(dur, info.firstUpstreamRxByteReceived()); EXPECT_FALSE(info.lastUpstreamRxByteReceived()); upstream_timing.onLastUpstreamRxByteReceived(test_time_.timeSystem()); - info.setUpstreamTiming(upstream_timing); dur = checkDuration(dur, info.lastUpstreamRxByteReceived()); EXPECT_FALSE(info.downstreamTiming().firstDownstreamTxByteSent()); @@ -140,6 +137,7 @@ TEST_F(StreamInfoImplTest, MiscSettersAndGetters) { EXPECT_EQ(nullptr, stream_info.upstreamInfo()); EXPECT_EQ(Http::Protocol::Http2, stream_info.protocol().value()); EXPECT_FALSE(stream_info.upstreamConnectionId().has_value()); + stream_info.setUpstreamInfo(std::make_shared()); stream_info.protocol(Http::Protocol::Http10); EXPECT_EQ(Http::Protocol::Http10, stream_info.protocol().value()); @@ -166,7 +164,7 @@ TEST_F(StreamInfoImplTest, MiscSettersAndGetters) { EXPECT_EQ(nullptr, stream_info.upstreamHost()); Upstream::HostDescriptionConstSharedPtr host(new NiceMock()); - stream_info.onUpstreamHostSelected(host); + stream_info.upstreamInfo()->setUpstreamHost(host); EXPECT_EQ(host, stream_info.upstreamHost()); EXPECT_FALSE(stream_info.healthCheck()); @@ -184,8 +182,7 @@ TEST_F(StreamInfoImplTest, MiscSettersAndGetters) { FilterState::LifeSpan::FilterChain); EXPECT_EQ(1, stream_info.filterState()->getDataReadOnly("test").access()); - EXPECT_EQ(nullptr, stream_info.upstreamFilterState()); - stream_info.setUpstreamFilterState(stream_info.filterState()); + stream_info.upstreamInfo()->setUpstreamFilterState(stream_info.filterState()); EXPECT_EQ(1, stream_info.upstreamFilterState()->getDataReadOnly("test").access()); @@ -199,11 +196,11 @@ TEST_F(StreamInfoImplTest, MiscSettersAndGetters) { "D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B"; auto ssl_info = std::make_shared(); EXPECT_CALL(*ssl_info, sessionId()).WillRepeatedly(testing::ReturnRef(session_id)); - stream_info.setUpstreamSslConnection(ssl_info); + stream_info.upstreamInfo()->setUpstreamSslConnection(ssl_info); EXPECT_EQ(session_id, stream_info.upstreamSslConnection()->sessionId()); EXPECT_FALSE(stream_info.upstreamConnectionId().has_value()); - stream_info.setUpstreamConnectionId(12345); + stream_info.upstreamInfo()->setUpstreamConnectionId(12345); ASSERT_TRUE(stream_info.upstreamConnectionId().has_value()); EXPECT_EQ(12345, stream_info.upstreamConnectionId().value()); diff --git a/test/common/stream_info/test_util.h b/test/common/stream_info/test_util.h index adad891693ba4..c676c44ec1fcb 100644 --- a/test/common/stream_info/test_util.h +++ b/test/common/stream_info/test_util.h @@ -25,6 +25,7 @@ class TestStreamInfo : public StreamInfo::StreamInfoImpl { MonotonicTime now = timeSystem().monotonicTime(); start_time_monotonic_ = now; end_time_ = now + std::chrono::milliseconds(3); + setUpstreamInfo(std::make_shared()); } SystemTime startTime() const override { return start_time_; } diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 8ca0a1e0ae956..e04f7ef26c58b 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1093,8 +1093,9 @@ TEST_F(TcpProxyTest, AccessDownstreamAndUpstreamProperties) { raiseEventUpstreamConnected(0); EXPECT_EQ(filter_callbacks_.connection().streamInfo().downstreamAddressProvider().sslConnection(), filter_callbacks_.connection().ssl()); - EXPECT_EQ(filter_callbacks_.connection().streamInfo().upstreamLocalAddress(), - upstream_connections_.at(0)->streamInfo().downstreamAddressProvider().localAddress()); + EXPECT_EQ( + filter_callbacks_.connection().streamInfo().upstreamLocalAddress().get(), + upstream_connections_.at(0)->streamInfo().downstreamAddressProvider().localAddress().get()); EXPECT_EQ(filter_callbacks_.connection().streamInfo().upstreamSslConnection(), upstream_connections_.at(0)->streamInfo().downstreamAddressProvider().sslConnection()); } diff --git a/test/common/tcp_proxy/tcp_proxy_test_base.h b/test/common/tcp_proxy/tcp_proxy_test_base.h index b6b437b2f38ee..7464ce7738bb1 100644 --- a/test/common/tcp_proxy/tcp_proxy_test_base.h +++ b/test/common/tcp_proxy/tcp_proxy_test_base.h @@ -62,11 +62,6 @@ class TcpProxyTestBase : public testing::Test { TcpProxyTestBase() { ON_CALL(*factory_context_.access_log_manager_.file_, write(_)) .WillByDefault(SaveArg<0>(&access_log_data_)); - ON_CALL(filter_callbacks_.connection_.stream_info_, onUpstreamHostSelected(_)) - .WillByDefault(Invoke( - [this](Upstream::HostDescriptionConstSharedPtr host) { upstream_host_ = host; })); - ON_CALL(filter_callbacks_.connection_.stream_info_, upstreamHost()) - .WillByDefault(ReturnPointee(&upstream_host_)); ON_CALL(filter_callbacks_.connection_.stream_info_, setUpstreamClusterInfo(_)) .WillByDefault(Invoke([this](const Upstream::ClusterInfoConstSharedPtr& cluster_info) { upstream_cluster_ = cluster_info; diff --git a/test/common/tracing/http_tracer_impl_test.cc b/test/common/tracing/http_tracer_impl_test.cc index 1b263c8dfe661..173173494bc47 100644 --- a/test/common/tracing/http_tracer_impl_test.cc +++ b/test/common/tracing/http_tracer_impl_test.cc @@ -95,6 +95,10 @@ TEST(HttpTracerUtilityTest, IsTracing) { class HttpConnManFinalizerImplTest : public testing::Test { protected: + HttpConnManFinalizerImplTest() { + Upstream::HostDescriptionConstSharedPtr shared_host(host_); + stream_info.upstreamInfo()->setUpstreamHost(shared_host); + } struct CustomTagCase { std::string custom_tag; bool set; @@ -117,6 +121,7 @@ class HttpConnManFinalizerImplTest : public testing::Test { NiceMock span; NiceMock config; NiceMock stream_info; + Upstream::MockHostDescription* host_{new NiceMock()}; }; TEST_F(HttpConnManFinalizerImplTest, OriginalAndLongPath) { @@ -257,7 +262,7 @@ tag: d } TEST_F(HttpConnManFinalizerImplTest, StreamInfoLogs) { - stream_info.host_->cluster_.name_ = "my_upstream_cluster"; + host_->hostname_ = "my_upstream_cluster"; EXPECT_CALL(stream_info, bytesReceived()).WillOnce(Return(10)); EXPECT_CALL(stream_info, bytesSent()).WillOnce(Return(11)); @@ -292,8 +297,8 @@ TEST_F(HttpConnManFinalizerImplTest, StreamInfoLogs) { } TEST_F(HttpConnManFinalizerImplTest, UpstreamClusterTagSet) { - stream_info.host_->cluster_.name_ = "my_upstream_cluster"; - stream_info.host_->cluster_.observability_name_ = "my_upstream_cluster_observable"; + host_->cluster_.name_ = "my_upstream_cluster"; + host_->cluster_.observability_name_ = "my_upstream_cluster_observable"; EXPECT_CALL(stream_info, bytesReceived()).WillOnce(Return(10)); EXPECT_CALL(stream_info, bytesSent()).WillOnce(Return(11)); @@ -402,8 +407,7 @@ TEST_F(HttpConnManFinalizerImplTest, SpanCustomTags) { std::shared_ptr host_metadata = std::make_shared(); (*host_metadata->mutable_filter_metadata())["m.host"].MergeFrom(fake_struct); - (*stream_info.host_->cluster_.metadata_.mutable_filter_metadata())["m.cluster"].MergeFrom( - fake_struct); + (*host_->cluster_.metadata_.mutable_filter_metadata())["m.cluster"].MergeFrom(fake_struct); absl::optional protocol = Http::Protocol::Http10; EXPECT_CALL(stream_info, bytesReceived()).WillOnce(Return(10)); @@ -411,7 +415,7 @@ TEST_F(HttpConnManFinalizerImplTest, SpanCustomTags) { absl::optional response_code; EXPECT_CALL(stream_info, responseCode()).WillRepeatedly(ReturnPointee(&response_code)); EXPECT_CALL(stream_info, bytesSent()).WillOnce(Return(100)); - EXPECT_CALL(*stream_info.host_, metadata()).WillRepeatedly(Return(host_metadata)); + EXPECT_CALL(*host_, metadata()).WillRepeatedly(Return(host_metadata)); EXPECT_CALL(config, customTags()); EXPECT_CALL(span, setTag(_, _)).Times(testing::AnyNumber()); @@ -728,6 +732,8 @@ class HttpTracerImplTest : public testing::Test { driver_ = new NiceMock(); DriverPtr driver_ptr(driver_); tracer_ = std::make_shared(std::move(driver_ptr), local_info_); + Upstream::HostDescriptionConstSharedPtr shared_host(host_); + stream_info_.upstreamInfo()->setUpstreamHost(shared_host); } Http::TestRequestHeaderMapImpl request_headers_{ @@ -742,6 +748,7 @@ class HttpTracerImplTest : public testing::Test { NiceMock config_; NiceMock* driver_; HttpTracerSharedPtr tracer_; + Upstream::MockHostDescription* host_{new NiceMock()}; }; TEST_F(HttpTracerImplTest, BasicFunctionalityNullSpan) { @@ -799,10 +806,9 @@ TEST_F(HttpTracerImplTest, ChildUpstreamSpanTest) { const std::string ob_cluster_name = "ob fake cluster"; EXPECT_CALL(stream_info_, responseCode()).WillRepeatedly(ReturnPointee(&response_code)); EXPECT_CALL(stream_info_, protocol()).WillRepeatedly(ReturnPointee(&protocol)); - EXPECT_CALL(*(stream_info_.host_), address()).WillOnce(Return(remote_address)); - EXPECT_CALL(stream_info_.host_->cluster_, name()).WillOnce(ReturnRef(cluster_name)); - EXPECT_CALL(stream_info_.host_->cluster_, observabilityName()) - .WillOnce(ReturnRef(ob_cluster_name)); + EXPECT_CALL(*host_, address()).WillOnce(Return(remote_address)); + EXPECT_CALL(host_->cluster_, name()).WillOnce(ReturnRef(cluster_name)); + EXPECT_CALL(host_->cluster_, observabilityName()).WillOnce(ReturnRef(ob_cluster_name)); EXPECT_CALL(*second_span, setTag(_, _)).Times(testing::AnyNumber()); EXPECT_CALL(*second_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/2"))); diff --git a/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc b/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc index 481c249c779c2..766cf7ca31c6f 100644 --- a/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc +++ b/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc @@ -117,8 +117,8 @@ class HttpGrpcAccessLogTest : public testing::Test { void expectLogRequestMethod(const std::string& request_method) { NiceMock stream_info; - stream_info.host_ = nullptr; stream_info.start_time_ = SystemTime(1h); + stream_info.upstreamInfo()->setUpstreamHost(nullptr); Http::TestRequestHeaderMapImpl request_headers{ {":method", request_method}, @@ -175,7 +175,7 @@ TEST_F(HttpGrpcAccessLogTest, Marshalling) { { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); stream_info.start_time_monotonic_ = MonotonicTime(1h); stream_info.last_downstream_tx_byte_sent_ = 2ms; @@ -231,7 +231,7 @@ response: {} { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); stream_info.last_downstream_tx_byte_sent_ = std::chrono::nanoseconds(2000000); @@ -264,14 +264,14 @@ response: {} stream_info.start_time_ = SystemTime(1h); stream_info.last_downstream_rx_byte_received_ = 2ms; - stream_info.first_upstream_tx_byte_sent_ = 4ms; - stream_info.last_upstream_tx_byte_sent_ = 6ms; - stream_info.first_upstream_rx_byte_received_ = 8ms; - stream_info.last_upstream_rx_byte_received_ = 10ms; + EXPECT_CALL(stream_info, firstUpstreamTxByteSent()).WillOnce(Return(4ms)); + EXPECT_CALL(stream_info, lastUpstreamTxByteSent()).WillOnce(Return(6ms)); + EXPECT_CALL(stream_info, firstUpstreamRxByteReceived()).WillOnce(Return(8ms)); + EXPECT_CALL(stream_info, lastUpstreamRxByteReceived()).WillOnce(Return(10ms)); stream_info.first_downstream_tx_byte_sent_ = 12ms; stream_info.last_downstream_tx_byte_sent_ = 14ms; - stream_info.setUpstreamLocalAddress( + stream_info.upstream_info_->setUpstreamLocalAddress( std::make_shared("10.0.0.2")); stream_info.protocol_ = Http::Protocol::Http10; stream_info.addBytesReceived(10); @@ -363,9 +363,9 @@ protocol_version: HTTP10 { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); - stream_info.upstream_transport_failure_reason_ = "TLS error"; + stream_info.upstream_info_->setUpstreamTransportFailureReason("TLS error"); Http::TestRequestHeaderMapImpl request_headers{ {":method", "WHACKADOO"}, @@ -398,7 +398,7 @@ response: {} { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); @@ -465,7 +465,7 @@ response: {} // TLSv1.2 { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); @@ -515,7 +515,7 @@ response: {} // TLSv1.1 { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); @@ -565,7 +565,7 @@ response: {} // TLSv1 { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); @@ -615,7 +615,7 @@ response: {} // Unknown TLS version (TLSv1.4) { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); @@ -685,7 +685,7 @@ TEST_F(HttpGrpcAccessLogTest, MarshallingAdditionalHeaders) { { NiceMock stream_info; - stream_info.host_ = nullptr; + stream_info.upstreamInfo()->setUpstreamHost(nullptr); stream_info.start_time_ = SystemTime(1h); Http::TestRequestHeaderMapImpl request_headers{ diff --git a/test/fuzz/utility.h b/test/fuzz/utility.h index 6647c6cf4d591..55b4772e15639 100644 --- a/test/fuzz/utility.h +++ b/test/fuzz/utility.h @@ -157,7 +157,8 @@ inline std::unique_ptr fromStreamInfo(const test::fuzz::StreamIn auto upstream_metadata = std::make_shared( replaceInvalidStringValues(stream_info.upstream_metadata())); ON_CALL(*upstream_host, metadata()).WillByDefault(testing::Return(upstream_metadata)); - test_stream_info->onUpstreamHostSelected(upstream_host); + test_stream_info->setUpstreamInfo(std::make_shared()); + test_stream_info->upstreamInfo()->setUpstreamHost(upstream_host); auto address = stream_info.has_address() ? Envoy::Network::Address::resolveProtoAddress(stream_info.address()) : Network::Utility::resolveUrl("tcp://10.0.0.1:443"); @@ -165,7 +166,7 @@ inline std::unique_ptr fromStreamInfo(const test::fuzz::StreamIn stream_info.has_upstream_local_address() ? Envoy::Network::Address::resolveProtoAddress(stream_info.upstream_local_address()) : Network::Utility::resolveUrl("tcp://10.0.0.1:10000"); - test_stream_info->setUpstreamLocalAddress(upstream_local_address); + test_stream_info->upstreamInfo()->setUpstreamLocalAddress(upstream_local_address); test_stream_info->downstream_connection_info_provider_ = std::make_shared(address, address); test_stream_info->downstream_connection_info_provider_->setRequestedServerName( diff --git a/test/mocks/stream_info/mocks.cc b/test/mocks/stream_info/mocks.cc index 3bc5473c55385..a2a2776e78b87 100644 --- a/test/mocks/stream_info/mocks.cc +++ b/test/mocks/stream_info/mocks.cc @@ -14,6 +14,16 @@ using testing::ReturnRef; namespace Envoy { namespace StreamInfo { +namespace { +absl::optional duration(absl::optional time, + MonotonicTime start_time_monotonic) { + if (!time) { + return {}; + } + + return std::chrono::duration_cast(time.value() - start_time_monotonic); +} +} // namespace MockStreamInfo::MockStreamInfo() : start_time_(ts_.systemTime()), @@ -21,6 +31,10 @@ MockStreamInfo::MockStreamInfo() downstream_connection_info_provider_(std::make_shared( std::make_shared("127.0.0.2"), std::make_shared("127.0.0.1"))) { + upstream_info_ = std::make_unique(); + Upstream::HostDescriptionConstSharedPtr host{ + new testing::NiceMock()}; + upstream_info_->setUpstreamHost(host); ON_CALL(*this, setResponseFlag(_)).WillByDefault(Invoke([this](ResponseFlag response_flag) { response_flags_ |= response_flag; })); @@ -38,14 +52,22 @@ MockStreamInfo::MockStreamInfo() ON_CALL(*this, startTimeMonotonic()).WillByDefault(ReturnPointee(&start_time_monotonic_)); ON_CALL(*this, lastDownstreamRxByteReceived()) .WillByDefault(ReturnPointee(&last_downstream_rx_byte_received_)); - ON_CALL(*this, firstUpstreamTxByteSent()) - .WillByDefault(ReturnPointee(&first_upstream_tx_byte_sent_)); - ON_CALL(*this, lastUpstreamTxByteSent()) - .WillByDefault(ReturnPointee(&last_upstream_tx_byte_sent_)); - ON_CALL(*this, firstUpstreamRxByteReceived()) - .WillByDefault(ReturnPointee(&first_upstream_rx_byte_received_)); - ON_CALL(*this, lastUpstreamRxByteReceived()) - .WillByDefault(ReturnPointee(&last_upstream_rx_byte_received_)); + ON_CALL(*this, firstUpstreamTxByteSent()).WillByDefault(Invoke([this]() { + return duration(upstream_info_->upstreamTiming().first_upstream_tx_byte_sent_, + start_time_monotonic_); + })); + ON_CALL(*this, lastUpstreamTxByteSent()).WillByDefault(Invoke([this]() { + return duration(upstream_info_->upstreamTiming().last_upstream_tx_byte_sent_, + start_time_monotonic_); + })); + ON_CALL(*this, firstUpstreamRxByteReceived()).WillByDefault(Invoke([this]() { + return duration(upstream_info_->upstreamTiming().first_upstream_rx_byte_received_, + start_time_monotonic_); + })); + ON_CALL(*this, lastUpstreamRxByteReceived()).WillByDefault(Invoke([this]() { + return duration(upstream_info_->upstreamTiming().last_upstream_rx_byte_received_, + start_time_monotonic_); + })); ON_CALL(*this, firstDownstreamTxByteSent()) .WillByDefault(ReturnPointee(&first_downstream_tx_byte_sent_)); ON_CALL(*this, lastDownstreamTxByteSent()) @@ -57,19 +79,17 @@ MockStreamInfo::MockStreamInfo() .count()); })); ON_CALL(*this, downstreamTiming()).WillByDefault(ReturnRef(downstream_timing_)); - ON_CALL(*this, setUpstreamLocalAddress(_)) - .WillByDefault( - Invoke([this](const Network::Address::InstanceConstSharedPtr& upstream_local_address) { - upstream_local_address_ = upstream_local_address; - })); - ON_CALL(*this, upstreamLocalAddress()).WillByDefault(ReturnRef(upstream_local_address_)); + ON_CALL(*this, upstreamInfo()).WillByDefault(Invoke([this]() { return upstream_info_; })); + ON_CALL(testing::Const(*this), upstreamInfo()).WillByDefault(Invoke([this]() { + return OptRef(*upstream_info_); + })); + ON_CALL(*this, upstreamLocalAddress()).WillByDefault(Invoke([this]() { + return upstream_info_->upstreamLocalAddress(); + })); ON_CALL(*this, downstreamAddressProvider()) .WillByDefault(ReturnPointee(downstream_connection_info_provider_)); - ON_CALL(*this, setUpstreamSslConnection(_)) - .WillByDefault(Invoke( - [this](const auto& connection_info) { upstream_connection_info_ = connection_info; })); ON_CALL(*this, upstreamSslConnection()).WillByDefault(Invoke([this]() { - return upstream_connection_info_; + return upstream_info_->upstreamSslConnection(); })); ON_CALL(*this, protocol()).WillByDefault(ReturnPointee(&protocol_)); ON_CALL(*this, responseCode()).WillByDefault(ReturnPointee(&response_code_)); @@ -94,38 +114,33 @@ MockStreamInfo::MockStreamInfo() return response_flags_ != 0; })); ON_CALL(*this, responseFlags()).WillByDefault(Return(response_flags_)); - ON_CALL(*this, upstreamHost()).WillByDefault(ReturnPointee(&host_)); - + ON_CALL(*this, upstreamHost()).WillByDefault(Invoke([this]() { + return upstream_info_->upstreamHost(); + })); ON_CALL(*this, dynamicMetadata()).WillByDefault(ReturnRef(metadata_)); ON_CALL(Const(*this), dynamicMetadata()).WillByDefault(ReturnRef(metadata_)); ON_CALL(*this, filterState()).WillByDefault(ReturnRef(filter_state_)); ON_CALL(Const(*this), filterState()).WillByDefault(Invoke([this]() -> const FilterState& { return *filter_state_; })); - ON_CALL(*this, upstreamFilterState()).WillByDefault(ReturnRef(upstream_filter_state_)); - ON_CALL(*this, setUpstreamFilterState(_)) - .WillByDefault(Invoke([this](const FilterStateSharedPtr& filter_state) { - upstream_filter_state_ = filter_state; - })); + ON_CALL(*this, upstreamFilterState()).WillByDefault(Invoke([this]() { + return upstream_info_->upstreamFilterState(); + })); ON_CALL(*this, setRouteName(_)).WillByDefault(Invoke([this](const absl::string_view route_name) { route_name_ = std::string(route_name); })); ON_CALL(*this, getRouteName()).WillByDefault(ReturnRef(route_name_)); ON_CALL(*this, upstreamTransportFailureReason()) - .WillByDefault(ReturnRef(upstream_transport_failure_reason_)); - ON_CALL(*this, setConnectionID(_)).WillByDefault(Invoke([this](uint64_t id) { - connection_id_ = id; - })); + .WillByDefault(ReturnRef(upstream_info_->upstreamTransportFailureReason())); + ON_CALL(*this, setUpstreamInfo(_)) + .WillByDefault(Invoke([this](std::shared_ptr info) { upstream_info_ = info; })); ON_CALL(*this, setFilterChainName(_)) .WillByDefault(Invoke([this](const absl::string_view filter_chain_name) { filter_chain_name_ = std::string(filter_chain_name); })); ON_CALL(*this, filterChainName()).WillByDefault(ReturnRef(filter_chain_name_)); - ON_CALL(*this, setUpstreamConnectionId(_)).WillByDefault(Invoke([this](uint64_t id) { - upstream_connection_id_ = id; - })); ON_CALL(*this, upstreamConnectionId()).WillByDefault(Invoke([this]() { - return upstream_connection_id_; + return upstream_info_->upstreamConnectionId(); })); ON_CALL(*this, setAttemptCount(_)).WillByDefault(Invoke([this](uint32_t attempt_count) { attempt_count_ = attempt_count; @@ -141,8 +156,8 @@ MockStreamInfo::MockStreamInfo() .WillByDefault(Invoke([this](const BytesMeterSharedPtr& downstream_bytes_meter) { downstream_bytes_meter_ = downstream_bytes_meter; })); - ON_CALL(*this, upstreamTiming()).WillByDefault(ReturnRef(upstream_timing_)); - ON_CALL(Const(*this), upstreamTiming()).WillByDefault(Return(upstream_timing_)); + ON_CALL(*this, upstreamTiming()).WillByDefault(ReturnRef(upstream_info_->upstreamTiming())); + ON_CALL(Const(*this), upstreamTiming()).WillByDefault(Return(upstream_info_->upstreamTiming())); } MockStreamInfo::~MockStreamInfo() = default; diff --git a/test/mocks/stream_info/mocks.h b/test/mocks/stream_info/mocks.h index 4f96a1d16ff4b..39503abbc73db 100644 --- a/test/mocks/stream_info/mocks.h +++ b/test/mocks/stream_info/mocks.h @@ -6,6 +6,7 @@ #include "source/common/network/socket_impl.h" #include "source/common/stream_info/filter_state_impl.h" +#include "source/common/stream_info/stream_info_impl.h" #include "test/mocks/upstream/host.h" #include "test/test_common/simulated_time_system.h" @@ -30,7 +31,6 @@ class MockStreamInfo : public StreamInfo { MOCK_METHOD(SystemTime, startTime, (), (const)); MOCK_METHOD(MonotonicTime, startTimeMonotonic, (), (const)); MOCK_METHOD(absl::optional, lastDownstreamRxByteReceived, (), (const)); - MOCK_METHOD(void, setUpstreamTiming, (const UpstreamTiming&)); MOCK_METHOD(void, setUpstreamInfo, (std::shared_ptr)); MOCK_METHOD(std::shared_ptr, upstreamInfo, ()); MOCK_METHOD(OptRef, upstreamInfo, (), (const)); @@ -68,12 +68,10 @@ class MockStreamInfo : public StreamInfo { MOCK_METHOD(bool, hasAnyResponseFlag, (), (const)); MOCK_METHOD(uint64_t, responseFlags, (), (const)); MOCK_METHOD(Upstream::HostDescriptionConstSharedPtr, upstreamHost, (), (const)); - MOCK_METHOD(void, setUpstreamLocalAddress, (const Network::Address::InstanceConstSharedPtr&)); MOCK_METHOD(const Network::Address::InstanceConstSharedPtr&, upstreamLocalAddress, (), (const)); MOCK_METHOD(bool, healthCheck, (), (const)); MOCK_METHOD(void, healthCheck, (bool is_health_check)); MOCK_METHOD(const Network::ConnectionInfoProvider&, downstreamAddressProvider, (), (const)); - MOCK_METHOD(void, setUpstreamSslConnection, (const Ssl::ConnectionInfoConstSharedPtr&)); MOCK_METHOD(Ssl::ConnectionInfoConstSharedPtr, upstreamSslConnection, (), (const)); MOCK_METHOD(Router::RouteConstSharedPtr, route, (), (const)); MOCK_METHOD(envoy::config::core::v3::Metadata&, dynamicMetadata, ()); @@ -84,8 +82,6 @@ class MockStreamInfo : public StreamInfo { MOCK_METHOD(const FilterStateSharedPtr&, filterState, ()); MOCK_METHOD(const FilterState&, filterState, (), (const)); MOCK_METHOD(const FilterStateSharedPtr&, upstreamFilterState, (), (const)); - MOCK_METHOD(void, setUpstreamFilterState, (const FilterStateSharedPtr&)); - MOCK_METHOD(void, setUpstreamTransportFailureReason, (absl::string_view)); MOCK_METHOD(const std::string&, upstreamTransportFailureReason, (), (const)); MOCK_METHOD(void, setRequestHeaders, (const Http::RequestHeaderMap&)); MOCK_METHOD(const Http::RequestHeaderMap*, getRequestHeaders, (), (const)); @@ -101,7 +97,6 @@ class MockStreamInfo : public StreamInfo { MOCK_METHOD(void, setConnectionID, (uint64_t)); MOCK_METHOD(void, setFilterChainName, (const absl::string_view)); MOCK_METHOD(const std::string&, filterChainName, (), (const)); - MOCK_METHOD(void, setUpstreamConnectionId, (uint64_t)); MOCK_METHOD(absl::optional, upstreamConnectionId, (), (const)); MOCK_METHOD(void, setAttemptCount, (uint32_t), ()); MOCK_METHOD(absl::optional, attemptCount, (), (const)); @@ -109,17 +104,10 @@ class MockStreamInfo : public StreamInfo { MOCK_METHOD(const BytesMeterSharedPtr&, getDownstreamBytesMeter, (), (const)); MOCK_METHOD(void, setUpstreamBytesMeter, (const BytesMeterSharedPtr&)); MOCK_METHOD(void, setDownstreamBytesMeter, (const BytesMeterSharedPtr&)); - std::shared_ptr> host_{ - new testing::NiceMock()}; Envoy::Event::SimulatedTimeSystem ts_; SystemTime start_time_; MonotonicTime start_time_monotonic_; absl::optional last_downstream_rx_byte_received_; - absl::optional first_upstream_tx_byte_sent_; - absl::optional last_upstream_tx_byte_sent_; - absl::optional first_upstream_rx_byte_received_; - absl::optional connection_id_; - absl::optional last_upstream_rx_byte_received_; absl::optional first_downstream_tx_byte_sent_; absl::optional last_downstream_tx_byte_sent_; absl::optional end_time_; @@ -127,23 +115,18 @@ class MockStreamInfo : public StreamInfo { absl::optional response_code_; absl::optional response_code_details_; absl::optional connection_termination_details_; - UpstreamTiming upstream_timing_; + std::shared_ptr upstream_info_; uint64_t response_flags_{}; envoy::config::core::v3::Metadata metadata_; - FilterStateSharedPtr upstream_filter_state_; FilterStateSharedPtr filter_state_; uint64_t bytes_received_{}; uint64_t bytes_sent_{}; - Network::Address::InstanceConstSharedPtr upstream_local_address_; std::shared_ptr downstream_connection_info_provider_; BytesMeterSharedPtr upstream_bytes_meter_; BytesMeterSharedPtr downstream_bytes_meter_; Ssl::ConnectionInfoConstSharedPtr downstream_connection_info_; - Ssl::ConnectionInfoConstSharedPtr upstream_connection_info_; std::string route_name_; - std::string upstream_transport_failure_reason_; std::string filter_chain_name_; - absl::optional upstream_connection_id_; absl::optional attempt_count_; DownstreamTiming downstream_timing_; };