Skip to content

Commit

Permalink
filter: add ability to resetIdleTimer (#17274)
Browse files Browse the repository at this point in the history
Signed-off-by: Jose Nino <[email protected]>
  • Loading branch information
junr03 authored Jul 10, 2021
1 parent fd19330 commit b11c25f
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 5 deletions.
5 changes: 5 additions & 0 deletions envoy/http/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,11 @@ class StreamFilterCallbacks {
* added to.
*/
virtual void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) PURE;

/**
* Called when filter activity indicates that the stream idle timeout should be reset.
*/
virtual void resetIdleTimer() PURE;
};

/**
Expand Down
9 changes: 4 additions & 5 deletions source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,6 @@ class AsyncStreamImpl : public AsyncClient::Stream,
AsyncStreamImpl(AsyncClientImpl& parent, AsyncClient::StreamCallbacks& callbacks,
const AsyncClient::StreamOptions& options);

// Http::StreamDecoderFilterCallbacks
void requestRouteConfigUpdate(Http::RouteConfigUpdatedCallbackSharedPtr) override {
NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
}

// Http::AsyncClient::Stream
void sendHeaders(RequestHeaderMap& headers, bool end_stream) override;
void sendData(Buffer::Instance& data, bool end_stream) override;
Expand Down Expand Up @@ -436,6 +431,10 @@ class AsyncStreamImpl : public AsyncClient::Stream,
}
void addUpstreamSocketOptions(const Network::Socket::OptionsSharedPtr&) override {}
Network::Socket::OptionsSharedPtr getUpstreamSocketOptions() const override { return {}; }
void requestRouteConfigUpdate(Http::RouteConfigUpdatedCallbackSharedPtr) override {
NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
}
void resetIdleTimer() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }

// ScopeTrackedObject
void dumpState(std::ostream& os, int indent_level) const override {
Expand Down
4 changes: 4 additions & 0 deletions source/common/http/filter_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ void ActiveStreamFilterBase::clearRouteCache() {
parent_.filter_manager_callbacks_.clearRouteCache();
}

void ActiveStreamFilterBase::resetIdleTimer() {
parent_.filter_manager_callbacks_.resetIdleTimer();
}

void FilterMatchState::evaluateMatchTreeWithNewData(MatchDataUpdateFunc update_func) {
if (match_tree_evaluated_ || !matching_data_) {
return;
Expand Down
1 change: 1 addition & 0 deletions source/common/http/filter_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ struct ActiveStreamFilterBase : public virtual StreamFilterCallbacks,
Tracing::Config& tracingConfig() override;
const ScopeTrackedObject& scope() override;
void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) override;
void resetIdleTimer() override;

// Functions to set or get iteration state.
bool canIterate() { return iteration_state_ == IterationState::Continue; }
Expand Down
17 changes: 17 additions & 0 deletions test/common/http/filter_manager_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,23 @@ TEST_F(FilterManagerTest, MultipleOnLocalReply) {
filter_manager_->destroyFilters();
}

TEST_F(FilterManagerTest, ResetIdleTimer) {
initialize();

std::shared_ptr<MockStreamDecoderFilter> decoder_filter(new NiceMock<MockStreamDecoderFilter>());

EXPECT_CALL(filter_factory_, createFilterChain(_))
.WillRepeatedly(Invoke([&](FilterChainFactoryCallbacks& callbacks) -> void {
callbacks.addStreamDecoderFilter(decoder_filter);
}));
filter_manager_->createFilterChain();

EXPECT_CALL(filter_manager_callbacks_, resetIdleTimer());
decoder_filter->callbacks_->resetIdleTimer();

filter_manager_->destroyFilters();
}

} // namespace
} // namespace Http
} // namespace Envoy
1 change: 1 addition & 0 deletions test/integration/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ envoy_cc_test(
deps = [
":http_protocol_integration_lib",
"//test/integration/filters:backpressure_filter_config_lib",
"//test/integration/filters:reset_idle_timer_filter_lib",
"//test/test_common:test_time_lib",
"@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
"@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto",
Expand Down
15 changes: 15 additions & 0 deletions test/integration/filters/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,21 @@ envoy_cc_test_library(
],
)

envoy_cc_test_library(
name = "reset_idle_timer_filter_lib",
srcs = [
"reset_idle_timer_filter.cc",
],
deps = [
":common_lib",
"//envoy/http:filter_interface",
"//envoy/registry",
"//envoy/server:filter_config_interface",
"//source/extensions/filters/http/common:pass_through_filter_lib",
"//test/extensions/filters/http/common:empty_http_filter_config_lib",
],
)

envoy_cc_test_library(
name = "set_response_code_filter_lib",
srcs = [
Expand Down
39 changes: 39 additions & 0 deletions test/integration/filters/reset_idle_timer_filter.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#include <string>

#include "envoy/http/filter.h"
#include "envoy/registry/registry.h"
#include "envoy/server/filter_config.h"

#include "source/extensions/filters/http/common/pass_through_filter.h"

#include "test/extensions/filters/http/common/empty_http_filter_config.h"

namespace Envoy {

// A test filter that resets the stream idle timer via callbacks.
class ResetIdleTimerFilter : public Http::PassThroughFilter {
public:
Http::FilterDataStatus decodeData(Buffer::Instance&, bool) override {
decoder_callbacks_->resetIdleTimer();
return Http::FilterDataStatus::Continue;
}
};

class ResetIdleTimerFilterConfig : public Extensions::HttpFilters::Common::EmptyHttpFilterConfig {
public:
ResetIdleTimerFilterConfig() : EmptyHttpFilterConfig("reset-idle-timer-filter") {}

Http::FilterFactoryCb createFilter(const std::string&,
Server::Configuration::FactoryContext&) override {
return [](Http::FilterChainFactoryCallbacks& callbacks) -> void {
callbacks.addStreamFilter(std::make_shared<::Envoy::ResetIdleTimerFilter>());
};
}
};

// perform static registration
static Registry::RegisterFactory<ResetIdleTimerFilterConfig,
Server::Configuration::NamedHttpFilterConfigFactory>
register_;

} // namespace Envoy
29 changes: 29 additions & 0 deletions test/integration/idle_timeout_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,35 @@ TEST_P(IdleTimeoutIntegrationTest, RequestTimeoutIsNotDisarmedByEncode100Continu
EXPECT_EQ("request timeout", response->body());
}

// Per-stream idle timeout reset from within a filter.
TEST_P(IdleTimeoutIntegrationTest, PerStreamIdleTimeoutResetFromFilter) {
config_helper_.addFilter(R"EOF(
name: reset-idle-timer-filter
)EOF");
enable_per_stream_idle_timeout_ = true;

auto response = setupPerStreamIdleTimeoutTest();

sleep();
codec_client_->sendData(*request_encoder_, 1, false);

// Two sleeps should trigger the timer, as each advances time by timeout / 2. However, the data
// frame above would have caused the filter to reset the timer. Thus, the stream should not be
// reset yet.
sleep();

EXPECT_FALSE(response->complete());

sleep();

waitForTimeout(*response, "downstream_rq_idle_timeout");

// Now the timer should have triggered.
EXPECT_TRUE(response->complete());
EXPECT_EQ("408", response->headers().getStatusValue());
EXPECT_EQ("stream timeout", response->body());
}

// TODO(auni53) create a test filter that hangs and does not send data upstream, which would
// trigger a configured request_timer

Expand Down
2 changes: 2 additions & 0 deletions test/mocks/http/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ class MockStreamDecoderFilterCallbacks : public StreamDecoderFilterCallbacks,
MOCK_METHOD(const Network::Connection*, connection, ());
MOCK_METHOD(Event::Dispatcher&, dispatcher, ());
MOCK_METHOD(void, resetStream, ());
MOCK_METHOD(void, resetIdleTimer, ());
MOCK_METHOD(Upstream::ClusterInfoConstSharedPtr, clusterInfo, ());
MOCK_METHOD(Router::RouteConstSharedPtr, route, ());
MOCK_METHOD(Router::RouteConstSharedPtr, route, (const Router::RouteCallback&));
Expand Down Expand Up @@ -292,6 +293,7 @@ class MockStreamEncoderFilterCallbacks : public StreamEncoderFilterCallbacks,
MOCK_METHOD(const Network::Connection*, connection, ());
MOCK_METHOD(Event::Dispatcher&, dispatcher, ());
MOCK_METHOD(void, resetStream, ());
MOCK_METHOD(void, resetIdleTimer, ());
MOCK_METHOD(Upstream::ClusterInfoConstSharedPtr, clusterInfo, ());
MOCK_METHOD(void, requestRouteConfigUpdate, (std::function<void()>));
MOCK_METHOD(bool, canRequestRouteConfigUpdate, ());
Expand Down

0 comments on commit b11c25f

Please sign in to comment.