search

owen-it / laravel-auditing

Record the change log from models in Laravel
https://laravel-auditing.com
MIT License
3.01k stars 387 forks source link

Queued Auditing #846

Closed Orrison closed 9 months ago

Orrison commented 1 year ago

Per discussions such as #562, this sets up auditing to be dispatched on the Queue.

By default, the listener will go onto the sync connection. So if no configuration is done, the package will work as it did before, auditing immediately in the current request.

Once you configure the audit.queue.connect to be something other than sync, the audits will then be dispatched to be processed asynchronously.

Customization of the queue it is put on, as well as the delay, is also possible.

It is set up similarly to the Auditing/Audited flow. An Event (DispatchingAudit) is fired where you can prevent the auditing dispatch from happening by having a listener return false.

Orrison commented 1 year ago

Pushed a fix that should resolve one of the test errors. Unsure about the PHP 7 failures though. Having trouble getting this package to work in PHP 7

Orrison commented 1 year ago

Copied what y'all were doing in the GitHub Workflow setup locally and fixed the PHP 7.4 errors. I believe all tests should pass now if you want to run the test suite again.

Orrison commented 1 year ago

@MortenDHansen just wanted to check in to see if there is anything I can do to move this queued auditing concept along in the package.

Even if it requires me to change how I implement it here, I am completely open and available to do so.

mohamedgaber-intake40 commented 10 months ago

may i know , why this pull request is not merged ? i think this feature is needed to be exists in the package

Orrison commented 9 months ago

@MortenDHansen, I just want to ping one last time to see whether or not this PR could go anywhere here.

Queue auditing would be an amazing improvement. Could you let me know if there is any feedback on my implementation, I am happy to put time into going a different direction if you feel that it's required.

Thank you!

MortenDHansen commented 9 months ago

It seems like it would work. I was worried that it would solve too narrow a case by only augmenting the actual data saving. I will get to try it out and then we could probably merge it. I don't see anything that should break, but i have to try it :)

parallels999 commented 9 months ago

This version gave me problems, I will try to locate them and give my feedback

MortenDHansen commented 9 months ago

This version gave me problems, I will try to locate them and give my feedback

Please do! I didn't have any issues with it on my applications, but if we have issues that are hard to track down, we should roll an update without the queue system asap. If it breaks for users, then we need to make it opt-in

parallels999 commented 9 months ago

I found my problem image I have an Audited event listener

'OwenIt\Auditing\Events\Audited' => [
    'App\Listeners\AuditedListener',
]

But seems that DispatchAudit is triggered first, and Audited no longer does what it did before, Works again, if i revert

-$this->dispatchAudit($model->setAuditEvent('created'));
+Auditor::execute($model->setAuditEvent('created'));

I don't know if the problem is that I'm handling the event listeners wrong, but they had worked for me since several major previous versions without modifying anything.

Orrison commented 9 months ago

I found my problem image I have an Audited event listener

'OwenIt\Auditing\Events\Audited' => [
    'App\Listeners\AuditedListener',
]

But seems that DispatchAudit is triggered first, and Audited no longer does what it did before, Works again, if i revert

-$this->dispatchAudit($model->setAuditEvent('created'));
+Auditor::execute($model->setAuditEvent('created'));

I don't know if the problem is that I'm handling the event listeners wrong, but they had worked for me since several major previous versions without modifying anything.

Where are you having these issues? In testing or in staging/production?

The dispatchAudit call just triggers a queued listener that calls Auditor::execute, which works the same as before, performing the audit and then dispatching the Audited event.

parallels999 commented 9 months ago

Where are you having these issues? In testing or in staging/production?

everywhere

dispatching the Audited event

Yes, I don't know what's happening, maybe it's as if the order of events has changed, or as if the changes to the model are not made in the same instance.

I will try to do more tests and find a solution.

parallels999 commented 9 months ago

question, is changing contracts a breaking change?

danharrin commented 9 months ago

lgtm

parallels999 commented 9 months ago

@Orrison I already isolated my problem Look at the 2737, is laravel recently created model(spl_object_id), using sync right v13.6.0 left v13.6.0 just i did remove implements ShouldQueue on ProcessDispatchAudit, image Without queue, spl_object_id remains, it means that the instance reference is the same in all events With queue, spl_object_id changes on ProcessDispatchAudit to 2782, means that there is a second clone of my model, so the model on my audit event listeners is not the same as the other laravel events, so the process stop with 500 error

we need to make it opt-in

Maybe

mattvb91 commented 9 months ago

question, is changing contracts a breaking change?

Seems to be. I've just upgraded cause phpstan is failing and now all of my tests where I validate audits were created are failing. Havent had a chance to look closer yet

cosmastech commented 9 months ago

Our pipelines are also failing with this change.

mattvb91 commented 9 months ago

Fixed for me in 13.6.1 thanks everyone

Tofandel commented 9 months ago

Heey hey hey, this is a breaking change (interface signature change) and should have been released in 14.x, I mean there is a reason there is Contracts in the namespace, because you can't change it without breaking stuff

It should be reverted on 13.x (ASAP because it will cause yet another breaking change for people who solved the issue in their project but they will be able to upgrade to 14.x to solve it, but solve a lot of headaches for peolpe who have yet to upgrade) and rereleased on 14.x

Otherwise anyone implementing this interface will get a fatal error in php Declaration of App\Resolvers\UserResolver::resolve() must be compatible with OwenIt\Auditing\Contracts\UserResolver::resolve(OwenIt\Auditable $auditable)

chrisRedwine commented 9 months ago

I believe this also broke Lumen compatibility, with the introduction of Illuminate\Foundation\Events\Dispatchable

erikn69 commented 9 months ago

@chrisRedwine #883

MortenDHansen commented 9 months ago

And now v13.6.2 should fix the contracts issue so that it doesn't break. I haven't tried it on Lumen, so if anyone experiences issues there, let us now!

Big thanks to @erikn69 again ❤️

kenfai commented 9 months ago

The change has produced a lot of logging when using Supervisor to manage process queues and job logging, and Laravel Excel package to import large amount of data to the database.

[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:32][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit
[2023-12-21 14:36:33][] Processed:  OwenIt\Auditing\Listeners\ProcessDispatchAudit

I understand this may not be related to Laravel-Auditing library, but is there anyway to disable logging for the above audit listener class by default?

MortenDHansen commented 9 months ago

The change has produced a lot of logging when using Supervisor to manage process queues and job logging, and Laravel Excel package to import large amount of data to the database.

[2023-12-21 14:36:32][] Processing: OwenIt\Auditing\Listeners\ProcessDispatchAudit

I understand this may not be related to Laravel-Auditing library, but is there anyway to disable logging for the above audit listener class by default?

Hi, could you please create an issue for it? 🙂