Skip to content

feat(monitor): add cron monitor check-ins (monitor run / list)#1069

Merged
BYK merged 6 commits into
mainfrom
feat/monitor-checkins
Jun 4, 2026
Merged

feat(monitor): add cron monitor check-ins (monitor run / list)#1069
BYK merged 6 commits into
mainfrom
feat/monitor-checkins

Conversation

@BYK

@BYK BYK commented Jun 4, 2026

Copy link
Copy Markdown
Member

Summary

Implements the sentry monitor command group for full parity with the legacy Rust sentry-cli monitors commands. Closes #1052.

sentry monitor run <slug> -- <command>

Wraps an arbitrary command with cron monitor check-in envelopes:

  • Sends in_progress check-in on start, then ok/error (with duration) on completion based on exit code
  • DSN-based auth (--dsn / SENTRY_DSN / auto-detect fallback)
  • Forwards stdio, SIGINT/SIGTERM; preserves child exit code
  • Sets SENTRY_MONITOR_SLUG in child environment
  • Non-fatal check-in delivery — failures are logged, wrapped command still runs
  • --schedule (crontab format) upserts the monitor config on the open check-in
  • Dependent flags: --check-in-margin, --max-runtime, --timezone, --failure-issue-threshold, --recovery-threshold (all require --schedule)
  • -e/--environment (default production)
  • -- separator is optional — enabled allowArgumentEscapeSequence globally so flags after -- are passed to the wrapped command

sentry monitor list [<org>/]

Org-scoped list using the stable retrieveMonitorsForAnOrganization SDK endpoint:

  • Columns: ID, SLUG, NAME, STATUS, SCHEDULE
  • Cursor pagination (-c next/prev), --json/--fields, auto-detect/project-search modes
  • sentry monitors plural alias

Supporting changes

  • SentryMonitor + MonitorConfig Zod schemas in src/types/sentry.ts
  • src/lib/envelope/checkin-builder.tsbuildMonitorConfig(), buildCheckIn()
  • src/lib/api/monitors.tslistMonitors(), listMonitorsPaginated()
  • requireDsn() now accepts an optional usageHint parameter
  • allowArgumentEscapeSequence enabled in Stricli scanner config
  • monitor list added to ORG_ONLY_COMMANDS in complete.ts

Tests

  • Property tests for buildMonitorConfig + buildCheckIn invariants (dependent-flag validation, schedule round-tripping, open/close field presence)
  • Unit tests for monitor run (exit-code propagation, env var, check-in lifecycle, non-fatal send failures, schedule upsert, spawn error handling)
  • Schema validation tests for SentryMonitorSchema (crontab + interval responses, passthrough, rejection of missing core fields)
  • Updated completions.property.test.ts for drift detection

E2E verified

Tested end-to-end against a mock ingest server — confirmed two correct check_in envelopes with shared check_in_id, monitor_config on open only, duration on close, correct environment, schedule upsert, and exit-code preservation.

Implement the cron monitor command group for full parity with the legacy
Rust sentry-cli `monitors` commands.

`sentry monitor run <slug> -- <command>`:
- Wraps a command with cron check-in envelopes (in_progress on start,
  ok/error with duration on completion)
- DSN-based auth with auto-detect fallback
- Forwards stdio, SIGINT/SIGTERM; preserves child exit code
- Sets SENTRY_MONITOR_SLUG in child environment
- Non-fatal check-in delivery (logged, never aborts wrapped command)
- --schedule (crontab) upserts monitor config on the open check-in
- Dependent flags: --check-in-margin, --max-runtime, --timezone,
  --failure-issue-threshold, --recovery-threshold (all require --schedule)
- -e/--environment (default production)
- -- separator is optional (enabled allowArgumentEscapeSequence globally)

`sentry monitor list [<org>/]`:
- Org-scoped via buildOrgListCommand using retrieveMonitorsForAnOrganization
- Columns: ID, SLUG, NAME, STATUS, SCHEDULE
- Cursor pagination, --json/--fields, auto-detect/project-search modes
- `sentry monitors` plural alias

Supporting changes:
- SentryMonitor + MonitorConfig Zod schemas in src/types/sentry.ts
- src/lib/envelope/checkin-builder.ts (buildMonitorConfig, buildCheckIn)
- src/lib/api/monitors.ts (listMonitors, listMonitorsPaginated)
- requireDsn() now accepts an optional usageHint parameter
- allowArgumentEscapeSequence enabled in Stricli scanner config
- monitor added to ORG_ONLY_COMMANDS in complete.ts

Tests:
- Property tests for buildMonitorConfig + buildCheckIn invariants
- Unit tests for monitor run (exit code, env var, check-in lifecycle,
  non-fatal send failures, schedule upsert, spawn error handling)
- Schema validation tests for SentryMonitorSchema

Closes #1052
@BYK BYK self-assigned this Jun 4, 2026
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-1069/

Built to branch gh-pages at 2026-06-04 21:26 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

❌ Patch coverage is 70.80%. Project has 4594 uncovered lines.
❌ Project coverage is 82.01%. Comparing base (base) to head (head).

