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

Add queue information to task_enqueued log #341

Closed
badziyoussef opened this issue Oct 14, 2023 · 5 comments · Fixed by #345
Closed

Add queue information to task_enqueued log #341

badziyoussef opened this issue Oct 14, 2023 · 5 comments · Fixed by #345
Assignees

Comments

@badziyoussef
Copy link
Contributor

Is there a way to add queue information to this log?

    logger.info( 
         "task_enqueued", 
         child_task_id=headers.get("id") if headers else body.get("id"), 
         child_task_name=headers.get("task") if headers else body.get("task"), 
     )

Something like

    logger.info( 
         "task_enqueued", 
         child_task_id=headers.get("id") if headers else body.get("id"), 
         child_task_name=headers.get("task") if headers else body.get("task"), 
         priority=,
         queue=
     )
@jrobichaud
Copy link
Owner

With the current state of the code it is not possible but this is something we could consider adding.

I read more on the subject:
https://docs.celeryq.dev/en/stable/userguide/routing.html

There are multiple things to consider:

  • there are differences in queues configuration by broker (redis vs amqp)
  • what to do with routing_key
  • priority is not the only queue_order_strategy so it may be different
  • not everyone use queues/priorities
  • queue could be determined by routing and not explicitly set
  • some projects may not want to log anything about queue.
  • queue could be either logged only at the enqueue or bound with each log of the task

My approach would be
to:

  • detect if there are queues in app.conf.task_queues to activate queue logging
  • if so add meta at enqueue: child_task_queue (it is to avoid conflict if someone bind task_queue in a nested task)
  • add signal/configuration at task enqueue to be able to add more meta
  • make sure it is possible optionally to bind task_queue when task starts.

What do you think?

@badziyoussef
Copy link
Contributor Author

badziyoussef commented Oct 15, 2023

Yes, I completly agree with your analysis, also as a quick thing, adding properties to modify_context_before_task_publish signal to be like that

signals.modify_context_before_task_publish.send(
        sender=receiver_before_task_publish, context=context, properties=kwargs.get('properties', {})
    )

will solve my issue.

You think that could be a good solution?

@jrobichaud
Copy link
Owner

jrobichaud commented Oct 16, 2023

I started to prototype it but I am going to need to play more with celery queues before finalizing it.

Adding the properties will maybe solve your problem but I would like to be able to add to task_enqueued the name of the queue and the priority.

@badziyoussef
Copy link
Contributor Author

I started to prototype it but I am going to need to play more with celery queues before finalizing it.

Adding the properties will maybe solve your problem but I would like to be able to add to task_enqueued the name of the queue and the priority.

Would be good to add priority if exists to task_enqueued, to avoid binding it to other logs like request_finished.

@jrobichaud jrobichaud self-assigned this Oct 18, 2023
@jrobichaud
Copy link
Owner

jrobichaud commented Oct 19, 2023

@badziyoussef 6.1.0.dev1 is deployed, this is a pre-release.

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 a pull request may close this issue.

2 participants