Open mkeeler opened 2 years ago
Hi, @mkeeler , I am interested in implementing this feature and would like to give it a shot. Could you help clarify the following questions regarding the new feature?
local_request_timeout_ms
and local_connect_timeout_ms
should be in the same level as MaxInboundConnections. So the modified struct will look liketype ServiceConfigEntry struct {
// omitted fields
MaxInboundConnections int
+ LocalConnectTimeOut int
+ LocalRequestTimeOut int
}
What do you think?
It seems that most of the dynamic configurations has already been added to the service-default. Is there any other fields we need to consider with these 2?
The fields appear in the service definition as follows (in the order of precedence):
service {
name = "B"
id = "B-id-1"
connect {
sidecar_service {
proxy {
config {
max_inbound_connections = 100
local_connect_timeout_ms = 20000 // 20s; override the timemout from service-defaults
local_request_timeout_ms = 60000 // 60s; override the timemout from service-defaults
}
upstreams = [
{
destination_name = "C"
config {
limits {
max_connections = 200
}
}
}
]
}
}
}
Kind = "service-defaults"
Name = "B"
Config {
local_connect_timeout_ms = 10000 // 10s; change default local_connect_timeout_ms
local_request_timeout_ms = 20000 // 20s; change default local_request_timeout_ms
}
UpstreamConfig = {
Defaults = {
Limits = {
MaxConnections = 512
MaxPendingRequests = 512
MaxConcurrentRequests = 512
}
}
}
Kind = "proxy-defaults"
Name = "global"
Config {
local_connect_timeout_ms = 1000
local_request_timeout_ms = 20000
max_connections = 200
}
Thanks in advance.
Feature Description
Today you can configure Envoy proxy configuration both within a service instance and in the
proxy-defaults/global
configuration entry. Theprotocol
configuration can also be set within theservice-defaults
configuration entry. Other configurations likelocal_request_timeout_ms
andlocal_connect_timeout_ms
cannot be set here.The feature request is to support setting some of these configurations within the
service-defaults
config entry. However some others such as thebind_address
probably don't make sense to set centrally so its not quite so simple as just merge an extra config source.Use Case(s)
I encountered a gRPC API where the local app could take several minutes to return data for a specific API. I had wanted to set
local_request_timeout_ms
for all instances of the service but it wasn't possible. I had to resort to configuring by modifying all instances of the service.Food for Thought
If an application is configured with a
local_request_timeout_ms
value, would it be a good idea to auto-propagate this timeout to consumers/downstreams of that service. That way we don't have to configure the timeout in 2 different places. Maybe downstreams would want to timeout sooner. In that case they could specify the timeout and this would just be a way to modify the default timeout value.