~sircmpwn/sr.ht-dev

meta.sr.ht: Add webhook queue monitoring v1 APPLIED

Ignas Kiela: 1
 Add webhook queue monitoring
Julien Moutinho: 1
 Fix Unix socket support in RedisQueueCollector

 3 files changed, 9 insertions(+), 2 deletions(-)
#644698 alpine.yml success
#644699 archlinux.yml success
#644700 debian.yml success
We should probably patch Celery to use standard URLs. Anyone up for it?
So, I've given a try to patch Celery and Kombu,
but the parsing code is hard to follow and anyway I don't see a way
to handle unix:// without reserving it to the Redis transport,
but that does not seems upstreamable because Kombu supports
other transports which may also be reachable by a Unix socket.
So this looks like a dead end to me.

I've then turned back to patching sourcehut,
because AFAICS RedisQueueCollector is only used on Celery's broker URL,
hence it makes sense to use Celery's parser instead of Redis'.
That's what the following patch does.

I've tested that the webhooks service now starts with URL like:
redis+socket:///run/redis-sourcehut-todosrht/redis.sock?virtual_host=1
However I've not tested the actual collection done by RedisQueueCollector,
only that ResponseError is still raised when the key is not a list,
as the current comment says.

Regards,
Rather than import celery for this, I would prefer to write a small
function ourselves which rewrites the URL into a format appropriate for
Redis.from_url.
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~sircmpwn/sr.ht-dev/patches/27192/mbox | git am -3
Learn more about email & git

[PATCH meta.sr.ht] Add webhook queue monitoring Export this patch

---
Depends on the core.sr.ht patch

 metasrht/app.py      | 3 +++
 metasrht/webhooks.py | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/metasrht/app.py b/metasrht/app.py
index 89c59bc..b190875 100644
--- a/metasrht/app.py
+++ b/metasrht/app.py
@@ -49,6 +49,9 @@ class MetaApp(SrhtFlask):
            from metasrht.blueprints.billing import billing
            self.register_blueprint(billing)

        from metasrht.webhooks import webhook_metrics_collector
        self.metrics_registry.register(webhook_metrics_collector)

        @self.context_processor
        def inject():
            return {
diff --git a/metasrht/webhooks.py b/metasrht/webhooks.py
index 3f0ba01..3e1149e 100644
--- a/metasrht/webhooks.py
+++ b/metasrht/webhooks.py
@@ -7,8 +7,11 @@ if not hasattr(db, "session"):
    db.init()
from srht.webhook import Event
from srht.webhook.celery import CeleryWebhook, make_worker
from srht.metrics import RedisQueueCollector

worker = make_worker(broker=cfg("meta.sr.ht", "webhooks", "redis://"))
webhook_broker = cfg("meta.sr.ht", "webhooks", "redis://")
worker = make_worker(broker=webhook_broker)
webhook_metrics_collector = RedisQueueCollector(webhook_broker, "srht_webhooks", "Webhook queue length")

class UserWebhook(CeleryWebhook):
    events = [
-- 
2.25.1
meta.sr.ht/patches: SUCCESS in 3m42s

[Add webhook queue monitoring][0] from [Ignas Kiela][1]

[0]: https://lists.sr.ht/~sircmpwn/sr.ht-dev/patches/27192
[1]: mailto:me@ignaskiela.eu

✓ #644700 SUCCESS meta.sr.ht/patches/debian.yml    https://builds.sr.ht/~sircmpwn/job/644700
✓ #644699 SUCCESS meta.sr.ht/patches/archlinux.yml https://builds.sr.ht/~sircmpwn/job/644699
✓ #644698 SUCCESS meta.sr.ht/patches/alpine.yml    https://builds.sr.ht/~sircmpwn/job/644698
Thanks!

To git@git.sr.ht:~sircmpwn/meta.sr.ht
   38c8c76..9931df3  master -> master
Le mer. 08 déc. 2021 10h26 +0200, Ignas Kiela a écrit :

[PATCH core.sr.ht] Fix Unix socket support in RedisQueueCollector Export this patch

The broker URL is not necessarily in the format expected by Redis.from_url

Especially, Redis.from_url supports this format for Unix sockets:
    unix:///run/redis-sourcehut-metasrht/redis.sock?db=0
See https://redis-py.readthedocs.io/en/stable/#redis.ConnectionPool.from_url

Whereas Celery+Kombu support Redis but also other transports
and thus expect another scheme:
    redis+socket:///run/redis-sourcehut-metasrht/redis.sock?virtual_host=1
See https://docs.celeryproject.org/en/stable/userguide/configuration.html#redis-backend-settings
and https://github.com/celery/celery/blob/e5d99801e4b56a02af4a2e183879c767228d2817/celery/backends/redis.py#L299-L352
and https://github.com/celery/kombu/blob/master/kombu/utils/url.py
---
 srht/metrics.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/srht/metrics.py b/srht/metrics.py
index 68caf8e..2df5777 100644
--- a/srht/metrics.py
+++ b/srht/metrics.py
@@ -1,11 +1,12 @@
import time
from celery import Celery
from prometheus_client.metrics_core import GaugeMetricFamily
from redis import Redis, ResponseError


class RedisQueueCollector:
    def __init__(self, broker, name, documentation, queue_name="celery"):
        self.redis = Redis.from_url(broker)
        self.redis = Celery("collector", broker=broker).connection_for_read().channel().client
        self.queue_name = queue_name
        self.name = name
        self.documentation = documentation
-- 
2.35.1
Rather than import celery for this, I would prefer to write a small
function ourselves which rewrites the URL into a format appropriate for
Redis.from_url.