danihodovic / celery-exporter

A Prometheus exporter for Celery metrics
MIT License
399 stars 87 forks source link

feat: Add custom accept content configuration #186

Closed amandahla closed 1 year ago

amandahla commented 1 year ago

Reference: https://github.com/danihodovic/celery-exporter/issues/153

Problem: celery-exporter expects default Celery configuration.

Proposal: New custom-config parameter allowing configuration load from a module.

danihodovic commented 1 year ago

Thanks for the contribution.

I prefer to make the exporter configurable with environment variables. This makes it easier to run it in cloud environments (e.g not necessary to provision config maps in K8s). What do you think? How are you deploying the exporter?

Also see https://12factor.net/config

amandahla commented 1 year ago

Thanks for the contribution.

I prefer to make the exporter configurable with environment variables. This makes it easier to run it in cloud environments (e.g not necessary to provision config maps in K8s). What do you think? How are you deploying the exporter?

Also see https://12factor.net/config

Hi, thanks for the quick answer.

Other option would be use https://docs.celeryq.dev/en/stable/reference/celery.html?highlight=environment%20variable#celery.Celery.config_from_envvar but it works expecting a new module as well. What do you think?

Update: Or we can do as the application that I'm working with: https://github.com/indico/indico/blob/master/indico/core/celery/core.py#L70

New config options are set in an env var and then the config is updated.

danihodovic commented 1 year ago

If we're only looking to resolve #153, why don't we add a click parameter / environment variable for CELERY_ACCEPT_CONTENT and pass in a dictionary to config_from_object?

self.app.config_from_object(dict(
    accept_content=accept_content
))

Another option is to take any CELERY_ prefixed environment variable and inject it into config_from_object. I think Django does something similar: https://docs.celeryq.dev/en/stable/django/first-steps-with-django.html#id1

amandahla commented 1 year ago

I didn't test. Would "accept_content" be enough to fix that issue? If positive, then a env var CELERY_ACCEPT_CONTENT would be great (and easier to do).

danihodovic commented 1 year ago

I didn't test. Would "accept_content" be enough to fix that issue? If positive, then a env var CELERY_ACCEPT_CONTENT would be great (and easier to do).

I suspect it would.

You can test it by rebuilding the image docker build . -t amandahla/celery-exporter and pushing it to docker hub. Deploy it from there and see if it resolves your problem.

amandahla commented 1 year ago

Local test ran perfectly! :) Example:

export CE_ACCEPT_CONTENT="json,pickle"
danihodovic commented 1 year ago

Released as 0.5.8. https://hub.docker.com/layers/danihodovic/celery-exporter/0.5.8/images/sha256-de42c4dd5db867bde3761f808e7223e16bc74c0042e97b6042d3509a68605b60?context=explore