Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose celery queue metrics #1275

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

HTRafal
Copy link

@HTRafal HTRafal commented Apr 14, 2023

This was tested by running flower locally and letting it read metrics from a real broker.

This is a draft / proposal. I'm open for suggestions on how to implement it in a better way.

Implements #1173

queue_names = set([capp.conf.CELERY_DEFAULT_QUEUE]) | \
set([q.name for q in capp.conf.CELERY_QUEUES or [] if q.name])

queues = yield broker.queues(sorted(queue_names))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The website hangs a bit sometimes. This might be running in the main thread. Looking for ways to fix it. Just spawn a new Thread?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably fixed with @tornado.concurrent.run_on_executor (https://mike.depalatis.net/blog/tornado-background-tasks.html)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed for good this time:

  • moved update metrics loop to a while loop with asyncio sleep
  • fixed Redis broker calls to be run in ThreadPoolExecutor instead of main thread

@HTRafal HTRafal force-pushed the queue_length_metric branch 3 times, most recently from 924d2f1 to 1f2f7c0 Compare April 18, 2023 08:32
@HTRafal HTRafal changed the title Expose queue length as a prometheus metric Expose celery queue metrics Apr 18, 2023
@HTRafal
Copy link
Author

HTRafal commented Apr 18, 2023

Alternative approach is to keep it simple and expose just the queue lengths without number of online workers. The last commit seems the most hacky. I'm ok with removing it - commit / draft PR without online workers

@HTRafal
Copy link
Author

HTRafal commented Jun 12, 2023

While this was open version 2.0 was released and I got a lot of conflicts, so I resolved all of them and squashed everything to a single commit.

@HTRafal HTRafal closed this Jun 12, 2023
@HTRafal HTRafal reopened this Jun 12, 2023
mher and others added 4 commits June 17, 2023 18:44
Implements mher#1173

* Quick fix synchronous redis calls.
  Redis broker calls were synchronous, so blocking the main thread. This
  moves them to ThreadPoolExecutor
* Update workers list regularly for up-to-date metrics
  Previously self.workers was only updated only on UI events. This is no
  longer fine as we use the data to produce up-to-date metrics
@arrdem
Copy link

arrdem commented Nov 7, 2023

Hey folks, we've been running a patched flower specifically for this feature for a minute now. Is there anything we can do to support getting this merged in?

@MatthiasVervaet
Copy link

Could this pull request be merged?

@FMiranda97
Copy link

@mher can we have this one merged please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants