laravel / pulse

Laravel Pulse is a real-time application performance monitoring tool and dashboard for your Laravel application.
https://pulse.laravel.com
MIT License
1.43k stars 165 forks source link

Records auto-cleaner #317

Closed didac-adria closed 7 months ago

didac-adria commented 7 months ago

Hi!

I know this is in betta still, but I think the basic implementation of Pulse should contain a DB records cleaner. Otherwise they pile-up. Do you guys have something in mind already to incorporate a command to clean records older than 7 days (I think is the maximum amount of days to see data in the dashboards)? Or am I missing something and records should be deleted but somehow they aren't in our backend?

driesvints commented 7 months ago

This already exists: https://laravel.com/docs/10.x/pulse#trimming

didac-adria commented 7 months ago

Yeah I know @driesvints , but it's not working on our side. Maybe I don't understand the documentation. We have records almost a month old. I think the maximum age is 7 days.

didac-adria commented 7 months ago

Just for clarification, we have the default configuration:

'trim' => [
            'lottery' => [1, 1_000],
            'keep' => '7 days',
        ],

Also, I'm trying to check the code to see how it works, because to my eyes, it's not clear how the lottery works. If I discover how it works, I may make a PR to the documentation to help.

didac-adria commented 7 months ago

@driesvints We've been trying to investigate how it works but we don't reach solid conclusions: 1- We've tried to reduce the lottery to see if it prunes the records automatically. It doesn't 2- We have tried to add some logs to check for errors. We don't see any.

We have also realized that we have to manually restart the deamon from forge in order to make it get the new php code after each deployment.

So: 1- Can you give us a hint about how the flow works to make it clean and why 'lottery' => [1, 1], doesn't work? 2- Maybe we need a system to be able to restart the deamon of pulse:check in each deployment, the same way there is one for horizon.

Thanks in advance!

sinnbeck commented 7 months ago

Hmm it seems there is a bug. I used xdebug and it never hits the trim method.

If I remove ... from trim() here it works. It is already inside a closure :) https://github.com/laravel/pulse/blob/1.x/src/Pulse.php#L314

//either do
->winner(fn () => $this->rescue($ingest->trim(...))
//or
->winner(fn () => $this->rescue(fn () => $ingest->trim()))

@didac-adria You can restart with php artisan pulse:restart

didac-adria commented 7 months ago

Oh great! I'm adding pulse:restart to our deployment hooks. Thanks!

timacdonald commented 7 months ago

Closing this as we have a fix up.

Heads up that this mainly impacts apps writing directly to the database and not using the performance focused Redis ingest.

Sorry for the troubles, folks.

didac-adria commented 7 months ago

Hey team! Thank you all for your contributions!

See you!