Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion api/envoy/api/v2/route/route.proto
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,11 @@ message VirtualHost {
//
// Envoy supports routing on HTTP method via :ref:`header matching
// <envoy_api_msg_route.HeaderMatcher>`.
// [#comment:next free field: 14]
// [#comment:next free field: 15]
message Route {
// Name for the route.
string name = 14;

// Route matching parameters.
RouteMatch match = 1 [(validate.rules).message.required = true, (gogoproto.nullable) = false];

Expand Down
3 changes: 3 additions & 0 deletions api/envoy/data/accesslog/v2/accesslog.proto
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ message AccessLogCommon {
// upstream transport socket. Common TLS failures are in
// :ref:`TLS trouble shooting <arch_overview_ssl_trouble_shooting>`.
string upstream_transport_failure_reason = 18;

// The name of the route
string route_name = 19;
}

// Flags indicating occurrences during request/response processing.
Expand Down
3 changes: 3 additions & 0 deletions docs/root/configuration/access_log.rst
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,9 @@ The following command operators are supported:
TCP
Not implemented ("-").

%ROUTE_NAME%
Name of the route.

%UPSTREAM_HOST%
Upstream host URL (e.g., tcp://ip:port for TCP connections).

Expand Down
2 changes: 2 additions & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Version history
1.11.0 (Pending)
================
* access log: added a new field for downstream TLS session ID to file and gRPC access logger.
* access log: added a new field for route name to file and gRPC access logger.
* access log: added a new field for response code details in :ref:`file access logger<config_access_log_format_response_code_details>` and :ref:`gRPC access logger<envoy_api_field_data.accesslog.v2.HTTPResponseProperties.response_code_details>`.
* admin: the administration interface now includes a :ref:`/ready endpoint <operations_admin_interface>` for easier readiness checks.
* admin: extend :ref:`/runtime_modify endpoint <operations_admin_interface_runtime_modify>` to support parameters within the request body.
Expand All @@ -30,6 +31,7 @@ Version history
:ref:`buffer_flush_timeout <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.ConnPoolSettings.buffer_flush_timeout>` to control how quickly the buffer is flushed if it is not full.
* router: add support for configuring a :ref:`grpc timeout offset <envoy_api_field_route.RouteAction.grpc_timeout_offset>` on incoming requests.
* router: added ability to control retry back-off intervals via :ref:`retry policy <envoy_api_msg_route.RetryPolicy.RetryBackOff>`.
* router: added a route name field to each http route in route.Route list
* router: per try timeouts will no longer start before the downstream request has been received
in full by the router. This ensures that the per try timeout does not account for slow
downstreams and that will not start before the global timeout.
Expand Down
10 changes: 10 additions & 0 deletions include/envoy/router/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ class DirectResponseEntry : public ResponseEntry {
*/
virtual void rewritePathHeader(Http::HeaderMap& headers,
bool insert_envoy_original_path) const PURE;

/**
* @return std::string& the name of the route.
*/
virtual const std::string& routeName() const PURE;
};

/**
Expand Down Expand Up @@ -711,6 +716,11 @@ class RouteEntry : public ResponseEntry {
* @returns the internal redirect action which should be taken on this route.
*/
virtual InternalRedirectAction internalRedirectAction() const PURE;

/**
* @return std::string& the name of the route.
*/
virtual const std::string& routeName() const PURE;
};

/**
Expand Down
9 changes: 9 additions & 0 deletions include/envoy/stream_info/stream_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,15 @@ class StreamInfo {
*/
virtual void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) PURE;

/**
* @param std::string name denotes the name of the route.
*/
virtual void setRouteName(absl::string_view name) PURE;

/**
* @return std::string& the name of the route.
*/
virtual const std::string& getRouteName() const PURE;
/**
* @param bytes_received denotes number of bytes to add to total received bytes.
*/
Expand Down
5 changes: 5 additions & 0 deletions source/common/access_log/access_log_formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,11 @@ StreamInfoFormatter::StreamInfoFormatter(const std::string& field_name) {
return UnspecifiedValueString;
}
};
} else if (field_name == "ROUTE_NAME") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add this change to the public docs as well as version history. Also, please make a parallel change to add this to the gRPC access logs.