Files with missing lines (4)
File Patch % Lines
src/commands/monitor/run.ts 80.00% ⚠️ 16 Missing and 5 partials
src/commands/monitor/list.ts 25.00% ⚠️ 15 Missing
src/lib/api/monitors.ts 0.00% ⚠️ 8 Missing
src/lib/envelope/checkin-builder.ts 96.15% ⚠️ 1 Missing and 1 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    82.09%    82.01%    -0.08%
==========================================
  Files          362       367        +5
  Lines        25398     25537      +139
  Branches     16685     16750       +65
==========================================
+ Hits         20849     20943       +94
- Misses        4549      4594       +45
- Partials      1751      1759        +8

Generated by Codecov Action

@sentry-warden sentry-warden Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No fetch timeout in sendEnvelopeRequest blocks child process startup indefinitely (src/types/index.ts:127)

The fetch call in sendEnvelopeRequest has no timeout; since sendCheckInSafely for the in-progress check-in is awaited before spawn (run.ts:282), a hung TCP connection to the ingest endpoint will prevent the wrapped command from ever starting — contradicting the PR's 'non-fatal check-in delivery' goal.

Evidence
  • sendEnvelopeRequest at transport.ts:127 calls fetch(new Request(url, {...})) with no signal / AbortController timeout.
  • sendCheckInSafely calls sendEnvelopeRequest and does not race it against a timeout itself.
  • In run.ts, await sendCheckInSafely(..., 'in-progress') at line 266–276 is fully awaited before spawn(cmd, cmdArgs, ...) at line 282.
  • If the Sentry ingest endpoint accepts the TCP connection but never responds, fetch never settles, spawn is never reached, and the wrapped cron job never runs.
  • The closing check-in (run.ts:327) has the same defect: a hung response prevents the parent process from exiting after the command finishes.

Identified by Warden find-bugs

@sentry-warden sentry-warden Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

process.once signal handlers allow a second Ctrl+C to kill the parent before the closing check-in is sent

In src/commands/monitor/run.ts, process.once('SIGINT', onSigint) removes itself after the first SIGINT. If the child is slow to exit and the user presses Ctrl+C a second time, Node.js falls back to its default SIGINT handler (process.exit(130)), which terminates the parent synchronously—skipping both the finally cleanup and the subsequent sendCheckInSafely call, leaving the monitor permanently stuck in in_progress.

Fix: replace process.once with process.on and keep the removeListener calls in the finally block as they already are, so all signals during the child's lifetime are forwarded.

Evidence
  • process.once('SIGINT', onSigint) at run.ts:293 fires once and auto-removes the handler.
  • The await new Promise<number> at run.ts:299 is still pending when the second SIGINT arrives.
  • With no listener registered, Node.js invokes its default SIGINT behaviour: process.exit(130).
  • process.exit() is synchronous and does not await pending async continuations, so the finally block at run.ts:318 and the sendCheckInSafely call at run.ts:327 are never reached.
  • The same pattern applies to process.once('SIGTERM', onSigterm) at run.ts:294.

Identified by Warden find-bugs

Comment thread src/commands/monitor/run.ts Outdated
Comment thread src/commands/monitor/run.ts Outdated
Comment thread src/lib/api/monitors.ts Outdated
- Wrap makeDsn() in try/catch to surface ValidationError on malformed
  DSNs (matching event/send.ts and transport.ts patterns)
- Map signal-killed child to 128+N exit code (Unix convention) instead
  of always exiting 1 when the child is terminated by SIGINT/SIGTERM
- Use autoPaginate + API_MAX_PER_PAGE in listMonitors() to fetch all
  monitors for large orgs (matching listProjects pattern)
Comment thread src/commands/monitor/run.ts Outdated
Comment thread src/commands/monitor/run.ts
- Use os.constants.signals for complete signal→number mapping (covers
  SIGKILL=137, SIGPIPE=141, etc. — not just the 4 hardcoded signals)
- Add 30s timeout to sendCheckInSafely so a slow/unreachable ingest
  endpoint cannot stall the child process spawn or exit
Comment thread src/commands/monitor/run.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 146b8d5. Configure here.

Comment thread src/commands/monitor/run.ts Outdated
The setTimeout used for the check-in send timeout was never cleared,
keeping the Node event loop alive for up to 30s after the wrapped
command exits. Clear it in a finally block so the process exits
promptly.
Comment thread src/commands/monitor/run.ts Outdated
- Prevent unhandled rejection from orphaned fetch when timeout wins
  Promise.race (add no-op .catch() handler)
- Use process.on instead of process.once for signal forwarding so
  repeated SIGINT/SIGTERM still reach the child (prevents parent from
  exiting before the closing check-in is sent)
- Downgrade log.error to log.warn and log.info to log.debug in
  sendCheckInSafely (non-fatal path should not alarm users)
- Improve interval schedule display: 'every 1 hour' instead of '1 hour'
- Remove empty hideRoute from monitor route map
@BYK BYK merged commit e237d92 into main Jun 4, 2026
29 checks passed
@BYK BYK deleted the feat/monitor-checkins branch June 4, 2026 21:40
Comment thread src/commands/monitor/index.ts
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.

monitor: add cron monitor check-ins (monitor run / list)

1 participant