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 cron monitoring feature #774

Closed
wants to merge 4 commits into from
Closed

Conversation

bOmBeLq
Copy link

@bOmBeLq bOmBeLq commented Oct 30, 2023

This is implementation of https://docs.sentry.io/platforms/php/crons/
This approach will allow it to be used with crontab as welll as any other scheduler (k8s, etc.).
Just need to add proper options to your console command eg.

bin/console any_command --cron-monitor-slug=the_slug --cron-monitor-schedule "0 0 * * *" --cron-monitor-max-time=5 --cron-monitor-check-margin=2

max-time and check-margin is optional
Downside is readability and that you have to manually pass the schedule (which often is a duplicate). But couldn't come up with anything better.

@cleptric
Copy link
Member

Something similar can be achieved using the Sentry CLI, see https://docs.sentry.io/product/crons/getting-started/cli/.

I'm not too keen on adding this on top.

@bOmBeLq
Copy link
Author

bOmBeLq commented Oct 31, 2023

Didn't see that. But anyways you still have to:

  • maintain separate configuration for php and cli (this approach will simply grab DSN, APP_ENV from SF app)
  • create crons manually on sentry.io (php lib allows to create them automatically on first run)

Anyways that's fine if you are not merging. If that's the case please close PR. I will move it to separate bundle.

@ste93cry
Copy link
Collaborator

Imho, I don't think it's a good idea to mess with other people's CLI commands by adding options without their knowledge. This is not to say that I dislike the proposal of this feature, just that I'm not convinced this it the correct approach. I will leave the merit of deciding whether to continue or not to @cleptric, anyway.

@bOmBeLq
Copy link
Author

bOmBeLq commented Oct 31, 2023

don't think it's a good idea to mess with other people's CLI commands by adding options without their knowledge. This is not to say that I dislike the proposal of this feature, just that I'm not convinced this it the correct approach. I will leave the merit of deciding whether to continue or not to @cleptric, anyway.

can as well make this disabled by default but I see no issue moving this to separate bundle. Just let me know what's your decision :)

@cleptric
Copy link
Member

* maintain separate configuration for php and cli (this approach will simply grab DSN, APP_ENV from SF app)

Both the Symfony SDK and CLI can be configured with environment variables. SENTRY_ENVIRONMENT, SENTRY_RELEASE and SENTRY_DSN are supported.

* create crons manually on sentry.io (php lib allows to create them automatically on first run)

I just talked to the EM of the Crons team and asked to put this on the roadmap.

We definitely want to have tighter Crons integration in Symfony as well, but so far, I haven't seen a good way to do this. Laravel's schedules tasks are amazing UX-wise and easy for us to work with. If something like this comes up in Symfony, rest assured we'll add support for it!

@cleptric cleptric closed this Oct 31, 2023
@Jean85
Copy link
Collaborator

Jean85 commented Nov 2, 2023

@cleptric Symfony has the Scheduler component: https://symfony.com/components/Scheduler

https://symfony.com/blog/new-in-symfony-6-3-scheduler-component
(it's pretty recent, it's probably underutilized)

@cleptric
Copy link
Member

cleptric commented Nov 2, 2023

Neat, opened #776

@bOmBeLq
Copy link
Author

bOmBeLq commented Nov 2, 2023

@cleptric Symfony has the Scheduler component: https://symfony.com/components/Scheduler

https://symfony.com/blog/new-in-symfony-6-3-scheduler-component (it's pretty recent, it's probably underutilized)

Saw it but we are at lower SF version. Also this won't work if you are using something like k8s to schedule your jobs.

@cleptric
Copy link
Member

cleptric commented Nov 2, 2023

k8s is a completely different beast to tame, but we have some ideas.

@stixx
Copy link

stixx commented Jan 3, 2024

Why not only add the possibility to configure which commands are to be monitored and add a listener/subscriber which acts when the commands are executed and fires the check in events.

This would still allow the infrastructure of the application to configure how crons are run. Which in my opinion is the responsibility of the infrastructure and not the package.

Pros:

  • You're still in control of how crons/command's are scheduled and executed

Cons:

  • You can have different cron schedule configuration by the application and infrastructure.

Perhaps this should be viewed as two features:

  • Configuring the monitoring and fire the check in events
  • Creating actual cron's from the configuration if enabled using the symfony scheduler component

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