From c4b986b623ca2a6b7f92253b5abe43abf6427c8c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 6 Aug 2020 17:40:54 +0100 Subject: [PATCH 1/6] Add health check API and add to all http listeners --- synapse/app/generic_worker.py | 6 +++++- synapse/app/homeserver.py | 5 ++++- synapse/rest/health.py | 28 ++++++++++++++++++++++++++++ tests/rest/test_health.py | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 synapse/rest/health.py create mode 100644 tests/rest/test_health.py diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 1a16d0b9f884..7957586d6916 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -123,6 +123,7 @@ from synapse.rest.client.v2_alpha.keys import KeyChangesServlet, KeyQueryServlet from synapse.rest.client.v2_alpha.register import RegisterRestServlet from synapse.rest.client.versions import VersionsRestServlet +from synapse.rest.health import HealthResource from synapse.rest.key.v2 import KeyApiV2Resource from synapse.server import HomeServer from synapse.storage.databases.main.censor_events import CensorEventsStore @@ -493,7 +494,10 @@ def _listen_http(self, listener_config: ListenerConfig): site_tag = listener_config.http_options.tag if site_tag is None: site_tag = port - resources = {} + + # We always include a health resource. + resources = {"/health": HealthResource()} + for res in listener_config.http_options.resources: for name in res.names: if name == "metrics": diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index d87a77718e18..98d0d14a124b 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -68,6 +68,7 @@ from synapse.replication.tcp.resource import ReplicationStreamProtocolFactory from synapse.rest import ClientRestResource from synapse.rest.admin import AdminRestResource +from synapse.rest.health import HealthResource from synapse.rest.key.v2 import KeyApiV2Resource from synapse.rest.well_known import WellKnownResource from synapse.server import HomeServer @@ -98,7 +99,9 @@ def _listener_http(self, config: HomeServerConfig, listener_config: ListenerConf if site_tag is None: site_tag = port - resources = {} + # We always include a health resource. + resources = {"/health": HealthResource()} + for res in listener_config.http_options.resources: for name in res.names: if name == "openid" and "federation" in res.names: diff --git a/synapse/rest/health.py b/synapse/rest/health.py new file mode 100644 index 000000000000..d439b9ed2842 --- /dev/null +++ b/synapse/rest/health.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from twisted.web.resource import Resource + + +class HealthResource(Resource): + """A resource that does nothing except return a 200 with a body of `OK`, + which can be used as a health check. + """ + + isLeaf = 1 + + def render_GET(self, request): + request.setHeader(b"Content-Type", b"text/plain") + return b"OK" diff --git a/tests/rest/test_health.py b/tests/rest/test_health.py new file mode 100644 index 000000000000..ada2dab4c41e --- /dev/null +++ b/tests/rest/test_health.py @@ -0,0 +1,34 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +from synapse.rest.health import HealthResource + +from tests import unittest + + +class HealthCheckTests(unittest.HomeserverTestCase): + def setUp(self): + super().setUp() + + # replace the JsonResource with a WellKnownResource + self.resource = HealthResource() + + def test_health(self): + request, channel = self.make_request("GET", "/health", shorthand=False) + self.render(request) + + self.assertEqual(request.code, 200) + self.assertEqual(channel.result["body"], b"OK") From 35dfba4923284cd841410792905b7098085de02d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 7 Aug 2020 11:04:46 +0100 Subject: [PATCH 2/6] Don't log /health requests --- synapse/http/site.py | 9 ++++++++- synapse/rest/health.py | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index f506152fea87..79a9229a264d 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -286,7 +286,9 @@ def _finished_processing(self): # the connection dropped) code += "!" - self.site.access_logger.info( + log_level = logging.INFO if self._should_log_request() else logging.DEBUG + self.site.access_logger.log( + log_level, "%s - %s - {%s}" " Processed request: %.3fsec/%.3fsec (%.3fsec, %.3fsec) (%.3fsec/%.3fsec/%d)" ' %sB %s "%s %s %s" "%s" [%d dbevts]', @@ -314,6 +316,11 @@ def _finished_processing(self): except Exception as e: logger.warning("Failed to stop metrics: %r", e) + def _should_log_request(self) -> bool: + """Whether we should log at INFO that we processed the request. + """ + return self.path != b"/health" + class XForwardedForRequest(SynapseRequest): def __init__(self, *args, **kw): diff --git a/synapse/rest/health.py b/synapse/rest/health.py index d439b9ed2842..0170950bf382 100644 --- a/synapse/rest/health.py +++ b/synapse/rest/health.py @@ -19,6 +19,9 @@ class HealthResource(Resource): """A resource that does nothing except return a 200 with a body of `OK`, which can be used as a health check. + + Note: `SynapseRequest._should_log_request` ensures that requests to + `/health` do not get logged at INFO. """ isLeaf = 1 From 511c153b471aa40adabd670ca5a4616808efde68 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 7 Aug 2020 11:07:22 +0100 Subject: [PATCH 3/6] Newsfile --- changelog.d/8048.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8048.feature diff --git a/changelog.d/8048.feature b/changelog.d/8048.feature new file mode 100644 index 000000000000..8521d1920e7f --- /dev/null +++ b/changelog.d/8048.feature @@ -0,0 +1 @@ +Add a `/health` endpoint to every configured HTTP listener that can be used as a health check endpoint by load balancers. From 880ec86da9c4439f9ea3d1f1ffeb8a4b9781facc Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 7 Aug 2020 13:14:36 +0100 Subject: [PATCH 4/6] Update tests/rest/test_health.py Co-authored-by: Patrick Cloke --- tests/rest/test_health.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/test_health.py b/tests/rest/test_health.py index ada2dab4c41e..2d021f656542 100644 --- a/tests/rest/test_health.py +++ b/tests/rest/test_health.py @@ -23,7 +23,7 @@ class HealthCheckTests(unittest.HomeserverTestCase): def setUp(self): super().setUp() - # replace the JsonResource with a WellKnownResource + # replace the JsonResource with a HealthResource. self.resource = HealthResource() def test_health(self): From 9e322989330947546b9c1d8fbdeda1bbda7bf6f7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 7 Aug 2020 13:17:48 +0100 Subject: [PATCH 5/6] Doc --- docs/reverse_proxy.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/reverse_proxy.md b/docs/reverse_proxy.md index 7bfb96eff623..59f7cf885bfb 100644 --- a/docs/reverse_proxy.md +++ b/docs/reverse_proxy.md @@ -139,3 +139,10 @@ client IP addresses are recorded correctly. Having done so, you can then use `https://matrix.example.com` (instead of `https://matrix.example.com:8448`) as the "Custom server" when connecting to Synapse from a client. + + +## Health check endpoint + +Synapse exposes a `/health` endpoint on every configured HTTP listener that +always returns 200 OK (and doesn't get logged) for use by reverse proxies as a +health check endpoint. From 0c430cd2b676fd095251aad078f12df30eae4b41 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 7 Aug 2020 13:57:56 +0100 Subject: [PATCH 6/6] Update docs/reverse_proxy.md Co-authored-by: Patrick Cloke --- docs/reverse_proxy.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reverse_proxy.md b/docs/reverse_proxy.md index 59f7cf885bfb..fd48ba0874c2 100644 --- a/docs/reverse_proxy.md +++ b/docs/reverse_proxy.md @@ -143,6 +143,6 @@ connecting to Synapse from a client. ## Health check endpoint -Synapse exposes a `/health` endpoint on every configured HTTP listener that -always returns 200 OK (and doesn't get logged) for use by reverse proxies as a -health check endpoint. +Synapse exposes a health check endpoint for use by reverse proxies. +Each configured HTTP listener has a `/health` endpoint which always returns +200 OK (and doesn't get logged).