Skip to content

feat(selfhost): expand Sentry observability context for self-host runtime - #1881

Closed
jonathanchang31 wants to merge 3 commits into
JSONbored:mainfrom
jonathanchang31:feat-1824-selfhost-sentry-context
Closed

feat(selfhost): expand Sentry observability context for self-host runtime#1881
jonathanchang31 wants to merge 3 commits into
JSONbored:mainfrom
jonathanchang31:feat-1824-selfhost-sentry-context

Conversation

@jonathanchang31

Copy link
Copy Markdown
Contributor

Summary

  • Expand self-host Sentry coverage with a safer operational taxonomy for critical runtime failures.
  • Add structured low-cardinality tags and context for scheduler, queue, Orb relay, GitHub check-run, and AI provider failure paths.
  • Strengthen Sentry scrubbing so request bodies, prompts, review text, private config, auth paths, and secret-like fields are redacted before events leave the box.
  • Document alert classes and self-host Sentry data-boundary expectations.

Related Issue

Closes: #1824

Change Type

  • Feature
  • Bug fix
  • Tests
  • Documentation
  • Breaking change
  • Refactor only

What Changed

  • Added safe Sentry tag/context mapping in src/selfhost/sentry.ts.
  • Expanded scrubbing rules to cover sensitive payload/content fields and local auth/config path fields.
  • Added self-host monitor context for:
    • scheduled loop
    • Orb export
    • Orb relay drain
  • Added structured observability fields to critical error logs and capture sites for:
    • process crashes / unhandled rejections
    • scheduled loop failures
    • Orb export / relay registration / relay drain failures
    • queue reclaim / dead-letter failures
    • GitHub broker token failures
    • gate/check-run permission failures
    • AI provider failures / provider exhaustion
  • Updated self-host operations and security docs.
  • Added regression tests for Sentry scrubbing, safe tag behavior, and monitor context.

Real Behavior Proof

  • npx vitest run test/unit/selfhost-sentry.test.ts test/unit/selfhost-monitored-work.test.ts
  • npm run test:ci
  • npm audit --audit-level=moderate

Checklist

  • Sentry remains a complete no-op when SENTRY_DSN is unset
  • Sensitive payloads are scrubbed before capture
  • High-cardinality identifiers are not promoted to Sentry tags
  • Critical self-host paths carry actionable subsystem/operation/reason metadata
  • Monitor coverage includes scheduled recurring self-host work
  • Unit tests added/updated
  • Docs updated
  • Full local validation passed

Notes

  • This change is scoped to self-host observability only.
  • It does not enable Sentry by default; operators must still provide their own DSN.

@dosubot dosubot Bot added the size:L label Jun 30, 2026
@loopover-orb

loopover-orb Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-06-30 19:17:11 UTC

14 files · 1 AI reviewer · 1 blocker · readiness 48/100 · CI green · dirty

🛑 Suggested Action - Reject/Close

  • AI reviewers agree on a likely critical defect: src/selfhost/sentry.ts:18 makes the `review` key itself sensitive because `(text|body)?` is optional, so any event context named `review` from `captureReviewFailure` is replaced with `[redacted]` and the safe repo/PR context added at src/selfhost/sentry.ts:230 never reaches Sentry. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
The PR broadens self-host Sentry taxonomy, adds low-cardinality context in several runtime paths, and documents the intended Sentry data boundary. Most of the structured fields are straightforward, but the new scrub regex removes a context object that the PR is trying to preserve, and the GitHub in-flight cache change regresses the existing fail-open behavior for pre-response network errors. Those are reachable behavior changes, not just observability polish.

Blockers

  • src/selfhost/sentry.ts:18 makes the `review` key itself sensitive because `(text|body)?` is optional, so any event context named `review` from `captureReviewFailure` is replaced with `[redacted]` and the safe repo/PR context added at src/selfhost/sentry.ts:230 never reaches Sentry.
  • src/github/client.ts:289 now makes coalesced callers throw the leader's pre-response network error instead of doing their own fetch when no cached replay exists, so two simultaneous cacheable GETs can both fail from one transient leader failure even though the follower path previously succeeded.
Nits — 6 non-blocking
  • nit: apps/gittensory-ui/src/routes/docs.self-hosting-operations.tsx:139 documents `repo` as the tag name, but several changed log paths still emit `repository`, so operators may need to filter on both unless the log tag mapper normalizes aliases.
  • nit: src/selfhost/sentry.ts:240 keeps direct structured-log tag keys separate from `SENTRY_CONTEXT_TAGS`, which makes alias behavior drift likely as new fields are added.
  • In src/selfhost/sentry.ts, change `SENSITIVE_CONTENT_KEY` to match concrete content fields such as `reviewText`, `reviewBody`, `commentText`, and `commentBody`, then add a regression that preserves `contexts.review.repo` while redacting actual review text/body fields.
  • In src/github/client.ts, restore the old follower fallback by sharing only cached replay data in `inFlightCacheableGets`, or have followers perform their own fetch when the leader result is `{ ok: false }`.
  • Normalize structured log aliases through the same safe tag mapping used for capture contexts so `repository` consistently becomes `repo` and `queueBackend` consistently becomes `backend`.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.

Why this is blocked

  • src/selfhost/sentry.ts:18 makes the `review` key itself sensitive because `(text|body)?` is optional, so any event context named `review` from `captureReviewFailure` is replaced with `[redacted]` and the safe repo/PR context added at src/selfhost/sentry.ts:230 never reaches Sentry.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (size label size:L; no linked issue context).
Validation posture ❌ 5/25 Preflight is holding this PR; address the blocker before review.
Contributor workload ✅ 10/10 Author activity: 151 registered-repo PR(s), 82 merged, 11 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jonathanchang31; Gittensor profile; 151 PR(s), 11 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
Contributor next steps
  • Explain no-issue PR.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Fix the blocker.
  • Triage stale or unlinked PRs.
  • Refresh registry data or choose a registered active repo.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. labels Jun 30, 2026

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I appreciate the work on this PR (selfhost is the most important focus area right now), but there are merge conflicts, and this PR seems to mix the concerns from 2 or more separate issues into a single one - there are also blockers in the review:

Blockers

src/selfhost/sentry.ts:18 makes the `review` key itself sensitive because `(text|body)?` is optional, so any event context named `review` from `captureReviewFailure` is replaced with `[redacted]` and the safe repo/PR context added at src/selfhost/sentry.ts:230 never reaches Sentry.
src/github/client.ts:289 now makes coalesced callers throw the leader's pre-response network error instead of doing their own fetch when no cached replay exists, so two simultaneous cacheable GETs can both fail from one transient leader failure even though the follower path previously succeeded.
Nits — 6 non-blocking
Why this is blocked

src/selfhost/sentry.ts:18 makes the `review` key itself sensitive because `(text|body)?` is optional, so any event context named `review` from `captureReviewFailure` is replaced with `[redacted]` and the safe repo/PR context added at src/selfhost/sentry.ts:230 never reaches Sentry.

@JSONbored JSONbored closed this Jun 30, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

feat(observability): expand self-host Sentry context and monitors

2 participants