field_extractor_ = [](const StreamInfo::StreamInfo& stream_info) {
std::string route_name = stream_info.getRouteName();
return route_name.empty() ? UnspecifiedValueString : route_name;
};
} else if (field_name == "DOWNSTREAM_PEER_URI_SAN") {
field_extractor_ =
sslConnectionInfoStringExtractor([](const Ssl::ConnectionInfo& connection_info) {
Expand Down
3 changes: 2 additions & 1 deletion source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
Router::InternalRedirectAction internalRedirectAction() const override {
return Router::InternalRedirectAction::PassThrough;
}

const std::string& routeName() const override { return route_name_; }
static const NullHedgePolicy hedge_policy_;
static const NullRateLimitPolicy rate_limit_policy_;
static const NullRetryPolicy retry_policy_;
Expand All @@ -277,6 +277,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
Router::RouteEntry::UpgradeMap upgrade_map_;
const std::string& cluster_name_;
absl::optional<std::chrono::milliseconds> timeout_;
const std::string route_name_;
};

struct RouteImpl : public Router::Route {
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,
direct_response_body_(ConfigUtility::parseDirectResponseBody(route, factory_context.api())),
per_filter_configs_(route.typed_per_filter_config(), route.per_filter_config(),
factory_context),
time_source_(factory_context.dispatcher().timeSource()),
route_name_(route.name()), time_source_(factory_context.dispatcher().timeSource()),
internal_redirect_action_(convertInternalRedirectAction(route.route())) {
if (route.route().has_metadata_match()) {
const auto filter_it = route.route().metadata_match().filter_metadata().find(
Expand Down
7 changes: 7 additions & 0 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ class SslRedirector : public DirectResponseEntry {
void rewritePathHeader(Http::HeaderMap&, bool) const override {}
Http::Code responseCode() const override { return Http::Code::MovedPermanently; }
const std::string& responseBody() const override { return EMPTY_STRING; }
const std::string& routeName() const override { return route_name_; }

private:
const std::string route_name_;
};

class SslRedirectRoute : public Route {
Expand Down Expand Up @@ -376,6 +380,7 @@ class RouteEntryImplBase : public RouteEntry,
Http::Code clusterNotFoundResponseCode() const override {
return cluster_not_found_response_code_;
}
const std::string& routeName() const override { return route_name_; }
const CorsPolicy* corsPolicy() const override { return cors_policy_.get(); }
void finalizeRequestHeaders(Http::HeaderMap& headers, const StreamInfo::StreamInfo& stream_info,
bool insert_envoy_original_path) const override;
Expand Down Expand Up @@ -462,6 +467,7 @@ class RouteEntryImplBase : public RouteEntry,
DynamicRouteEntry(const RouteEntryImplBase* parent, const std::string& name)
: parent_(parent), cluster_name_(name) {}

const std::string& routeName() const override { return parent_->routeName(); }
// Router::RouteEntry
const std::string& clusterName() const override { return cluster_name_; }
Http::Code clusterNotFoundResponseCode() const override {
Expand Down Expand Up @@ -653,6 +659,7 @@ class RouteEntryImplBase : public RouteEntry,
const absl::optional<Http::Code> direct_response_code_;
std::string direct_response_body_;
PerFilterConfigs per_filter_configs_;
const std::string route_name_;
TimeSource& time_source_;
InternalRedirectAction internal_redirect_action_;
};
Expand Down
3 changes: 3 additions & 0 deletions source/common/router/router.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,14 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
direct_response->finalizeResponseHeaders(response_headers, callbacks_->streamInfo());
},
absl::nullopt, StreamInfo::ResponseCodeDetails::get().DirectResponse);
callbacks_->streamInfo().setRouteName(direct_response->routeName());
return Http::FilterHeadersStatus::StopIteration;
}

// A route entry matches for the request.
route_entry_ = route_->routeEntry();
callbacks_->streamInfo().setRouteName(route_entry_->routeName());

Upstream::ThreadLocalCluster* cluster = config_.cm_.get(route_entry_->clusterName());
if (!cluster) {
config_.stats_.no_cluster_.inc();
Expand Down
7 changes: 7 additions & 0 deletions source/common/stream_info/stream_info_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ struct StreamInfoImpl : public StreamInfo {

Upstream::HostDescriptionConstSharedPtr upstreamHost() const override { return upstream_host_; }

void setRouteName(absl::string_view route_name) override {
route_name_ = std::string(route_name);
}

const std::string& getRouteName() const override { return route_name_; }

void setUpstreamLocalAddress(
const Network::Address::InstanceConstSharedPtr& upstream_local_address) override {
upstream_local_address_ = upstream_local_address;
Expand Down Expand Up @@ -220,6 +226,7 @@ struct StreamInfoImpl : public StreamInfo {
const Router::RouteEntry* route_entry_{};
envoy::api::v2::core::Metadata metadata_{};
FilterStateImpl filter_state_{};
std::string route_name_;

private:
uint64_t bytes_received_{};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,11 @@ void HttpGrpcAccessLog::log(const Http::HeaderMap* request_headers,
*common_properties->mutable_upstream_remote_address());
common_properties->set_upstream_cluster(stream_info.upstreamHost()->cluster().name());
}

if (!stream_info.getRouteName().empty()) {
common_properties->set_route_name(stream_info.getRouteName());
}

if (stream_info.upstreamLocalAddress() != nullptr) {
Network::Utility::addressToProtobufAddress(
*stream_info.upstreamLocalAddress(), *common_properties->mutable_upstream_local_address());
Expand Down
25 changes: 25 additions & 0 deletions test/common/access_log/access_log_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,31 @@ TEST_F(AccessLogImplTest, DownstreamDisconnect) {
output_);
}

TEST_F(AccessLogImplTest, RouteName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add blank line above

const std::string json = R"EOF(
{
"path": "/dev/null",
"format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %RESPONSE_FLAGS% %ROUTE_NAME% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\"\n"
}
)EOF";

InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromJson(json), context_);

EXPECT_CALL(*file_, write(_));
stream_info_.route_name_ = "route-test-name";
stream_info_.response_flags_ = StreamInfo::ResponseFlag::UpstreamConnectionFailure;
request_headers_.addCopy(Http::Headers::get().UserAgent, "user-agent-set");
request_headers_.addCopy(Http::Headers::get().RequestId, "id");
request_headers_.addCopy(Http::Headers::get().Host, "host");
request_headers_.addCopy(Http::Headers::get().ForwardedFor, "x.x.x.x");

log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_);
EXPECT_EQ(
"[1999-01-01T00:00:00.000Z] \"GET / HTTP/1.1\" 0 UF route-test-name 1 2 3 - \"x.x.x.x\" "
"\"user-agent-set\" \"id\" \"host\"\n",
output_);
}

TEST_F(AccessLogImplTest, EnvoyUpstreamServiceTime) {
const std::string json = R"EOF(
{
Expand Down
34 changes: 33 additions & 1 deletion test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2908,6 +2908,38 @@ static Http::TestHeaderMapImpl genRedirectHeaders(const std::string& host, const
return headers;
}

TEST_F(RouteMatcherTest, RouteName) {
std::string yaml = R"EOF(
virtual_hosts:
- name: "www2"
domains: ["www.lyft.com"]
routes:
- name: "route-test"
match: { prefix: "/"}
route:
cluster: "ufesservice"
- name: redirect
domains: [redirect.lyft.com]
routes:
- name: "route-test-2"
match: { path: /host }
redirect: { host_redirect: new.lyft.com }
)EOF";
NiceMock<Server::Configuration::MockFactoryContext> factory_context;
TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context, false);
{
Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/", "GET");
EXPECT_EQ("route-test", config.route(headers, 0)->routeEntry()->routeName());
}

{
Http::TestHeaderMapImpl headers =
genRedirectHeaders("redirect.lyft.com", "/host", false, false);
const DirectResponseEntry* redirect = config.route(headers, 0)->directResponseEntry();
EXPECT_EQ("route-test-2", redirect->routeName());
}
}

TEST_F(RouteMatcherTest, DirectResponse) {
const auto pathname =
TestEnvironment::writeStringToFileForTest("direct_response_body", "Example text 3");
Expand Down Expand Up @@ -5618,4 +5650,4 @@ name: foo

} // namespace
} // namespace Router
} // namespace Envoy
} // namespace Envoy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: put the newline back

16 changes: 16 additions & 0 deletions test/common/router/router_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2253,12 +2253,16 @@ TEST_F(RouterTest, AltStatName) {

TEST_F(RouterTest, Redirect) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any tests in here for actually setting stream info. Can you add them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattklein123 I have added a couple of checks for this. I also made a separate commit to with changes in access logs - this will cover the tests for route name in stream info. Let me know if this works - thanks!

MockDirectResponseEntry direct_response;
std::string route_name("route-test-name");
EXPECT_CALL(direct_response, newPath(_)).WillOnce(Return("hello"));
EXPECT_CALL(direct_response, routeName()).WillOnce(ReturnRef(route_name));
EXPECT_CALL(direct_response, rewritePathHeader(_, _));
EXPECT_CALL(direct_response, responseCode()).WillOnce(Return(Http::Code::MovedPermanently));
EXPECT_CALL(direct_response, responseBody()).WillOnce(ReturnRef(EMPTY_STRING));
EXPECT_CALL(direct_response, finalizeResponseHeaders(_, _));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));
absl::string_view route_name_view(route_name);
EXPECT_CALL(callbacks_.stream_info_, setRouteName(route_name_view));

