From 2a3f3c7c284af44adde654cfee234fed1d366249 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 12 Sep 2022 17:54:01 -0500 Subject: [PATCH 1/6] Be able to correlate timeouts in reverse-proxy layer in front of Synapse (tag specific request headers) Fix https://github.com/matrix-org/synapse/issues/13685 --- docs/usage/configuration/config_documentation.md | 8 ++++++++ synapse/api/auth.py | 14 ++++++++++++++ synapse/config/tracer.py | 10 ++++++++++ 3 files changed, 32 insertions(+) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index cd546041b2d4..8823727024bb 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3612,6 +3612,11 @@ Sub-options include: * `jaeger_config`: Jaeger can be configured to sample traces at different rates. All configuration options provided by Jaeger can be set here. Jaeger's configuration is mostly related to trace sampling which is documented [here](https://www.jaegertracing.io/docs/latest/sampling/). +* `request_headers_to_tag`: A list of headers to extract from the request and + add to to the top-level servlet tracing span as tags. Useful when you're using + a reverse proxy service like Cloudflare to protect your Synapse instance in + order to correlate and match up requests that timed out at the Cloudflare + layer to the Synapse traces. Example configuration: ```yaml @@ -3629,6 +3634,9 @@ opentracing: param: 1 logging: false + + request_headers_to_tag: + - "cf-ray" ``` --- ## Workers ## diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 4a75eb6b21da..6ef29b201f17 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -173,6 +173,20 @@ async def get_user_by_req( parent_span.set_tag("device_id", requester.device_id) if requester.app_service is not None: parent_span.set_tag("appservice_id", requester.app_service.id) + + # Tag any headers that we need to extract from the request. This + # is useful to specify any headers that a reverse-proxy in front + # of Synapse may be sending to correlate and match up something + # in that layer to a Synapse trace. ex. when Cloudflare times + # out it gives a `cf-ray` header which we can also tag here to + # find the trace. + for header_key in self.hs.config.tracing.request_headers_to_tag: + headers = request.requestHeaders.getRawHeaders(header_key) + if len(headers): + parent_span.set_tag( + SynapseTags.REQUEST_HEADER_PREFIX + header_key, headers[0] + ) + return requester @cancellable diff --git a/synapse/config/tracer.py b/synapse/config/tracer.py index c19270c6c56c..6d750e397fdb 100644 --- a/synapse/config/tracer.py +++ b/synapse/config/tracer.py @@ -37,6 +37,16 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.force_tracing_for_users: Set[str] = set() + # A list of headers to extract from the request and add to to the + # top-level servlet tracing span as tags. Useful when you're using a + # reverse proxy service like Cloudflare to protect your Synapse instance + # in order to correlate and match up requests that timed out at the + # Cloudflare layer to the Synapse traces. + self.request_headers_to_tag = opentracing_config.get( + "request_headers_to_tag", + [], + ) + if not self.opentracer_enabled: return From 7b73d0d898012de43f625f84f9c68580f90fe718 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 12 Sep 2022 17:58:51 -0500 Subject: [PATCH 2/6] Add changelog --- changelog.d/13786.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13786.feature diff --git a/changelog.d/13786.feature b/changelog.d/13786.feature new file mode 100644 index 000000000000..e43d6a83afe1 --- /dev/null +++ b/changelog.d/13786.feature @@ -0,0 +1 @@ +Add `opentracing.request_headers_to_tag` config to specify which request headers extract and tag traces with in order to correlate timeouts in reverse-proxy layers in front of Synapse with traces. From e244ee513bb42ce1a342d966f07852d648fea7ec Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 12 Sep 2022 18:00:58 -0500 Subject: [PATCH 3/6] Add tag --- synapse/logging/opentracing.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index ca2735dd6dc0..0319ea72fcf6 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -303,6 +303,10 @@ class SynapseTags: # incoming HTTP request ID (as written in the logs) REQUEST_ID = "request_id" + # Tag a header from the incoming request. The name of the header should be + # appended to this prefix. + REQUEST_HEADER_PREFIX = "request_header." + # HTTP request tag (used to distinguish full vs incremental syncs, etc) REQUEST_TAG = "request_tag" From b52f92190239e38964bf7be0b4906ec3bbe415d0 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 12 Sep 2022 18:11:10 -0500 Subject: [PATCH 4/6] Better handling --- synapse/api/auth.py | 2 +- synapse/config/tracer.py | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 6ef29b201f17..0f3d2e2d7830 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -182,7 +182,7 @@ async def get_user_by_req( # find the trace. for header_key in self.hs.config.tracing.request_headers_to_tag: headers = request.requestHeaders.getRawHeaders(header_key) - if len(headers): + if headers is not None: parent_span.set_tag( SynapseTags.REQUEST_HEADER_PREFIX + header_key, headers[0] ) diff --git a/synapse/config/tracer.py b/synapse/config/tracer.py index 6d750e397fdb..4bcb16ab02ef 100644 --- a/synapse/config/tracer.py +++ b/synapse/config/tracer.py @@ -42,10 +42,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # reverse proxy service like Cloudflare to protect your Synapse instance # in order to correlate and match up requests that timed out at the # Cloudflare layer to the Synapse traces. - self.request_headers_to_tag = opentracing_config.get( - "request_headers_to_tag", - [], - ) + self.request_headers_to_tag = [] if not self.opentracer_enabled: return @@ -72,3 +69,13 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: ("opentracing", "force_tracing_for_users", f"index {i}"), ) self.force_tracing_for_users.add(u) + + request_headers_to_tag = opentracing_config.get( + "request_headers_to_tag", + [], + ) + if not isinstance(request_headers_to_tag, list): + raise ConfigError( + "Expected a list", ("opentracing", "request_headers_to_tag") + ) + self.request_headers_to_tag = request_headers_to_tag From 8aa4be5225ba5c3635ad465f668a56479f97f69c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 12 Sep 2022 18:16:50 -0500 Subject: [PATCH 5/6] Header name has more context --- synapse/api/auth.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 0f3d2e2d7830..8737e1e95f49 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -180,11 +180,11 @@ async def get_user_by_req( # in that layer to a Synapse trace. ex. when Cloudflare times # out it gives a `cf-ray` header which we can also tag here to # find the trace. - for header_key in self.hs.config.tracing.request_headers_to_tag: - headers = request.requestHeaders.getRawHeaders(header_key) + for header_name in self.hs.config.tracing.request_headers_to_tag: + headers = request.requestHeaders.getRawHeaders(header_name) if headers is not None: parent_span.set_tag( - SynapseTags.REQUEST_HEADER_PREFIX + header_key, headers[0] + SynapseTags.REQUEST_HEADER_PREFIX + header_name, headers[0] ) return requester From 1e4f918517d46719dec1e4eadc89ca71e06391ba Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 12 Sep 2022 18:18:09 -0500 Subject: [PATCH 6/6] Make sure it's a string --- synapse/api/auth.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 8737e1e95f49..09ebd847a528 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -184,7 +184,8 @@ async def get_user_by_req( headers = request.requestHeaders.getRawHeaders(header_name) if headers is not None: parent_span.set_tag( - SynapseTags.REQUEST_HEADER_PREFIX + header_name, headers[0] + SynapseTags.REQUEST_HEADER_PREFIX + header_name, + str(headers[0]), ) return requester