-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(uptime): Add test monitor button to uptime configuration forms #106832
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
base: master
Are you sure you want to change the base?
feat(uptime): Add test monitor button to uptime configuration forms #106832
Conversation
Add a "Test Monitor" button that allows users to test their uptime monitor configuration before saving. The button calls the preview check endpoint and displays success/error feedback via toast messages. The button is added to both: - Legacy uptime alert form (alerts/rules/uptime) - New detector form (detectors/components/forms/uptime) Only visible when the uptime-runtime-assertions feature flag is enabled.
Update the uptime test monitor button to use the recommended fetchMutation pattern instead of the deprecated useApi approach per frontend guidelines.
Convert headers and assertion from potential MobX observable proxies to plain JavaScript arrays/objects before sending to the API, fixing 400 errors from the backend validator.
Use formModel.getTransformedData() to get properly transformed form data instead of manually converting MobX observables. This is the correct API for getting form data that will be sent to API endpoints.
- Move Test Monitor button to form footer next to submit button in both legacy and detector forms - Check check_result.status from uptime-checker response to determine success/failure instead of relying on HTTP status code - Add PreviewCheck* types to types.tsx matching the uptime-checker /execute_config response format - Update tests to use correct response format with check_result.status
Add label prop to TestUptimeMonitorButton allowing customization. Old form uses "Test Rule" while new detector form defaults to "Test Monitor".
Use Flex with gap instead of Fragment for proper spacing between delete and test buttons.
static/app/views/alerts/rules/uptime/testUptimeMonitorButton.tsx
Outdated
Show resolved
Hide resolved
- Move PreviewCheckPayload type to types.tsx for centralization - Use DEFAULT_UPTIME_DETECTOR_FORM_DATA_MAP for fallback values instead of duplicating constants - Remove region from preview check payload (not needed by endpoint) - Revert unrelated rspack.config.ts changes - Add tests for custom label prop, MISSED_WINDOW/DISALLOWED_BY_ROBOTS statuses, and assertion data
…ode organization - Move fallback handling from TestUptimeMonitorButton to its callers, allowing each context to provide appropriate defaults - Extract ConnectedTestUptimeMonitorButton to its own file - Remove unnecessary Observer wrapper in uptimeAlertForm - Access form context directly instead of subscribing to individual fields - Add DEFAULT_TIMEOUT_MS and DEFAULT_METHOD constants in uptimeAlertForm
- Remove IconPlay from TestUptimeMonitorButton - Add optional size prop to TestUptimeMonitorButton and ConnectedTestUptimeMonitorButton - Use size="sm" in EditExistingUptimeDetectorForm to match other footer buttons
|
@klochek uptime-preview-check endpoint 403s with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| url: data.url || undefined, | ||
| method: data.method || DEFAULT_UPTIME_DETECTOR_FORM_DATA_MAP.method, | ||
| headers: data.headers ?? DEFAULT_UPTIME_DETECTOR_FORM_DATA_MAP.headers, | ||
| body: data.body || null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test button sends body for GET/HEAD/OPTIONS requests
Medium Severity
The getFormData function in the detector form's test button always sends body if present, without checking whether the HTTP method supports request bodies. Methods like GET, HEAD, and OPTIONS shouldn't include bodies. The alert form version correctly checks methodHasBody(formModel) before including body data, but the detector form version doesn't perform this validation.
|
|
||
| // Preview Check Types (raw response from uptime-checker /execute_config endpoint) | ||
|
|
||
| export enum PreviewCheckStatus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we already have these types above in the file, I believe the just map to the same response i.e. CheckStatus, CheckStatusReason etc.
We might just need to extend those
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! These are intentionally separate for a few reasons:
-
Different API sources: The
PreviewCheck*types represent the raw response from the uptime-checker's/execute_configendpoint, whileCheckStatus/CheckStatusReasonrepresent what's stored and returned from Sentry's backend. They're distinct APIs with legitimately different response shapes. -
Different values that are context-specific:
CheckStatushasFAILURE_INCIDENT(computed by Sentry after multiple failures constitute an incident - not produced by uptime-checker)PreviewCheckStatusincludes values likeDISALLOWED_BY_ROBOTSthat uptime-checker produces but Sentry filters out of stored/returned dataPreviewCheckStatusReasonTypeincludes assertion-specific errors and miss reasons that aren't exposed in Sentry's API responses
-
TypeScript limitation: Enums can't extend other enums in TypeScript, so consolidation would require switching to
constobjects with union types, adding complexity without clear benefit.
The duplication of shared values is minimal and keeping them separate provides better type safety for each context.
Summary
/uptime-preview-check/endpoint to validate the monitor configurationcheck_result.statusfrom the responseChanges
TestUptimeMonitorButtoncomponent with proper TypeScript types for the preview check responsePreviewCheckStatusandPreviewCheckStatusReasonTypeenums totypes.tsxUptimeAlertFormand new detector forms (NewUptimeDetectorForm,EditExistingUptimeDetectorForm)uptime-runtime-assertionsfeature flagTest plan