Http::TestHeaderMapImpl response_headers{{":status", "301"}, {"location", "hello"}};
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true));
Expand All @@ -2270,12 +2274,16 @@ TEST_F(RouterTest, Redirect) {

TEST_F(RouterTest, RedirectFound) {
MockDirectResponseEntry direct_response;
std::string route_name("route-test-name");
EXPECT_CALL(direct_response, newPath(_)).WillOnce(Return("hello"));
EXPECT_CALL(direct_response, routeName()).WillOnce(ReturnRef(route_name));
EXPECT_CALL(direct_response, rewritePathHeader(_, _));
EXPECT_CALL(direct_response, responseCode()).WillOnce(Return(Http::Code::Found));
EXPECT_CALL(direct_response, responseBody()).WillOnce(ReturnRef(EMPTY_STRING));
EXPECT_CALL(direct_response, finalizeResponseHeaders(_, _));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));
absl::string_view route_name_view(route_name);
EXPECT_CALL(callbacks_.stream_info_, setRouteName(route_name_view));

Http::TestHeaderMapImpl response_headers{{":status", "302"}, {"location", "hello"}};
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true));
Expand All @@ -2287,9 +2295,13 @@ TEST_F(RouterTest, RedirectFound) {

TEST_F(RouterTest, DirectResponse) {
NiceMock<MockDirectResponseEntry> direct_response;
std::string route_name("route-test-name");
EXPECT_CALL(direct_response, routeName()).WillOnce(ReturnRef(route_name));
EXPECT_CALL(direct_response, responseCode()).WillRepeatedly(Return(Http::Code::OK));
EXPECT_CALL(direct_response, responseBody()).WillRepeatedly(ReturnRef(EMPTY_STRING));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));
absl::string_view route_name_view(route_name);
EXPECT_CALL(callbacks_.stream_info_, setRouteName(route_name_view));

