Monitor upsert support docs #8500
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| - `--check-in-margin`: The allowed margin of minutes after the expected check-in time that the monitor will not be considered missed for. | ||
| - `--max-runtime`: The allowed duration in minutes that the monitor may be in progress for before being considered failed due to timeout. | ||
| - `--timezone` _(beta, use cautiously!)_: A tz database string (e.g. "Europe/Vienna") representing the monitor's execution schedule's timezone. Please be **absolutely sure that you provide a valid tz database string** when using this argument; as of writing, the CLI lacks the capability to validate this string, and so **the monitor creation will silently fail when the string is invalid!** For now, we recommend verifying that the cron monitor was created successfully in Sentry when using this argument. |
There was a problem hiding this comment.
Since this --timezone option can cause the entire monitor upsert to silently fail if an invalid value is provided, I have labelled it as a beta feature for now. Is this appropriate?
We would need to make sure we remove the "beta" notice and the warning about the CLI not validating the string once we implement getsentry/relay#2718.
There was a problem hiding this comment.
I think the beta note makes sense
| @@ -59,6 +59,32 @@ sentry-cli monitors run my-monitor-slug -- python path/to/file.py | |||
| sentry-cli monitors run my-monitor-slug -- node path/to/file.js | |||
| ``` | |||
|
|
|||
| ### Configuring Your Monitor via the CLI | |||
|
|
|||
| You can also use the Sentry CLI to configure your cron monitor when you run your job. This way, you can avoid having to first set up the monitor through the Sentry web interface. | |||
There was a problem hiding this comment.
This line feels weird in the middle of the page since it's general to the CLI and not specific to this section. I think you can either delete it or move it up to right before the "Requirements" section.
There was a problem hiding this comment.
Can you please explain why you think this line is out of place here? I think it is necessary to include this line here to introduce the purpose of this feature; that is, that you can configure your monitor via the CLI instead of configuring via the web interface. The line also indicates that using this feature is optional, since monitors can also be created and configured in the Sentry web interface.
However, I agree that we should probably mention the possibility of setting up the monitor via the CLI itself in the "Requirements" section, so I will add a mention there.
There was a problem hiding this comment.
Ah I thought it was implied from where the page lives (Crons -> Set Up -> CLI) that you were using the CLI to create / set up a new monitor, so it felt like something that should go at the top of the page. But now I see that the original feature was just for checking in on an existing monitor.
I think to make this clearer, you could keep this line here, but rename the section header to "Create or Update Your Monitor" similar to the section for HTTP.
| - `--max-runtime`: The allowed duration in minutes that the monitor may be in progress for before being considered failed due to timeout. | ||
| - `--timezone` _(beta, use cautiously!)_: A tz database string (e.g. "Europe/Vienna") representing the monitor's execution schedule's timezone. Please be **absolutely sure that you provide a valid tz database string** when using this argument; as of writing, the CLI lacks the capability to validate this string, and so **the monitor creation will silently fail when the string is invalid!** For now, we recommend verifying that the cron monitor was created successfully in Sentry when using this argument. | ||
|
|
||
| Below are some usage examples: |
There was a problem hiding this comment.
Nice! I like how you have two examples here
|
|
||
| - `--check-in-margin`: The allowed margin of minutes after the expected check-in time that the monitor will not be considered missed for. | ||
| - `--max-runtime`: The allowed duration in minutes that the monitor may be in progress for before being considered failed due to timeout. | ||
| - `--timezone` _(beta, use cautiously!)_: A tz database string (e.g. "Europe/Vienna") representing the monitor's execution schedule's timezone. Please be **absolutely sure that you provide a valid tz database string** when using this argument; as of writing, the CLI lacks the capability to validate this string, and so **the monitor creation will silently fail when the string is invalid!** For now, we recommend verifying that the cron monitor was created successfully in Sentry when using this argument. |
There was a problem hiding this comment.
I think the beta note makes sense
Co-authored-by: vivianyentran <[email protected]>
|
|
||
| - `--check-in-margin`: The allowed margin of minutes after the expected check-in time that the monitor will not be considered missed for. | ||
| - `--max-runtime`: The allowed duration in minutes that the monitor may be in progress for before being considered failed due to timeout. | ||
| - `--timezone` _(beta, use cautiously!)_: A [tz database identifier string](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones#List) (e.g. "Europe/Vienna") representing the monitor's execution schedule's timezone. Please be **absolutely sure that you provide a valid tz database string** when using this argument; as of writing, the CLI lacks the capability to validate this string, and so **the monitor creation will silently fail when the string is invalid!** For now, we recommend verifying that the cron monitor was created successfully in Sentry when using this argument. |
There was a problem hiding this comment.
Why is this labeled as Beta?
There was a problem hiding this comment.
I added this to call folks' attention to the fact that using this argument can cause the upsert to fail silently; however, it is probably sufficient to just have the "use cautiously" label. The "beta" label is redundant, since all features described on this page are beta features, according to the note at the top of the page
|
|
||
| - `--check-in-margin`: The allowed margin of minutes after the expected check-in time that the monitor will not be considered missed for. | ||
| - `--max-runtime`: The allowed duration in minutes that the monitor may be in progress for before being considered failed due to timeout. | ||
| - `--timezone` _(beta, use cautiously!)_: A [tz database identifier string](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones#List) (e.g. "Europe/Vienna") representing the monitor's execution schedule's timezone. Please be **absolutely sure that you provide a valid tz database string** when using this argument; as of writing, the CLI lacks the capability to validate this string, and so **the monitor creation will silently fail when the string is invalid!** For now, we recommend verifying that the cron monitor was created successfully in Sentry when using this argument. |
There was a problem hiding this comment.
Can we add this timezone bit as a note instead of this long string at the end of the list item?
There was a problem hiding this comment.
Sure, a note was one of the options I also considered. Might be cleaner here
|
@evanpurkhiser I made some changes to address the concerns you brought up, does it look good now? |
Pre-merge checklist
If you work at Sentry, you're able to merge your own PR without review, but please don't unless there's a good reason.
Description of changes
This PR adds documentation for the new monitor upserts feature in the Sentry CLI, which allows users to create a cron monitor directly from the command line, without having to set it up in the Sentry web UI first.
Closes #8482