Be able to correlate timeouts in reverse-proxy layer in front of Synapse (pull request ID from header) (#13801)

Fix https://github.com/matrix-org/synapse/issues/13685

New config:

```diff
  listeners:
    - port: 8008
      tls: false
      type: http
      x_forwarded: true
+     request_id_header: "cf-ray"
      bind_addresses: ['::1', '127.0.0.1', '0.0.0.0']
```
This commit is contained in:
Eric Eastwood 2022-09-15 15:32:25 -05:00 committed by GitHub
parent 140af0cdb6
commit 5093cbf88d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 38 additions and 5 deletions

View File

@ -0,0 +1 @@
Add `listeners[x].request_id_header` config to specify which request header to extract and use as the request ID in order to correlate requests from a reverse-proxy.

View File

@ -45,6 +45,10 @@ listens to traffic on localhost. (Do not change `bind_addresses` to `127.0.0.1`
when using a containerized Synapse, as that will prevent it from responding when using a containerized Synapse, as that will prevent it from responding
to proxied traffic.) to proxied traffic.)
Optionally, you can also set
[`request_id_header`](../usage/configuration/config_documentation.md#listeners)
so that the server extracts and re-uses the same request ID format that the
reverse proxy is using.
## Reverse-proxy configuration examples ## Reverse-proxy configuration examples

View File

@ -434,7 +434,16 @@ Sub-options for each listener include:
* `tls`: set to true to enable TLS for this listener. Will use the TLS key/cert specified in tls_private_key_path / tls_certificate_path. * `tls`: set to true to enable TLS for this listener. Will use the TLS key/cert specified in tls_private_key_path / tls_certificate_path.
* `x_forwarded`: Only valid for an 'http' listener. Set to true to use the X-Forwarded-For header as the client IP. Useful when Synapse is * `x_forwarded`: Only valid for an 'http' listener. Set to true to use the X-Forwarded-For header as the client IP. Useful when Synapse is
behind a reverse-proxy. behind a [reverse-proxy](../../reverse_proxy.md).
* `request_id_header`: The header extracted from each incoming request that is
used as the basis for the request ID. The request ID is used in
[logs](../administration/request_log.md#request-log-format) and tracing to
correlate and match up requests. When unset, Synapse will automatically
generate sequential request IDs. This option is useful when Synapse is behind
a [reverse-proxy](../../reverse_proxy.md).
_Added in Synapse 1.68.0._
* `resources`: Only valid for an 'http' listener. A list of resources to host * `resources`: Only valid for an 'http' listener. A list of resources to host
on this port. Sub-options for each resource are: on this port. Sub-options for each resource are:

View File

@ -206,6 +206,7 @@ class HttpListenerConfig:
resources: List[HttpResourceConfig] = attr.Factory(list) resources: List[HttpResourceConfig] = attr.Factory(list)
additional_resources: Dict[str, dict] = attr.Factory(dict) additional_resources: Dict[str, dict] = attr.Factory(dict)
tag: Optional[str] = None tag: Optional[str] = None
request_id_header: Optional[str] = None
@attr.s(slots=True, frozen=True, auto_attribs=True) @attr.s(slots=True, frozen=True, auto_attribs=True)
@ -520,9 +521,11 @@ class ServerConfig(Config):
): ):
raise ConfigError("allowed_avatar_mimetypes must be a list") raise ConfigError("allowed_avatar_mimetypes must be a list")
self.listeners = [ listeners = config.get("listeners", [])
parse_listener_def(i, x) for i, x in enumerate(config.get("listeners", [])) if not isinstance(listeners, list):
] raise ConfigError("Expected a list", ("listeners",))
self.listeners = [parse_listener_def(i, x) for i, x in enumerate(listeners)]
# no_tls is not really supported any more, but let's grandfather it in # no_tls is not really supported any more, but let's grandfather it in
# here. # here.
@ -889,6 +892,9 @@ def read_gc_thresholds(
def parse_listener_def(num: int, listener: Any) -> ListenerConfig: def parse_listener_def(num: int, listener: Any) -> ListenerConfig:
"""parse a listener config from the config file""" """parse a listener config from the config file"""
if not isinstance(listener, dict):
raise ConfigError("Expected a dictionary", ("listeners", str(num)))
listener_type = listener["type"] listener_type = listener["type"]
# Raise a helpful error if direct TCP replication is still configured. # Raise a helpful error if direct TCP replication is still configured.
if listener_type == "replication": if listener_type == "replication":
@ -928,6 +934,7 @@ def parse_listener_def(num: int, listener: Any) -> ListenerConfig:
resources=resources, resources=resources,
additional_resources=listener.get("additional_resources", {}), additional_resources=listener.get("additional_resources", {}),
tag=listener.get("tag"), tag=listener.get("tag"),
request_id_header=listener.get("request_id_header"),
) )
return ListenerConfig(port, bind_addresses, listener_type, tls, http_config) return ListenerConfig(port, bind_addresses, listener_type, tls, http_config)

View File

@ -72,10 +72,12 @@ class SynapseRequest(Request):
site: "SynapseSite", site: "SynapseSite",
*args: Any, *args: Any,
max_request_body_size: int = 1024, max_request_body_size: int = 1024,
request_id_header: Optional[str] = None,
**kw: Any, **kw: Any,
): ):
super().__init__(channel, *args, **kw) super().__init__(channel, *args, **kw)
self._max_request_body_size = max_request_body_size self._max_request_body_size = max_request_body_size
self.request_id_header = request_id_header
self.synapse_site = site self.synapse_site = site
self.reactor = site.reactor self.reactor = site.reactor
self._channel = channel # this is used by the tests self._channel = channel # this is used by the tests
@ -172,7 +174,14 @@ class SynapseRequest(Request):
self._opentracing_span = span self._opentracing_span = span
def get_request_id(self) -> str: def get_request_id(self) -> str:
return "%s-%i" % (self.get_method(), self.request_seq) request_id_value = None
if self.request_id_header:
request_id_value = self.getHeader(self.request_id_header)
if request_id_value is None:
request_id_value = str(self.request_seq)
return "%s-%s" % (self.get_method(), request_id_value)
def get_redacted_uri(self) -> str: def get_redacted_uri(self) -> str:
"""Gets the redacted URI associated with the request (or placeholder if the URI """Gets the redacted URI associated with the request (or placeholder if the URI
@ -611,12 +620,15 @@ class SynapseSite(Site):
proxied = config.http_options.x_forwarded proxied = config.http_options.x_forwarded
request_class = XForwardedForRequest if proxied else SynapseRequest request_class = XForwardedForRequest if proxied else SynapseRequest
request_id_header = config.http_options.request_id_header
def request_factory(channel: HTTPChannel, queued: bool) -> Request: def request_factory(channel: HTTPChannel, queued: bool) -> Request:
return request_class( return request_class(
channel, channel,
self, self,
max_request_body_size=max_request_body_size, max_request_body_size=max_request_body_size,
queued=queued, queued=queued,
request_id_header=request_id_header,
) )
self.requestFactory = request_factory # type: ignore self.requestFactory = request_factory # type: ignore