Http::TestHeaderMapImpl response_headers{{":status", "200"}};
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true));
Expand All @@ -2303,10 +2315,14 @@ TEST_F(RouterTest, DirectResponse) {

TEST_F(RouterTest, DirectResponseWithBody) {
NiceMock<MockDirectResponseEntry> direct_response;
std::string route_name("route-test-name");
EXPECT_CALL(direct_response, routeName()).WillOnce(ReturnRef(route_name));
EXPECT_CALL(direct_response, responseCode()).WillRepeatedly(Return(Http::Code::OK));
const std::string response_body("static response");
EXPECT_CALL(direct_response, responseBody()).WillRepeatedly(ReturnRef(response_body));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));
absl::string_view route_name_view(route_name);
EXPECT_CALL(callbacks_.stream_info_, setRouteName(route_name_view));

Http::TestHeaderMapImpl response_headers{
{":status", "200"}, {"content-length", "15"}, {"content-type", "text/plain"}};
Expand Down
5 changes: 5 additions & 0 deletions test/common/stream_info/test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ class TestStreamInfo : public StreamInfo::StreamInfo {
const Ssl::ConnectionInfo* downstreamSslConnection() const override {
return downstream_connection_info_;
}
void setRouteName(absl::string_view route_name) override {
route_name_ = std::string(route_name);
}
const std::string& getRouteName() const override { return route_name_; }

const Router::RouteEntry* routeEntry() const override { return route_entry_; }

Expand Down Expand Up @@ -198,6 +202,7 @@ class TestStreamInfo : public StreamInfo::StreamInfo {
uint64_t response_flags_{};
Upstream::HostDescriptionConstSharedPtr upstream_host_{};
bool health_check_request_{};
std::string route_name_;
Network::Address::InstanceConstSharedPtr upstream_local_address_;
Network::Address::InstanceConstSharedPtr downstream_local_address_;
Network::Address::InstanceConstSharedPtr downstream_direct_remote_address_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ TEST_F(HttpGrpcAccessLogTest, Marshalling) {
stream_info.addBytesSent(20);
stream_info.response_code_ = 200;
stream_info.response_code_details_ = "via_upstream";
absl::string_view route_name_view("route-name-test");
stream_info.setRouteName(route_name_view);
ON_CALL(stream_info, hasResponseFlag(StreamInfo::ResponseFlag::FaultInjected))
.WillByDefault(Return(true));

Expand Down Expand Up @@ -313,6 +315,7 @@ TEST_F(HttpGrpcAccessLogTest, Marshalling) {
upstream_cluster: "fake_cluster"
response_flags:
fault_injected: true
route_name: "route-name-test"
protocol_version: HTTP10
request:
scheme: "scheme_value"
Expand Down
1 change: 1 addition & 0 deletions test/mocks/router/mocks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ MockRouteEntry::MockRouteEntry() {
ON_CALL(*this, metadata()).WillByDefault(ReturnRef(metadata_));
ON_CALL(*this, upgradeMap()).WillByDefault(ReturnRef(upgrade_map_));
ON_CALL(*this, hedgePolicy()).WillByDefault(ReturnRef(hedge_policy_));
ON_CALL(*this, routeName()).WillByDefault(ReturnRef(route_name_));
}

MockRouteEntry::~MockRouteEntry() {}
Expand Down
Loading