laravel / ideas

Issues board used for Laravel internals discussions.
939 stars 28 forks source link

Add a way to pre-process jobs before dispatch. #1342

Open atrauzzi opened 6 years ago

atrauzzi commented 6 years ago

I've been looking into adding tracing to my Laravel applications. To do this, I need to propagate or generate a unique identifier across application boundaries.

The trick is that I would love to do it in such a way that doesn't depend on me manually ferrying the data when I go to dispatch a job.

Instead, I was wondering if there could be (or already are?) lifecycle methods I can tap into to add additional data to be serialized along with a job. The job itself would have no expectation to interact with the data, but it would be seen and consumed during bootstrap by the tracing library.

This is particularly important when I dispatch my jobs to be handled by a worker thread. That worker thread needs some way to receive the correlation identifier as part of the job.

lk77 commented 6 years ago

Hello, You could implement your own dispatcher if you have specific needs.

atrauzzi commented 6 years ago

Oh, interesting. Didn't realize that was sitting in between!

Okay, so my only followup question would then be: does the framework use dispatchNow of that interface when dispatching in a worker process?

Thanks for the tip BTW.

lk77 commented 6 years ago

yes

    /**
     * Dispatch a job to its appropriate handler in the current process.
     *
     * @param  mixed  $job
     * @return mixed
     */
    public function dispatchNow($job)
    {
        return app(Dispatcher::class)->dispatchNow($job);
    }
hubertnnn commented 6 years ago

I think we would need a bit ore than that. Horizon is already doing some black magic to be able to add custom data to dispatched jobs, we might need some universal way of handling that, maybe by using events on dispatch. This would not only help in cases like above, but will make horizon possibly work with more queues than just redis without the need of extending to Connector class.

https://github.com/laravel/horizon/blob/1.0/src/RedisQueue.php

atrauzzi commented 6 years ago

My original approach was to look for an event being triggered. Only came here after discovering there wasn't one.

atrauzzi commented 6 years ago

So, I've managed to come up with one approach, although I think it's a bit smashy...

        $this->app->extend(\Illuminate\Bus\Dispatcher::class, function ($defaultDispatcher) {

            return new DispatcherType();
        });

This is obviously not great because I'm completely sideswiping the original binding. For now, I'm okay with it, and my type simply extends it.

Overall, I think it would be nicer if there was a way to hook dispatch and dispatchNow without getting into any DI tricks.

ajcastro commented 4 years ago

For anyone who will come here, here is how to actually do it.

$this->app->extend(\Illuminate\Bus\Dispatcher::class, function ($dispatcher, $app) {
    return new \App\Services\Dispatcher\MyDispatcher($app, $dispatcher);
});
<?php

namespace App\Services\Dispatcher;

class MyDispatcher extends \Illuminate\Bus\Dispatcher
{
    public function __construct($app, $dispatcher)
    {
        parent::__construct($app, $dispatcher->queueResolver); // we need to pass the queueResolver
    }

    public function dispatchToQueue($command)
    {
        // do anything you like during dispatch
        return parent::dispatchToQueue($command);
    }
}