Bugfixes

-------- - Fix a regression introduced in v1.2.0rc1 which led to incorrect labels on some prometheus metrics. ([\#5734](https://github.com/matrix-org/synapse/issues/5734)) -----BEGIN PGP SIGNATURE----- iQJHBAABCgAxFiEEgQG31Z317NrSMt0QiISIDS7+X/QFAl04Ur0THGFuZHJld0Bh bW9yZ2FuLnh5egAKCRCIhIgNLv5f9F4oD/0TY6S/SEd2uAmzor64ojmbX5BOwPzf j/wzUTrfvuf40EvkNPDpnejNZSvy/ysbaGQaQusv0SQKlV3xrvdn4RuMvnOWVWck kBsO+lvzOaUTR0KHDxN4y9F5eI2NdPbub4847PPVzyqSIHAd+kolxXS8kSBBhwpL yfaICWV/AOy5L7xN+JZ9IQpnegVAvUj5DmgXzDHd6VdeiHDVJuARaBgrR5uCkwVS ZoLRqZ95XV/qiguMAUvPOwyEqht2mwO64989MswP16YYm8oMkB5QA6I5nYnACsTP qk9YcN/oNvEfQXUhttku6MxK1/4yUMPUhEoDBDH7ebc0440QDtWN+IHTdA6oPVZB IuStL9YGY16m7Ltx37ZUA4URfNMiSeLHo3zKc/mCAcwxN4HyOjJewtxbG5zKQAOZ SMs8UcDwGR4zL1hnt8ZDNYtWwfzJBQIdGjoHvjXJEY7/1csTv2lmAwewFTXiqSAr 30GW5ews94kotqBK53zZT6V0F5gHNqgGHniOz1ZpqLLxYLqO3LSAGe97CrqlWUdX GkhA9tZyweknociD9fyyBmKdcFJ4mL4a+oGI5CMnSMph8UvCY8Y5XMb1T+iYEABI tA9G3mBvgkLPj+5V+8QggNkBafSigW2Q4FX7enGsDmiiskZOtfeKrAcVkapD4ooi 3I7IW5aetZr2IQ== =+JBn -----END PGP SIGNATURE----- Merge tag 'v1.2.0rc2' into develop Bugfixes -------- - Fix a regression introduced in v1.2.0rc1 which led to incorrect labels on some prometheus metrics. ([\#5734](https://github.com/matrix-org/synapse/issues/5734))
2024-12-11 08:04:20 -05:00 · 2019-07-24 13:47:51 +01:00 · 2019-07-24 13:47:51 +01:00 · baf081cd3b
commit baf081cd3b
parent 2276936bac 2d573e2e2b
11 changed files with 101 additions and 32 deletions
--- a/CHANGES.md
+++ b/CHANGES.md
@ -1,3 +1,12 @@
+Synapse 1.2.0rc2 (2019-07-24)
+=============================
+
+Bugfixes
+--------
+
+- Fix a regression introduced in v1.2.0rc1 which led to incorrect labels on some prometheus metrics. ([\#5734](https://github.com/matrix-org/synapse/issues/5734))
+
+
 Synapse 1.2.0rc1 (2019-07-22)
 =============================

--- a/synapse/init.py
+++ b/synapse/init.py
@ -35,4 +35,4 @@ try:
 except ImportError:
    pass

-__version__ = "1.2.0rc1"
+__version__ = "1.2.0rc2"
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@ -325,7 +325,9 @@ class BaseFederationServlet(object):
            if code is None:
                continue

-            server.register_paths(method, (pattern,), self._wrap(code))
+            server.register_paths(
+                method, (pattern,), self._wrap(code), self.__class__.__name__
+            )


 class FederationSendServlet(BaseFederationServlet):
--- a/synapse/http/server.py
+++ b/synapse/http/server.py
@ -245,7 +245,9 @@ class JsonResource(HttpServer, resource.Resource):

    isLeaf = True

-    _PathEntry = collections.namedtuple("_PathEntry", ["pattern", "callback"])
+    _PathEntry = collections.namedtuple(
+        "_PathEntry", ["pattern", "callback", "servlet_classname"]
+    )

    def __init__(self, hs, canonical_json=True):
        resource.Resource.__init__(self)
@ -255,12 +257,28 @@ class JsonResource(HttpServer, resource.Resource):
        self.path_regexs = {}
        self.hs = hs

-    def register_paths(self, method, path_patterns, callback):
+    def register_paths(self, method, path_patterns, callback, servlet_classname):
+        """
+        Registers a request handler against a regular expression. Later request URLs are
+        checked against these regular expressions in order to identify an appropriate
+        handler for that request.
+
+        Args:
+            method (str): GET, POST etc
+
+            path_patterns (Iterable[str]): A list of regular expressions to which
+                the request URLs are compared.
+
+            callback (function): The handler for the request. Usually a Servlet
+
+            servlet_classname (str): The name of the handler to be used in prometheus
+                and opentracing logs.
+        """
        method = method.encode("utf-8")  # method is bytes on py3
        for path_pattern in path_patterns:
            logger.debug("Registering for %s %s", method, path_pattern.pattern)
            self.path_regexs.setdefault(method, []).append(
-                self._PathEntry(path_pattern, callback)
+                self._PathEntry(path_pattern, callback, servlet_classname)
            )

    def render(self, request):
@ -275,13 +293,9 @@ class JsonResource(HttpServer, resource.Resource):
            This checks if anyone has registered a callback for that method and
            path.
        """
-        callback, group_dict = self._get_handler_for_request(request)
+        callback, servlet_classname, group_dict = self._get_handler_for_request(request)

-        servlet_instance = getattr(callback, "__self__", None)
-        if servlet_instance is not None:
-            servlet_classname = servlet_instance.__class__.__name__
-        else:
-            servlet_classname = "%r" % callback
+        # Make sure we have a name for this handler in prometheus.
        request.request_metrics.name = servlet_classname

        # Now trigger the callback. If it returns a response, we send it
@ -311,7 +325,8 @@ class JsonResource(HttpServer, resource.Resource):
            request (twisted.web.http.Request):

        Returns:
-            Tuple[Callable, dict[unicode, unicode]]: callback method, and the
+            Tuple[Callable, str, dict[unicode, unicode]]: callback method, the
+                label to use for that method in prometheus metrics, and the
                dict mapping keys to path components as specified in the
                handler's path match regexp.

@ -320,7 +335,7 @@ class JsonResource(HttpServer, resource.Resource):
                None, or a tuple of (http code, response body).
        """
        if request.method == b"OPTIONS":
-            return _options_handler, {}
+            return _options_handler, "options_request_handler", {}

        # Loop through all the registered callbacks to check if the method
        # and path regex match
@ -328,10 +343,10 @@ class JsonResource(HttpServer, resource.Resource):
            m = path_entry.pattern.match(request.path.decode("ascii"))
            if m:
                # We found a match!
-                return path_entry.callback, m.groupdict()
+                return path_entry.callback, path_entry.servlet_classname, m.groupdict()

        # Huh. No one wanted to handle that? Fiiiiiine. Send 400.
-        return _unrecognised_request_handler, {}
+        return _unrecognised_request_handler, "unrecognised_request_handler", {}

    def _send_response(
        self, request, code, response_json_object, response_code_message=None
--- a/synapse/http/servlet.py
+++ b/synapse/http/servlet.py
@ -290,11 +290,13 @@ class RestServlet(object):

            for method in ("GET", "PUT", "POST", "OPTIONS", "DELETE"):
                if hasattr(self, "on_%s" % (method,)):
+                    servlet_classname = self.__class__.__name__
                    method_handler = getattr(self, "on_%s" % (method,))
                    http_server.register_paths(
                        method,
                        patterns,
-                        trace_servlet(self.__class__.__name__, method_handler),
+                        trace_servlet(servlet_classname, method_handler),
+                        servlet_classname,
                    )

        else:
--- a/synapse/replication/http/_base.py
+++ b/synapse/replication/http/_base.py
@ -205,7 +205,7 @@ class ReplicationEndpoint(object):
        args = "/".join("(?P<%s>[^/]+)" % (arg,) for arg in url_args)
        pattern = re.compile("^/_synapse/replication/%s/%s$" % (self.NAME, args))

-        http_server.register_paths(method, [pattern], handler)
+        http_server.register_paths(method, [pattern], handler, self.__class__.__name__)

    def _cached_handler(self, request, txn_id, **kwargs):
        """Called on new incoming requests when caching is enabled. Checks
--- a/synapse/rest/admin/server_notice_servlet.py
+++ b/synapse/rest/admin/server_notice_servlet.py
@ -59,9 +59,14 @@ class SendServerNoticeServlet(RestServlet):

    def register(self, json_resource):
        PATTERN = "^/_synapse/admin/v1/send_server_notice"
-        json_resource.register_paths("POST", (re.compile(PATTERN + "$"),), self.on_POST)
        json_resource.register_paths(
-            "PUT", (re.compile(PATTERN + "/(?P<txn_id>[^/]*)$"),), self.on_PUT
+            "POST", (re.compile(PATTERN + "$"),), self.on_POST, self.__class__.__name__
+        )
+        json_resource.register_paths(
+            "PUT",
+            (re.compile(PATTERN + "/(?P<txn_id>[^/]*)$"),),
+            self.on_PUT,
+            self.__class__.__name__,
        )

    @defer.inlineCallbacks
--- a/synapse/rest/client/v1/room.py
+++ b/synapse/rest/client/v1/room.py
@ -67,11 +67,17 @@ class RoomCreateRestServlet(TransactionRestServlet):
        register_txn_path(self, PATTERNS, http_server)
        # define CORS for all of /rooms in RoomCreateRestServlet for simplicity
        http_server.register_paths(
-            "OPTIONS", client_patterns("/rooms(?:/.*)?$", v1=True), self.on_OPTIONS
+            "OPTIONS",
+            client_patterns("/rooms(?:/.*)?$", v1=True),
+            self.on_OPTIONS,
+            self.__class__.__name__,
        )
        # define CORS for /createRoom[/txnid]
        http_server.register_paths(
-            "OPTIONS", client_patterns("/createRoom(?:/.*)?$", v1=True), self.on_OPTIONS
+            "OPTIONS",
+            client_patterns("/createRoom(?:/.*)?$", v1=True),
+            self.on_OPTIONS,
+            self.__class__.__name__,
        )

    def on_PUT(self, request, txn_id):
@ -116,16 +122,28 @@ class RoomStateEventRestServlet(TransactionRestServlet):
        )

        http_server.register_paths(
-            "GET", client_patterns(state_key, v1=True), self.on_GET
+            "GET",
+            client_patterns(state_key, v1=True),
+            self.on_GET,
+            self.__class__.__name__,
        )
        http_server.register_paths(
-            "PUT", client_patterns(state_key, v1=True), self.on_PUT
+            "PUT",
+            client_patterns(state_key, v1=True),
+            self.on_PUT,
+            self.__class__.__name__,
        )
        http_server.register_paths(
-            "GET", client_patterns(no_state_key, v1=True), self.on_GET_no_state_key
+            "GET",
+            client_patterns(no_state_key, v1=True),
+            self.on_GET_no_state_key,
+            self.__class__.__name__,
        )
        http_server.register_paths(
-            "PUT", client_patterns(no_state_key, v1=True), self.on_PUT_no_state_key
+            "PUT",
+            client_patterns(no_state_key, v1=True),
+            self.on_PUT_no_state_key,
+            self.__class__.__name__,
        )

    def on_GET_no_state_key(self, request, room_id, event_type):
@ -845,18 +863,23 @@ def register_txn_path(servlet, regex_string, http_server, with_get=False):
        with_get: True to also register respective GET paths for the PUTs.
    """
    http_server.register_paths(
-        "POST", client_patterns(regex_string + "$", v1=True), servlet.on_POST
+        "POST",
+        client_patterns(regex_string + "$", v1=True),
+        servlet.on_POST,
+        servlet.__class__.__name__,
    )
    http_server.register_paths(
        "PUT",
        client_patterns(regex_string + "/(?P<txn_id>[^/]*)$", v1=True),
        servlet.on_PUT,
+        servlet.__class__.__name__,
    )
    if with_get:
        http_server.register_paths(
            "GET",
            client_patterns(regex_string + "/(?P<txn_id>[^/]*)$", v1=True),
            servlet.on_GET,
+            servlet.__class__.__name__,
        )


--- a/synapse/rest/client/v2_alpha/relations.py
+++ b/synapse/rest/client/v2_alpha/relations.py
@ -72,11 +72,13 @@ class RelationSendServlet(RestServlet):
            "POST",
            client_patterns(self.PATTERN + "$", releases=()),
            self.on_PUT_or_POST,
+            self.__class__.__name__,
        )
        http_server.register_paths(
            "PUT",
            client_patterns(self.PATTERN + "/(?P<txn_id>[^/]*)$", releases=()),
            self.on_PUT,
+            self.__class__.__name__,
        )

    def on_PUT(self, request, *args, **kwargs):
--- a/tests/test_server.py
+++ b/tests/test_server.py
@ -61,7 +61,10 @@ class JsonResourceTests(unittest.TestCase):

        res = JsonResource(self.homeserver)
        res.register_paths(
-            "GET", [re.compile("^/_matrix/foo/(?P<room_id>[^/]*)$")], _callback
+            "GET",
+            [re.compile("^/_matrix/foo/(?P<room_id>[^/]*)$")],
+            _callback,
+            "test_servlet",
        )

        request, channel = make_request(
@ -82,7 +85,9 @@ class JsonResourceTests(unittest.TestCase):
            raise Exception("boo")

        res = JsonResource(self.homeserver)
-        res.register_paths("GET", [re.compile("^/_matrix/foo$")], _callback)
+        res.register_paths(
+            "GET", [re.compile("^/_matrix/foo$")], _callback, "test_servlet"
+        )

        request, channel = make_request(self.reactor, b"GET", b"/_matrix/foo")
        render(request, res, self.reactor)
@ -105,7 +110,9 @@ class JsonResourceTests(unittest.TestCase):
            return make_deferred_yieldable(d)

        res = JsonResource(self.homeserver)
-        res.register_paths("GET", [re.compile("^/_matrix/foo$")], _callback)
+        res.register_paths(
+            "GET", [re.compile("^/_matrix/foo$")], _callback, "test_servlet"
+        )

        request, channel = make_request(self.reactor, b"GET", b"/_matrix/foo")
        render(request, res, self.reactor)
@ -122,7 +129,9 @@ class JsonResourceTests(unittest.TestCase):
            raise SynapseError(403, "Forbidden!!one!", Codes.FORBIDDEN)

        res = JsonResource(self.homeserver)
-        res.register_paths("GET", [re.compile("^/_matrix/foo$")], _callback)
+        res.register_paths(
+            "GET", [re.compile("^/_matrix/foo$")], _callback, "test_servlet"
+        )

        request, channel = make_request(self.reactor, b"GET", b"/_matrix/foo")
        render(request, res, self.reactor)
@ -143,7 +152,9 @@ class JsonResourceTests(unittest.TestCase):
            self.fail("shouldn't ever get here")

        res = JsonResource(self.homeserver)
-        res.register_paths("GET", [re.compile("^/_matrix/foo$")], _callback)
+        res.register_paths(
+            "GET", [re.compile("^/_matrix/foo$")], _callback, "test_servlet"
+        )

        request, channel = make_request(self.reactor, b"GET", b"/_matrix/foobar")
        render(request, res, self.reactor)
--- a/tests/utils.py
+++ b/tests/utils.py
@ -471,7 +471,7 @@ class MockHttpResource(HttpServer):

        raise KeyError("No event can handle %s" % path)

-    def register_paths(self, method, path_patterns, callback):
+    def register_paths(self, method, path_patterns, callback, servlet_name):
        for path_pattern in path_patterns:
            self.callbacks.append((method, path_pattern, callback))