Add Prometheus metric for sygnal_inflight_request_limit_drop #146
Conversation
| @@ -142,6 +144,13 @@ class ConcurrencyLimitedPushkin(Pushkin): | |||
|
|
|||
| UNDERSTOOD_CONFIG_FIELDS = {"inflight_request_limit"} | |||
|
|
|||
| RATELIMITING_DROPPED_REQUESTS = Counter( | |||
There was a problem hiding this comment.
question: if there are two pushkins (say, for GCM and APNS), do they both correctly refer to the same instance of Counter ? or do they each get their own instance (which will probably confuse the prometheus lib)?
(I can never quite remember how class-level attributes work)
There was a problem hiding this comment.
They should get the same one. I wasn't 100% sure that's what was wanted, but I think it is.
There was a problem hiding this comment.
My understanding was/is that the class attribute is initialised when the class declaration is executed, and that attribute stays with the class.
Note it is always referred to as ConcurrencyLimitedPushkin.RATELIMITING_DROPPED_REQUESTS, so it would always act like a namespaced global.
I didn't know whether subclasses would get access to it (not that it matters, since we access the field statically every time and not through self or cls), so I tried it:
oli@bbm-neon:~$ python3
Python 3.6.9 (default, Jul 17 2020, 12:50:27)
[GCC 8.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> class X:
... bob = []
...
>>> class Y(X):
... pass
...
>>> class Z(X):
... pass
...
>>> id(Y.bob)
140481390301896
>>> id(X.bob)
140481390301896
>>> id(Z.bob)
140481390301896There was a problem hiding this comment.
(and to answer the question, yes this is intended — the pushkin is distinguished by the pushkin label, as this is what Prometheus understands)
There was a problem hiding this comment.
In your example, even doing the following should return the same thing:
id(Y().bob)Doesn't matter if you access it via the class name or not (although I think it is clearer to access class-level variables that way).
There was a problem hiding this comment.
In your example, even doing the following should return the same thing:
id(Y().bob)Doesn't matter if you access it via the class name or not (although I think it is clearer to access class-level variables that way).
Yes you are right (and I suspected so), but my experience from Java is that 'access static vars by class name, even if you can do it by instance' :)
Features -------- - Add the ability to configure custom FCM options, which is necessary for using iOS with Firebase. ([\matrix-org#145](matrix-org#145)) - Add a Prometheus metric (`sygnal_inflight_request_limit_drop`) that shows the number of notifications dropped due to exceeding the in-flight concurrent request limit. ([\matrix-org#146](matrix-org#146)) Change-Id: Ia78c1a0c85765c29343a986ac7e7e93ca83b4b83
No description provided.