Skip to content

feat(observability): add sampled Sentry spans for review latency - #1909

Closed
jonathanchang31 wants to merge 2 commits into
JSONbored:mainfrom
jonathanchang31:feat-1734-sentry-review-traces
Closed

feat(observability): add sampled Sentry spans for review latency#1909
jonathanchang31 wants to merge 2 commits into
JSONbored:mainfrom
jonathanchang31:feat-1734-sentry-review-traces

Conversation

@jonathanchang31

Copy link
Copy Markdown
Contributor

Summary

  • Adds sampled Sentry tracing for the self-host review pipeline behind existing Sentry tracing config.
  • Keeps tracing opt-in: when SENTRY_TRACES_SAMPLE_RATE=0, behavior stays unchanged and no Sentry trace spans are emitted.
  • Connects major review-latency stages into one trace: webhook ingress, queue processing, GitHub API calls, REES requests, AI provider attempts, gate planning, and comment/check-run publishing.
  • Propagates trace context from webhook ingress into queued review jobs so slow or failed stages are visible end-to-end.
  • Keeps span attributes public-safe and low-cardinality.

Related Issue

Closes: #1734

Change Type

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

Real behavior proof

  • With SENTRY_TRACES_SAMPLE_RATE=0, tracing stays inert:
    • test/unit/selfhost-sentry.test.ts
  • With sampling enabled, review spans are created and trace headers are available:
    • test/unit/selfhost-sentry.test.ts
  • Webhook trace context is copied onto queued review jobs:
    • test/unit/webhook.test.ts
  • Full local validation passed:
    • npm run test:ci
    • npm audit --audit-level=moderate

Checklist

  • Issue scope matches #1734
  • Tracing remains opt-in
  • No prompts, diffs, tokens, headers, or private config attached to spans
  • Safe, bounded span attributes only
  • Tests added for off/on behavior and trace propagation
  • Full local CI passed
  • npm audit --audit-level=moderate passed

@dosubot dosubot Bot added the size:XL label 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.

#1904 already touches this issue, this is a duplicate, closing.

@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
@JSONbored JSONbored added the slop AI slop and/or attempts to game additional points via manipulation or alt profiles. label Jun 30, 2026
@loopover-orb

loopover-orb Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-06-30 21:08:11 UTC

17 files · 1 AI reviewer · no blockers · readiness 48/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
This adds a small adapter layer and wraps HTTP ingress, queue processing, GitHub/REES/AI/gate/publish stages in opt-in Sentry spans while preserving inert behavior when the sample rate is 0. The main propagation path is coherent: the self-host request stores current Sentry headers on the Request, webhook enqueue copies them onto JobMessage, and pg/sqlite consumers continue the queued trace while preserving the existing OTEL traceparent. I do not see a reachable correctness break in the changed code; the remaining issues are observability quality and coverage gaps rather than runtime defects.

Nits — 8 non-blocking
  • nit: src/github/client.ts:209 classifies several common review GitHub calls as the fallback "github-rest" bucket, including commit check-run listing and issue-comment PATCH paths, which makes the new latency spans less useful than intended.
  • nit: src/server.ts:691 reuses selfHostHttpRequestAttributes for Sentry without a visible assertion that those attributes exclude raw URLs, query strings, user agents, or other high-cardinality/request-specific values; please add or point to a test that locks that down for the new Sentry sink.
  • nit: src/selfhost/pg-queue.ts:362 and src/selfhost/sqlite-queue.ts:305 add queued Sentry trace continuation but only the enqueue side is covered in the visible tests; add one consumer-side test that verifies github-webhook messages pass sentryTrace/sentryBaggage into withReviewSpan.
  • src/github/client.ts:209: add explicit resource buckets for `/repos/{owner}/{repo}/commits/{ref}/check-runs` and `/repos/{owner}/{repo}/issues/comments/{comment_id}` so the new spans retain useful stage names.
  • src/server.ts:691: either sanitize `requestSpanAttributes` through a Sentry-specific allowlist or add a regression test proving only bounded route/status-style fields reach Sentry.
  • Pull request duplicates other open work — Check for an existing pull request or issue covering this change and coordinate or consolidate before continuing.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (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), 84 merged, 11 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jonathanchang31; Gittensor profile; 151 PR(s), 11 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: jonathanchang31
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 151 PR(s), 11 issue(s).
  • Related work: Titles/paths share 6 meaningful terms. (PR #1904)
  • Related work: Titles/paths share 6 meaningful terms. (PR #1904, PR #1907)
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
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. slop AI slop and/or attempts to game additional points via manipulation or alt profiles.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

observability(tracing): add sampled Sentry spans for review latency

2 participants