feat: Added sentryMonitor macro support for specifying environments, updated CONTRIBUTING

[Braunson]

Feature

• Added the new parameter to specify an array of environments that you would like the sentryMonitor() logic to run in
  • If nothing is specified, then the macro will run like normal
  • If one or more environments are specified and one matches it will run like normal
  • If one or more environments are specified and nothing matches it will run not run and return $this
• Updated the contribution guide as there is no longer a Composer phpcs command. I did try running cs-fix/cs-check and there was a ton of changes (looks like mainly LF -> CRLF changes) but I didn't commit any of those changes.

References

• Cronjobs Monitoring - Your feedback needed sentry#42283 (comment)

[stayallive]

I'm not a big fan of using config('app.env') directly, we should probably read that from the app()->environment() (without using global functions).

I do think having the option to enable/disable this functionality on an environment basis is useful, however I'm wondering if there is a case where this needs to be controlled on a monitor level or that it could be a global configuration (to enable / disable scheduled task monitoring). Or maybe we could do with both, having a config option to disable scheduled task monitoring AND have a way to specify the environments on the ->sentryMonitor macro.

@cleptric WDYT?

Edit: I totally forget, but thanks for taking the time to start the discussion and take the time to make a PR!

[Braunson]

Yeah I agree with about using app()->environment() instead, I didn't really like using config. I thought about defining the environments for all monitor calls in the config BUT my specific use case was to specify the environments for each scheduled command (instead of blanketing them all). Combining the two may be another option, having this defined in the sentry.php config, but also being able to override that directly in the sentryMonitor macro parameters.

I'm open to suggestions on changes.

[stayallive]

So I've changed a few things.

• Renamed the new argument variable to be a little more clear
• Remove the third test you added since it was basically the same as the other tests (they also don't provide a value for the arg) so not really needed
• Get the environment from the container, the instanceof check is mostly there for static analyzers since it should always be an instance of that contract (even on Lumen) but safer this way if it isn't
• Using detectEnvironment() to make the test pass, only way I could figure out how to update the environment and make the tests work (otherwise the environment is always testing)

[stayallive]

Thanks for taking the time to create a PR!

[cleptric]

@Braunson I was told that the Crons Team is working on a new feature on the Sentry UI to ignore specific environments. So, we might not need to add this to the SDK in the end.

Edit: The feature was just shipped. You can hover over the environment on the Cron Monitors page (https://sentry.io/crons/) to mute or delete an environment.

[Braunson]

@cleptric this still isn't ideal as when this feature is out of beta, I'd still not want certain environments reporting and taking a hit on the quota.

[cleptric]

You won't be charged per environment. We can leave this open in the meantime, and I'll take another look next year.

[cleptric]

We'll add a new before_send_check_in option. This will allow you to conditional disable sending check-ins. getsentry/sentry-php#1690

[cleptric]

before_send_check_in was added to the PHP SDK in v4.5.0. Just update the Laravel SDK to v4.2.0, and you can start using it.