diff --git a/.env.example b/.env.example index 748b79164c..ba5606852d 100644 --- a/.env.example +++ b/.env.example @@ -230,7 +230,9 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # --- Sentry error tracking (optional) --- # SENTRY_DSN= # enables self-host Sentry capture; unset = complete no-op # SENTRY_ENVIRONMENT=production -# SENTRY_TRACES_SAMPLE_RATE=0 # traces are off by default; errors still report +# SENTRY_TRACES_SAMPLE_RATE=0 # traces stay opt-in; start with 0.02-0.05 to sample review latency +# # through webhook ingress, queue processing, GitHub calls, REES, AI +# # provider attempts, gate planning, and comment/check-run publishing # SENTRY_RELEASE= # custom images only: set this ONLY when you uploaded source maps for # # the exact built bundle under this exact release id. Future official # # images bake GITTENSORY_VERSION=gittensory-selfhost@, so do @@ -255,7 +257,7 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # SENTRY_DSN_FILE= # optional mounted secret file; existing *_FILE loader reads it # SENTRY_ENVIRONMENT=selfhost # SENTRY_RELEASE= -# SENTRY_TRACES_SAMPLE_RATE=0 +# SENTRY_TRACES_SAMPLE_RATE=0 # keep 0 for error-only mode; raise to 0.02-0.05 for sampled traces # --- AI review backend (optional; without AI_PROVIDER reviews run deterministically) --- # AI_SUMMARIES_ENABLED=true diff --git a/src/github/app.ts b/src/github/app.ts index 70bf258ba6..98409f064f 100644 --- a/src/github/app.ts +++ b/src/github/app.ts @@ -10,6 +10,7 @@ import { timeoutFetch, } from "./client"; import { maintainerControlPanelUrl } from "./footer"; +import { withReviewSpan } from "../observability/review-trace"; import type { AgentActionMode } from "../settings/agent-execution"; import { signRs256Jwt } from "../utils/crypto"; import { errorMessage } from "../utils/json"; @@ -581,14 +582,22 @@ async function createOrUpdateNamedCheckRun( if (!owner || !repo) throw new Error(`Invalid repository full name: ${repoFullName}`); - return await withInstallationTokenRetry(env, installationId, async (token) => { - // makeInstallationOctokit injects the shared per-request timeout (a stalled PATCH can never orphan the - // in_progress check) AND suppresses the check-run writes under a non-live mode (dry-run / pause / freeze). - const octokit = makeInstallationOctokit(env, token, check.mode, githubRateLimitAdmissionKeyForInstallation(installationId)); - // Point the merge-box "Details" link at the repo's Gittensory maintainer panel instead of GitHub's generic - // check page. Spread conditionally so a URL-construction failure (null) just omits it. (#audit-details-url) - const detailsUrl = maintainerControlPanelUrl(env, repoFullName); - const detailsUrlBody = detailsUrl ? { details_url: detailsUrl } : {}; + return await withReviewSpan( + "review.publish.check_run", + { + repo: repoFullName, + name: check.name, + conclusion: check.conclusion ?? advisory.conclusion, + }, + async () => + await withInstallationTokenRetry(env, installationId, async (token) => { + // makeInstallationOctokit injects the shared per-request timeout (a stalled PATCH can never orphan the + // in_progress check) AND suppresses the check-run writes under a non-live mode (dry-run / pause / freeze). + const octokit = makeInstallationOctokit(env, token, check.mode, githubRateLimitAdmissionKeyForInstallation(installationId)); + // Point the merge-box "Details" link at the repo's Gittensory maintainer panel instead of GitHub's generic + // check page. Spread conditionally so a URL-construction failure (null) just omits it. (#audit-details-url) + const detailsUrl = maintainerControlPanelUrl(env, repoFullName); + const detailsUrlBody = detailsUrl ? { details_url: detailsUrl } : {}; // POST a fresh check-run THIS App owns. Used for a brand-new run AND as the cross-app fallback below. const postNewCheckRun = async (): Promise => { @@ -699,62 +708,60 @@ async function createOrUpdateNamedCheckRun( return outcome; }; - try { - if (check.checkRunId) { - const out = await patchCheckRun(check.checkRunId); - if (out) return await finish(out); - } else if (check.updateExisting !== "never") { - const existing = await octokit.request( - "GET /repos/{owner}/{repo}/commits/{ref}/check-runs", - { - owner, - repo, - ref: headSha, - check_name: check.name, - filter: "latest", - per_page: 1, - }, - ); - const existingCheckRun = (existing.data as CheckRunListResponse) - .check_runs?.[0]; - if ( - existingCheckRun && - (check.updateExisting !== "in_progress_only" || - (existingCheckRun.status ?? "").toLowerCase() !== "completed") - ) { - const out = await patchCheckRun(existingCheckRun.id); - if (out) return await finish(out); + try { + if (check.checkRunId) { + const out = await patchCheckRun(check.checkRunId); + if (out) return await finish(out); + } else if (check.updateExisting !== "never") { + const existing = await octokit.request( + "GET /repos/{owner}/{repo}/commits/{ref}/check-runs", + { + owner, + repo, + ref: headSha, + check_name: check.name, + filter: "latest", + per_page: 1, + }, + ); + const existingCheckRun = (existing.data as CheckRunListResponse) + .check_runs?.[0]; + if ( + existingCheckRun && + (check.updateExisting !== "in_progress_only" || + (existingCheckRun.status ?? "").toLowerCase() !== "completed") + ) { + const out = await patchCheckRun(existingCheckRun.id); + if (out) return await finish(out); + } + } + return await finish(await postNewCheckRun()); + } catch (error) { + if (isCheckRunPermissionError(error)) { + const e = error as { status?: number; message?: string }; + console.error( + JSON.stringify({ + level: "error", + event: "check_run_post_denied", + repository: `${owner}/${repo}`, + status: e.status ?? null, + message: (e.message ?? "Resource not accessible by integration").slice( + 0, + 300, + ), + }), + ); + return { + kind: "permission_missing", + warning: + "GitHub App Checks: write permission is missing. Enable it in the GitHub App settings and re-approve the installation.", + }; + } + throw error; } - } - return await finish(await postNewCheckRun()); - } catch (error) { - if (isCheckRunPermissionError(error)) { - // Capture the ACTUAL response (status + body). A 403 here is often NOT a real permission gap (the App has - // Checks:write) — it can be a per-PR access quirk (e.g. a fork-head commit the App can't write to) — and this - // log is the only way to tell why, instead of an opaque "permission missing". Surfaces to Sentry with a real - // message via console.error (#review-403-context). - const e = error as { status?: number; message?: string }; - console.error( - JSON.stringify({ - level: "error", - event: "check_run_post_denied", - repository: `${owner}/${repo}`, - status: e.status ?? null, - message: (e.message ?? "Resource not accessible by integration").slice( - 0, - 300, - ), - }), - ); - return { - kind: "permission_missing", - warning: - "GitHub App Checks: write permission is missing. Enable it in the GitHub App settings and re-approve the installation.", - }; - } - throw error; - } - }); + }), + { op: "github.check_run" }, + ); } function outputForCheckRunUpdate(output: CheckRunOutput): CheckRunOutput { diff --git a/src/github/client.ts b/src/github/client.ts index 890a498513..dd3a20e675 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -1,4 +1,5 @@ import { Octokit } from "@octokit/core"; +import { withReviewSpan } from "../observability/review-trace"; import { isGlobalAgentFrozen, recordAuditEvent } from "../db/repositories"; import { isGlobalAgentPause, resolveAgentActionMode, type AgentActionMode } from "../settings/agent-execution"; import { incr } from "../selfhost/metrics"; @@ -205,6 +206,19 @@ function rateLimitAdmissionKey(init: GitHubTimeoutFetchInit | undefined): GitHub return key ? key : null; } +function githubSpanResource(url: string): string { + if (!url.startsWith(`${GITHUB_API_PREFIX}/`)) return "external"; + const path = githubApiPath(url); + if (/^\/repos\/[^/]+\/[^/]+\/check-runs(?:$|[/?#])/.test(path)) return "check-runs"; + if (/^\/repos\/[^/]+\/[^/]+\/issues\/\d+\/comments(?:$|[/?#])/.test(path)) return "issue-comments"; + if (/^\/repos\/[^/]+\/[^/]+\/pulls(?:$|\/)/.test(path)) return "pulls"; + if (/^\/repos\/[^/]+\/[^/]+\/branches\/[^/]+\/protection\/required_status_checks(?:$|[?#])/.test(path)) return "required-status-checks"; + if (/^\/repos\/[^/]+\/[^/]+(?:$|[?#])/.test(path)) return "repository"; + if (/^\/app\/installations\/\d+(?:$|[?#])/.test(path)) return "app-installations"; + if (/^\/users\/[^/?#]+(?:$|[?#])/.test(path)) return "users"; + return "github-rest"; +} + function requestInitForFetch(init: GitHubTimeoutFetchInit | undefined): RequestInit | undefined { if (!init || (!("githubRateLimitAdmission" in init) && !("githubRateLimitAdmissionKey" in init))) return init; const { githubRateLimitAdmission: _omitted, githubRateLimitAdmissionKey: _omittedKey, ...rest } = init; @@ -274,22 +288,43 @@ async function replayableResponse(response: Response): Promise { - let response: Response; - const fetchInit = requestInitForFetch(init); - const admissionKey = rateLimitAdmissionKey(init); - for (let attempt = 0; ; attempt += 1) { - response = fetchInit?.signal - ? await fetch(input, fetchInit) - : await fetch(input, { - ...(fetchInit ?? {}), - signal: AbortSignal.timeout(GITHUB_FETCH_TIMEOUT_MS), - }); - if (admissionKey) observeGitHubRestRateLimit(requestUrl(input), response, admissionKey); - // Retry a transient rate-limit (with backoff) instead of surfacing it; stop once exhausted or it's not a limit. - if (attempt >= GITHUB_RATE_LIMIT_MAX_RETRIES || !(await isRateLimitedResponse(response))) break; - await sleep(rateLimitRetryMs(response, attempt)); - } - return response; + const url = requestUrl(input); + const method = requestMethod(input, init); + const headers = requestHeaders(input, init); + const conditional = hasConditionalRequestHeader(headers); + const cls = method === "GET" && !conditional ? githubCacheClassForUrl(url) : null; + return await withReviewSpan( + "github.request", + { + "github.request.method": method, + "github.request.resource": githubSpanResource(url), + "github.request.cache_class": cls ?? "volatile", + }, + async () => { + let response: Response; + const fetchInit = requestInitForFetch(init); + const admissionKey = rateLimitAdmissionKey(init); + for (let attempt = 0; ; attempt += 1) { + response = fetchInit?.signal + ? await fetch(input, fetchInit) + : await fetch(input, { + ...(fetchInit ?? {}), + signal: AbortSignal.timeout(GITHUB_FETCH_TIMEOUT_MS), + }); + if (admissionKey) observeGitHubRestRateLimit(url, response, admissionKey); + // Retry a transient rate-limit (with backoff) instead of surfacing it; stop once exhausted or it's not a limit. + if ( + attempt >= GITHUB_RATE_LIMIT_MAX_RETRIES || + !(await isRateLimitedResponse(response)) + ) { + break; + } + await sleep(rateLimitRetryMs(response, attempt)); + } + return response; + }, + { op: "http.client" }, + ); } async function fetchAndMaybeCacheGitHubGet( diff --git a/src/github/comments.ts b/src/github/comments.ts index 2b519f1b74..8a5fb2c08d 100644 --- a/src/github/comments.ts +++ b/src/github/comments.ts @@ -1,3 +1,4 @@ +import { withReviewSpan } from "../observability/review-trace"; import { withInstallationTokenRetry } from "./app"; import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client"; import type { AgentActionMode } from "../settings/agent-execution"; @@ -53,52 +54,54 @@ async function createOrUpdateIssueCommentWithMarker( const [owner, repo] = repoFullName.split("/"); if (!owner || !repo) throw new Error(`Invalid repository full name: ${repoFullName}`); - return await withInstallationTokenRetry(env, installationId, async (token) => { - // Non-live mode suppresses the comment create/update writes; the GET marker-search probe below still runs. - const octokit = makeInstallationOctokit(env, token, options.mode ?? "live", githubRateLimitAdmissionKeyForInstallation(installationId)); - const botLogin = `${env.GITHUB_APP_SLUG}[bot]`; - const markers = markerAliases(marker); - const existing: IssueComment[] = []; - for (let page = 1; page <= COMMENT_SEARCH_PAGE_LIMIT; page += 1) { - const response = await octokit.request("GET /repos/{owner}/{repo}/issues/{issue_number}/comments", { - owner, - repo, - issue_number: issueNumber, - per_page: 100, - page, - }); - const batch = response.data as IssueComment[]; - existing.push(...batch.filter((comment) => isGittensoryBotComment(comment, botLogin) && markers.some((candidate) => comment.body?.includes(candidate)))); - if (batch.length < 100) break; - } - const canonical = canonicalMarkerComment(existing); - if (canonical) { - // Idempotency (#4): skip the PATCH when the rendered body is byte-identical to what's already posted. The - // re-gate sweep re-renders the same surface every cycle for an unchanged PR; without this, every cycle PATCHes - // GitHub (a write + rate-limit cost) for no visible change. Defense-in-depth alongside the head_sha publish - // marker — also collapses a duplicate webhook delivery for the same commit. - if (canonical.body === body) { - await deleteDuplicateMarkerComments(octokit, owner, repo, existing, canonical.id); - return { id: canonical.id, ...(canonical.html_url !== undefined ? { html_url: canonical.html_url } : {}) }; - } - const response = await octokit.request("PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}", { - owner, - repo, - comment_id: canonical.id, - body, - }); - await deleteDuplicateMarkerComments(octokit, owner, repo, existing, canonical.id); - return response.data as { id: number; html_url?: string }; - } - if (options.createIfMissing === false) return null; - const response = await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/comments", { - owner, - repo, - issue_number: issueNumber, - body, - }); - return response.data as { id: number; html_url?: string }; - }); + return await withReviewSpan( + "review.publish.comment", + { repo: repoFullName, pr: issueNumber }, + async () => + await withInstallationTokenRetry(env, installationId, async (token) => { + // Non-live mode suppresses the comment create/update writes; the GET marker-search probe below still runs. + const octokit = makeInstallationOctokit(env, token, options.mode ?? "live", githubRateLimitAdmissionKeyForInstallation(installationId)); + const botLogin = `${env.GITHUB_APP_SLUG}[bot]`; + const markers = markerAliases(marker); + const existing: IssueComment[] = []; + for (let page = 1; page <= COMMENT_SEARCH_PAGE_LIMIT; page += 1) { + const response = await octokit.request("GET /repos/{owner}/{repo}/issues/{issue_number}/comments", { + owner, + repo, + issue_number: issueNumber, + per_page: 100, + page, + }); + const batch = response.data as IssueComment[]; + existing.push(...batch.filter((comment) => isGittensoryBotComment(comment, botLogin) && markers.some((candidate) => comment.body?.includes(candidate)))); + if (batch.length < 100) break; + } + const canonical = canonicalMarkerComment(existing); + if (canonical) { + if (canonical.body === body) { + await deleteDuplicateMarkerComments(octokit, owner, repo, existing, canonical.id); + return { id: canonical.id, ...(canonical.html_url !== undefined ? { html_url: canonical.html_url } : {}) }; + } + const response = await octokit.request("PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}", { + owner, + repo, + comment_id: canonical.id, + body, + }); + await deleteDuplicateMarkerComments(octokit, owner, repo, existing, canonical.id); + return response.data as { id: number; html_url?: string }; + } + if (options.createIfMissing === false) return null; + const response = await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/comments", { + owner, + repo, + issue_number: issueNumber, + body, + }); + return response.data as { id: number; html_url?: string }; + }), + { op: "github.comment" }, + ); } function isGittensoryBotComment(comment: IssueComment, botLogin: string): boolean { diff --git a/src/github/webhook.ts b/src/github/webhook.ts index 07ef4656fd..360f74163f 100644 --- a/src/github/webhook.ts +++ b/src/github/webhook.ts @@ -5,7 +5,10 @@ import { sha256Hex, verifyGitHubSignature } from "../utils/crypto"; import { parsePositiveInt } from "../utils/json"; import { relayVerify } from "../orb/relay"; import { isSelfHostedReviewRuntime } from "../selfhost/review-runtime"; -import { getSelfHostRequestTraceParent } from "../selfhost/trace-context"; +import { + getSelfHostRequestReviewTraceHeaders, + getSelfHostRequestTraceParent, +} from "../selfhost/trace-context"; import { isNonActionableWebhookNoise } from "./self-authored"; const DEFAULT_MAX_WEBHOOK_BODY_BYTES = 1024 * 1024; @@ -39,7 +42,16 @@ export async function handleGitHubWebhook(c: Context<{ Bindings: Env }>): Promis * webhook receiver above AND the Orb relay receiver below (they verify the body differently — GitHub's HMAC vs the * Orb relay HMAC — then share everything after). */ export async function enqueueVerifiedWebhook(c: Context<{ Bindings: Env }>, deliveryId: string, eventName: string, rawBody: string): Promise { - const result = await enqueueWebhookByEnv(c.env, deliveryId, eventName, rawBody, getSelfHostRequestTraceParent(c.req.raw)); + const traceHeaders = getSelfHostRequestReviewTraceHeaders(c.req.raw); + const result = await enqueueWebhookByEnv( + c.env, + deliveryId, + eventName, + rawBody, + getSelfHostRequestTraceParent(c.req.raw), + traceHeaders?.sentryTrace, + traceHeaders?.baggage, + ); switch (result) { case "review_unavailable": return c.json({ error: "selfhost_review_runtime_required" }, 410); @@ -66,7 +78,15 @@ export type EnqueueWebhookResult = "queued" | "duplicate" | "ignored" | "invalid * webhooks at /v1/orb/webhook and forwards/pends them for registered self-host engines. Direct review execution * now requires the self-host runtime cache so stale Cloudflare review-webhook traffic fails loudly instead of being * accepted into a Worker path that no longer performs reviews. */ -export async function enqueueWebhookByEnv(env: Env, deliveryId: string, eventName: string, rawBody: string, traceParent?: string): Promise { +export async function enqueueWebhookByEnv( + env: Env, + deliveryId: string, + eventName: string, + rawBody: string, + traceParent?: string, + sentryTrace?: string, + sentryBaggage?: string, +): Promise { if (!isSelfHostedReviewRuntime(env)) return "review_unavailable"; let payload: GitHubWebhookPayload; @@ -105,7 +125,15 @@ export async function enqueueWebhookByEnv(env: Env, deliveryId: string, eventNam await recordWebhookEvent(env, { ...eventRow, status: "queued" }); - const message: JobMessage = { type: "github-webhook", deliveryId, eventName, payload, ...(traceParent ? { traceParent } : {}) }; + const message: JobMessage = { + type: "github-webhook", + deliveryId, + eventName, + payload, + ...(traceParent ? { traceParent } : {}), + ...(sentryTrace ? { sentryTrace } : {}), + ...(sentryBaggage ? { sentryBaggage } : {}), + }; try { // Send to the dedicated WEBHOOKS lane (not the shared JOBS queue) so a maintenance burst on JOBS can never // starve real GitHub events into the DLQ. (#audit-webhook-queue) diff --git a/src/observability/review-trace.ts b/src/observability/review-trace.ts new file mode 100644 index 0000000000..13437f3821 --- /dev/null +++ b/src/observability/review-trace.ts @@ -0,0 +1,44 @@ +export type ReviewTraceHeaders = { + sentryTrace?: string | undefined; + baggage?: string | undefined; +}; + +export type ReviewSpanOptions = { + forceTransaction?: boolean | undefined; + op?: string | undefined; + parent?: ReviewTraceHeaders | undefined; +}; + +type ReviewTraceAdapter = { + withSpan( + name: string, + attributes: Record | undefined, + fn: () => T | Promise, + options?: ReviewSpanOptions, + ): Promise; + currentTraceHeaders(): ReviewTraceHeaders | undefined; +}; + +let adapter: ReviewTraceAdapter | null = null; + +export function setReviewTraceAdapter(next: ReviewTraceAdapter | null): void { + adapter = next; +} + +export async function withReviewSpan( + name: string, + attributes: Record | undefined, + fn: () => T | Promise, + options?: ReviewSpanOptions, +): Promise { + if (!adapter) return await fn(); + return await adapter.withSpan(name, attributes, fn, options); +} + +export function currentReviewTraceHeaders(): ReviewTraceHeaders | undefined { + return adapter?.currentTraceHeaders(); +} + +export function resetReviewTraceAdapterForTest(): void { + adapter = null; +} diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 2a8fbe55bb..0eae2b6fe9 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -203,6 +203,7 @@ import { delayUntil, shouldWaitForGitHubRateLimit, } from "../github/rate-limit"; +import { withReviewSpan } from "../observability/review-trace"; import { downgradeCloseToHold, downgradeMergeToHold, @@ -1540,56 +1541,62 @@ async function maybeRunAgentMaintenance( settings.contributorBlacklist, ); - const planned = planAgentMaintenanceActions({ - conclusion: gate.conclusion, - blockerTitles: gate.blockers.map((blocker) => blocker.title), - // Public-safe finding identifiers retained for telemetry/action reasons. They no longer refute a blocker on - // green CI; once the gate says failure, the close/hold decision follows that verdict. - gateBlockerCodes: gate.blockers.map((blocker) => blocker.code), - autonomy: settings.autonomy, - autoMaintain: settings.autoMaintain, - slopGateMinScore: settings.slopGateMinScore, - changedPaths, - hardGuardrailGlobs, - authorIsOwner, - authorIsAutomationBot, - closeOwnerAuthors: settings.closeOwnerAuthors, - ciState: ciAggregate.ciState, - failingCheckNames: ciAggregate.failingDetails.map((detail) => detail.name), - ciRequiredContextsVerified: hasVerifiedRequiredContexts(requiredContexts), - ...(blacklistEntry !== null - ? { blacklistMatch: { matched: true, reason: blacklistEntry.reason } } - : {}), - // Always threaded (the DB layer populates it, default "slop"); the planner applies its own fallback. - blacklistLabel: settings.blacklistLabel, - ...(linkedIssueHardRule !== undefined ? { linkedIssueHardRule } : {}), - // Flag-then-close double-check: thread the loaded verify config so the planner FLAGS first then closes on - // re-verification (default ON). Only passed when a rule is on (the planner reads it only for a violation). - linkedIssueVerify: { - verifyBeforeClose: linkedIssueRulesConfig.verifyBeforeClose, - closeDelaySeconds: linkedIssueRulesConfig.closeDelaySeconds, - }, - pr: { - mergeableState: liveMergeState ?? pr.mergeableState, - reviewDecision: liveReviewDecision ?? pr.reviewDecision, - slopRisk: pr.slopRisk, - labels: pr.labels, - // Duplicate-winner adjudication (#dup-winner): the gate's open-only duplicate siblings drive the close - // reason ("duplicate of another open PR" via agent-actions when count > 0). When the flag is ON and this - // PR is the cluster winner, force the count to 0 so the winner's close reason OMITS the duplicate cause - // (it can still close on its own merits — CI/conflict/blockers). Flag-OFF short-circuits ⇒ the real - // count is used (byte-identical). Sparse legacy rows fail closed so duplicate evidence remains visible. - linkedDuplicateCount: dupWinnerLinkedDuplicateCount( - linkedIssueDuplicatePullRequestRecordsForGate(pr, otherOpenPullRequests), - pr.number, - pr.linkedIssueClaimedAt, - env.GITTENSORY_DUPLICATE_WINNER === "true", - ), - headSha: pr.headSha, - mergeBlockedSha: pr.mergeBlockedSha, - approvedHeadSha: pr.approvedHeadSha, - }, - }); + const planned = await withReviewSpan( + "review.gate.actions", + { repo: repoFullName, pr: pr.number, gateConclusion: gate.conclusion }, + async () => + planAgentMaintenanceActions({ + conclusion: gate.conclusion, + blockerTitles: gate.blockers.map((blocker) => blocker.title), + // Public-safe finding identifiers retained for telemetry/action reasons. They no longer refute a blocker on + // green CI; once the gate says failure, the close/hold decision follows that verdict. + gateBlockerCodes: gate.blockers.map((blocker) => blocker.code), + autonomy: settings.autonomy, + autoMaintain: settings.autoMaintain, + slopGateMinScore: settings.slopGateMinScore, + changedPaths, + hardGuardrailGlobs, + authorIsOwner, + authorIsAutomationBot, + closeOwnerAuthors: settings.closeOwnerAuthors, + ciState: ciAggregate.ciState, + failingCheckNames: ciAggregate.failingDetails.map((detail) => detail.name), + ciRequiredContextsVerified: hasVerifiedRequiredContexts(requiredContexts), + ...(blacklistEntry !== null + ? { blacklistMatch: { matched: true, reason: blacklistEntry.reason } } + : {}), + // Always threaded (the DB layer populates it, default "slop"); the planner applies its own fallback. + blacklistLabel: settings.blacklistLabel, + ...(linkedIssueHardRule !== undefined ? { linkedIssueHardRule } : {}), + // Flag-then-close double-check: thread the loaded verify config so the planner FLAGS first then closes on + // re-verification (default ON). Only passed when a rule is on (the planner reads it only for a violation). + linkedIssueVerify: { + verifyBeforeClose: linkedIssueRulesConfig.verifyBeforeClose, + closeDelaySeconds: linkedIssueRulesConfig.closeDelaySeconds, + }, + pr: { + mergeableState: liveMergeState ?? pr.mergeableState, + reviewDecision: liveReviewDecision ?? pr.reviewDecision, + slopRisk: pr.slopRisk, + labels: pr.labels, + // Duplicate-winner adjudication (#dup-winner): the gate's open-only duplicate siblings drive the close + // reason ("duplicate of another open PR" via agent-actions when count > 0). When the flag is ON and this + // PR is the cluster winner, force the count to 0 so the winner's close reason OMITS the duplicate cause + // (it can still close on its own merits — CI/conflict/blockers). Flag-OFF short-circuits ⇒ the real + // count is used (byte-identical). Sparse legacy rows fail closed so duplicate evidence remains visible. + linkedDuplicateCount: dupWinnerLinkedDuplicateCount( + linkedIssueDuplicatePullRequestRecordsForGate(pr, otherOpenPullRequests), + pr.number, + pr.linkedIssueClaimedAt, + env.GITTENSORY_DUPLICATE_WINNER === "true", + ), + headSha: pr.headSha, + mergeBlockedSha: pr.mergeBlockedSha, + approvedHeadSha: pr.approvedHeadSha, + }, + }), + { op: "gate.actions" }, + ); // Accuracy circuit-breakers (#self-improve / GAP-4): two INDEPENDENT, fail-open precision breakers, chained. // • MERGE breaker (holdonly:): when set, convert a would-MERGE into a human HOLD before executing. // • CLOSE breaker (closehold:): when set, convert a HEURISTIC would-CLOSE into a human HOLD (the @@ -5090,7 +5097,12 @@ async function maybePublishPrPublicSurface( gateSizeContext, ); gateEvaluation = gateEnabled - ? evaluateGateCheck(advisory, gatePolicy) + ? await withReviewSpan( + "review.gate.plan", + { repo: repoFullName, pr: pr.number, advisoryConclusion: advisory.conclusion }, + async () => evaluateGateCheck(advisory, gatePolicy), + { op: "gate.plan" }, + ) : undefined; // Deterministic content/registry surface lane (#1255) — flag-gated + per-repo allowlist, byte-identical when // off (evaluateWithSurfaceLane returns the generic evaluation unchanged and resolves no files). A metagraphed diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index e970979ef3..33179ae3c8 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -7,6 +7,7 @@ // — is unaffected). Default OFF → gathers nothing, prompt byte-identical. FULLY FAIL-SAFE: any timeout / non-200 / // network / parse error, or an empty brief, returns undefined and the review proceeds on diff + grounding + RAG. import { extractLinkedIssueNumbers, getIssue } from "../db/repositories"; +import { withReviewSpan } from "../observability/review-trace"; import { sanitizePublicComment } from "../queue-intelligence"; import { neutralizePromptInjection } from "./prompt-injection"; import type { PullRequestFileRecord } from "../types"; @@ -254,44 +255,55 @@ export async function buildReviewEnrichment( const profile = resolveReesProfile(env); const requestId = newReesRequestId(); try { - const response = await fetch(`${base.replace(/\/+$/, "")}/v1/enrich`, { - method: "POST", - headers: { - "user-agent": "gittensory-selfhost/1.0", - accept: "application/json", - "content-type": "application/json", - "x-gittensory-request-id": requestId, - ...(sharedSecret ? { authorization: `Bearer ${sharedSecret}` } : {}), + const response = await withReviewSpan( + "review.enrichment.request", + { + repo: input.repoFullName, + pr: input.prNumber, + reesProfile: profile ?? "default", + requestedAnalyzers: analyzers?.length ?? REES_ANALYZER_NAMES.length, }, - body: JSON.stringify({ - repoFullName: input.repoFullName, - prNumber: input.prNumber, - headSha: input.headSha, - baseSha: input.baseSha ?? null, - title: input.title, - ...(input.body ? { body: input.body } : {}), - author: input.author ?? undefined, - ...(input.linkedIssue ? { linkedIssue: input.linkedIssue } : {}), - ...(input.githubToken ? { githubToken: input.githubToken } : {}), - files: input.files.map((file) => ({ - path: file.path, - status: file.status ?? undefined, - previousPath: file.previousFilename ?? undefined, - patch: - typeof file.payload?.patch === "string" - ? file.payload.patch - : undefined, - })), - diff: input.diff, - ...(analyzers ? { analyzers } : {}), - ...(profile ? { profile } : {}), - budget: { - timeoutMs: analyzerBudgetMs, - maxBriefChars: MAX_ENRICHMENT_PROMPT_SECTION_CHARS, - }, - }), - signal: AbortSignal.timeout(timeoutMs), - }); + async () => + await fetch(`${base.replace(/\/+$/, "")}/v1/enrich`, { + method: "POST", + headers: { + "user-agent": "gittensory-selfhost/1.0", + accept: "application/json", + "content-type": "application/json", + "x-gittensory-request-id": requestId, + ...(sharedSecret ? { authorization: `Bearer ${sharedSecret}` } : {}), + }, + body: JSON.stringify({ + repoFullName: input.repoFullName, + prNumber: input.prNumber, + headSha: input.headSha, + baseSha: input.baseSha ?? null, + title: input.title, + ...(input.body ? { body: input.body } : {}), + author: input.author ?? undefined, + ...(input.linkedIssue ? { linkedIssue: input.linkedIssue } : {}), + ...(input.githubToken ? { githubToken: input.githubToken } : {}), + files: input.files.map((file) => ({ + path: file.path, + status: file.status ?? undefined, + previousPath: file.previousFilename ?? undefined, + patch: + typeof file.payload?.patch === "string" + ? file.payload.patch + : undefined, + })), + diff: input.diff, + ...(analyzers ? { analyzers } : {}), + ...(profile ? { profile } : {}), + budget: { + timeoutMs: analyzerBudgetMs, + maxBriefChars: MAX_ENRICHMENT_PROMPT_SECTION_CHARS, + }, + }), + signal: AbortSignal.timeout(timeoutMs), + }), + { op: "http.client" }, + ); if (!response.ok) { const bodyPreview = await response.text().catch(() => ""); // A non-2xx from REES (auth/5xx/bad-gateway) silently degraded the review to no-enrichment with no signal. diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 77c67b0068..ad63f7776c 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -11,6 +11,7 @@ import { isConfiguredSelfHostProvider, resolveConfiguredProviderNames } from "./ export { assertNoLegacySharedAiEnv } from "./ai-config"; import { incr } from "./metrics"; import { withOtelSpan } from "./otel"; +import { withReviewSpan } from "../observability/review-trace"; import { delimiter } from "node:path"; interface AiRunOptions { @@ -627,10 +628,21 @@ function runProviderWithOtel( model: string, options: AiRunOptions, ): Promise { - return withOtelSpan( + const spanAttributes = { + "ai.provider": provider.name, + "ai.model": model || "default", + "ai.request_kind": requestKind(options), + }; + return withReviewSpan( "selfhost.ai.provider", - { "ai.provider": provider.name, "ai.model": model || "default", "ai.request_kind": requestKind(options) }, - () => provider.ai.run(model, options), + spanAttributes, + async () => + await withOtelSpan( + "selfhost.ai.provider", + spanAttributes, + () => provider.ai.run(model, options), + ), + { op: "ai.run" }, ); } diff --git a/src/selfhost/pg-queue.ts b/src/selfhost/pg-queue.ts index de7fb3115e..d627675365 100644 --- a/src/selfhost/pg-queue.ts +++ b/src/selfhost/pg-queue.ts @@ -7,6 +7,7 @@ import { logAudit, extractPayloadType } from "./audit"; import { incr } from "./metrics"; import { withOtelSpan } from "./otel"; import { captureError } from "./sentry"; +import { withReviewSpan } from "../observability/review-trace"; import { consumingRetryDelayMs, deterministicJitterMs, @@ -361,11 +362,36 @@ export function createPgQueue( return true; } try { - await withOtelSpan( + const spanAttributes = { + "job.type": message.type, + "queue.backend": "postgres", + "job.attempt": Number(job.attempts) + 1, + }; + await withReviewSpan( "selfhost.queue.job", - { "job.type": message.type, "queue.backend": "postgres", "job.attempt": Number(job.attempts) + 1 }, - () => consume(message), - { parentTraceParent: message.type === "github-webhook" ? message.traceParent : undefined }, + spanAttributes, + async () => + await withOtelSpan( + "selfhost.queue.job", + spanAttributes, + () => consume(message), + { + parentTraceParent: + message.type === "github-webhook" + ? message.traceParent + : undefined, + }, + ), + { + op: "queue.process", + parent: + message.type === "github-webhook" + ? { + sentryTrace: message.sentryTrace, + baggage: message.sentryBaggage, + } + : undefined, + }, ); await pool.query(`DELETE FROM ${TABLE} WHERE id=$1`, [job.id]); await recordQueueMetric("gittensory_jobs_processed_total"); diff --git a/src/selfhost/sentry.ts b/src/selfhost/sentry.ts index ecd5957e55..cd892230f0 100644 --- a/src/selfhost/sentry.ts +++ b/src/selfhost/sentry.ts @@ -2,6 +2,10 @@ // env-gated, dynamically-imported selfhost-integration pattern (Redis/Qdrant/embed-provider in server.ts). // @sentry/node is NEVER imported at module top level — it loads lazily inside initSentry(), so it never enters // the Worker bundle (src/index.ts) and cloudflare:* stubbing stays clean. All helpers are safe to call when off. +import { + setReviewTraceAdapter, + type ReviewSpanOptions, +} from "../observability/review-trace"; import { currentOtelTraceIds } from "./otel"; type SentryNs = typeof import("@sentry/node"); @@ -13,7 +17,9 @@ type SentryScope = { }; let Sentry: SentryNs | undefined; let active = false; +let tracingActive = false; let sentryEnvironment = "production"; +const MAX_ATTRIBUTE_LENGTH = 160; const SECRET_KEY = /(token|secret|key|password|passwd|authorization|auth|dsn|cookie|bearer|credential|private)/i; @@ -89,6 +95,76 @@ function safeMonitorContext( return safe; } +function safeSpanAttributes( + attributes: Record | undefined, +): Record { + const safe: Record = {}; + if (!attributes) return safe; + for (const [key, value] of Object.entries(attributes)) { + if (SECRET_KEY.test(key) || value === null || value === undefined) continue; + if (typeof value === "string") { + safe[key] = + value.length > MAX_ATTRIBUTE_LENGTH + ? `${value.slice(0, MAX_ATTRIBUTE_LENGTH - 3)}...` + : value; + } else if (typeof value === "number" && Number.isFinite(value)) safe[key] = value; + else if (typeof value === "boolean") safe[key] = value; + } + return safe; +} + +function traceSampleRate(value: string | undefined): number { + const parsed = Number(value ?? "0"); + if (!Number.isFinite(parsed)) return 0; + return Math.max(0, Math.min(1, parsed)); +} + +async function withSentryTraceSpan( + name: string, + attributes: Record | undefined, + fn: () => T | Promise, + options?: ReviewSpanOptions, +): Promise { + if (!active || !tracingActive || !Sentry) return await fn(); + const run = async (): Promise => + await Sentry!.startSpan( + { + name, + ...(options?.op ? { op: options.op } : {}), + ...(options?.forceTransaction ? { forceTransaction: true } : {}), + attributes: safeSpanAttributes(attributes), + }, + async () => await fn(), + ); + const parentTrace = options?.parent?.sentryTrace; + if (!parentTrace) return await run(); + return await Sentry.continueTrace( + { + sentryTrace: parentTrace, + baggage: options?.parent?.baggage, + }, + run, + ); +} + +function installTraceAdapter(): void { + if (!tracingActive || !Sentry) { + setReviewTraceAdapter(null); + return; + } + setReviewTraceAdapter({ + withSpan: withSentryTraceSpan, + currentTraceHeaders() { + const span = Sentry!.getActiveSpan(); + if (!span) return undefined; + return { + sentryTrace: Sentry!.spanToTraceHeader(span), + baggage: Sentry!.spanToBaggageHeader(span), + }; + }, + }); +} + function setOtelTraceScope(scope: SentryScope): void { const trace = currentOtelTraceIds(); if (!trace) return; @@ -131,19 +207,25 @@ export function scrubEvent(event: T): T { /** Initialize Sentry from the environment. Returns false (and stays a no-op) when SENTRY_DSN is unset. */ export async function initSentry(env: NodeJS.ProcessEnv): Promise { - if (!env.SENTRY_DSN) return false; + if (!env.SENTRY_DSN) { + setReviewTraceAdapter(null); + return false; + } Sentry = await import("@sentry/node"); const release = resolveSentryRelease(env); sentryEnvironment = nonBlank(env.SENTRY_ENVIRONMENT) ?? "production"; + const tracesSampleRate = traceSampleRate(env.SENTRY_TRACES_SAMPLE_RATE); Sentry.init({ dsn: env.SENTRY_DSN, environment: sentryEnvironment, ...(release ? { release } : {}), - tracesSampleRate: Number(env.SENTRY_TRACES_SAMPLE_RATE ?? "0"), + tracesSampleRate, serverName: env.PUBLIC_API_ORIGIN, beforeSend: (e) => scrubEvent(e), }); active = true; + tracingActive = tracesSampleRate > 0; + installTraceAdapter(); return true; } @@ -363,7 +445,9 @@ export async function flushSentry(timeoutMs = 2000): Promise { export function resetSentryForTest(): void { Sentry = undefined; active = false; + tracingActive = false; sentryEnvironment = "production"; + setReviewTraceAdapter(null); } interface StructuredLogConsole { diff --git a/src/selfhost/sqlite-queue.ts b/src/selfhost/sqlite-queue.ts index 1173cd8eb9..5f79b4c3b1 100644 --- a/src/selfhost/sqlite-queue.ts +++ b/src/selfhost/sqlite-queue.ts @@ -8,6 +8,7 @@ import { logAudit, extractPayloadType } from "./audit"; import { incr } from "./metrics"; import { withOtelSpan } from "./otel"; import { captureError } from "./sentry"; +import { withReviewSpan } from "../observability/review-trace"; import { consumingRetryDelayMs, deterministicJitterMs, @@ -304,11 +305,36 @@ export function createSqliteQueue( return true; } try { - await withOtelSpan( + const spanAttributes = { + "job.type": message.type, + "queue.backend": "sqlite", + "job.attempt": job.attempts + 1, + }; + await withReviewSpan( "selfhost.queue.job", - { "job.type": message.type, "queue.backend": "sqlite", "job.attempt": job.attempts + 1 }, - () => consume(message), - { parentTraceParent: message.type === "github-webhook" ? message.traceParent : undefined }, + spanAttributes, + async () => + await withOtelSpan( + "selfhost.queue.job", + spanAttributes, + () => consume(message), + { + parentTraceParent: + message.type === "github-webhook" + ? message.traceParent + : undefined, + }, + ), + { + op: "queue.process", + parent: + message.type === "github-webhook" + ? { + sentryTrace: message.sentryTrace, + baggage: message.sentryBaggage, + } + : undefined, + }, ); driver.query(`DELETE FROM ${TABLE} WHERE id=?`, [job.id]); recordQueueMetric(driver, "gittensory_jobs_processed_total"); diff --git a/src/selfhost/trace-context.ts b/src/selfhost/trace-context.ts index 70859a6a90..436fda039c 100644 --- a/src/selfhost/trace-context.ts +++ b/src/selfhost/trace-context.ts @@ -1,4 +1,7 @@ +import type { ReviewTraceHeaders } from "../observability/review-trace"; + const requestTraceParents = new WeakMap(); +const requestReviewTraceHeaders = new WeakMap(); export function setSelfHostRequestTraceParent(request: Request, traceParent: string | undefined): void { if (traceParent) requestTraceParents.set(request, traceParent); @@ -9,6 +12,21 @@ export function getSelfHostRequestTraceParent(request: Request): string | undefi return requestTraceParents.get(request); } +export function setSelfHostRequestReviewTraceHeaders( + request: Request, + headers: ReviewTraceHeaders | undefined, +): void { + if (headers?.sentryTrace) requestReviewTraceHeaders.set(request, headers); + else requestReviewTraceHeaders.delete(request); +} + +export function getSelfHostRequestReviewTraceHeaders( + request: Request, +): ReviewTraceHeaders | undefined { + return requestReviewTraceHeaders.get(request); +} + export function clearSelfHostRequestTraceParent(request: Request): void { requestTraceParents.delete(request); + requestReviewTraceHeaders.delete(request); } diff --git a/src/server.ts b/src/server.ts index 2512655291..2c64f56f45 100644 --- a/src/server.ts +++ b/src/server.ts @@ -72,8 +72,13 @@ import { shutdownOpenTelemetry, withOtelSpan, } from "./selfhost/otel"; +import { + currentReviewTraceHeaders, + withReviewSpan, +} from "./observability/review-trace"; import { clearSelfHostRequestTraceParent, + setSelfHostRequestReviewTraceHeaders, setSelfHostRequestTraceParent, } from "./selfhost/trace-context"; import { @@ -686,55 +691,66 @@ async function main(): Promise { ); } } - return await withOtelSpan( + const requestSpanAttributes = selfHostHttpRequestAttributes(request, path); + return await withReviewSpan( "selfhost.http.request", - selfHostHttpRequestAttributes(request, path), - async () => { - const traceParent = currentOtelTraceParent(); - if (traceParent) setSelfHostRequestTraceParent(request, traceParent); - try { - // Instrument real app traffic — status-class counter + latency histogram. (Infra endpoints - // /health /ready /metrics and the setup wizard already returned above and are not counted.) - const startedReq = Date.now(); - const finish = (response: Response): Response => { - incr("gittensory_http_requests_total", { - status: `${Math.floor(response.status / 100)}xx`, - }); - observe( - "gittensory_http_request_duration_seconds", - (Date.now() - startedReq) / 1000, + requestSpanAttributes, + async () => + await withOtelSpan( + "selfhost.http.request", + requestSpanAttributes, + async () => { + const traceParent = currentOtelTraceParent(); + if (traceParent) setSelfHostRequestTraceParent(request, traceParent); + setSelfHostRequestReviewTraceHeaders( + request, + currentReviewTraceHeaders(), ); - setCurrentOtelSpanAttributes(selfHostHttpResponseAttributes(response.status)); - return response; - }; - // Webhook delivery dedup: return 204 immediately for already-processed delivery IDs. - // We mark only AFTER a successful response — failed/rejected webhooks must be retryable. - const isWebhook = - webhookCache && - path === "/v1/github/webhook" && - request.method === "POST"; - const deliveryId = isWebhook - ? request.headers.get("x-github-delivery") - : null; - if (deliveryId) { - const seen = await webhookCache!.get(`delivery:${deliveryId}`); - if (seen) { - incr("gittensory_webhook_dedup_total"); - return finish(new Response(null, { status: 204 })); + try { + // Instrument real app traffic — status-class counter + latency histogram. (Infra endpoints + // /health /ready /metrics and the setup wizard already returned above and are not counted.) + const startedReq = Date.now(); + const finish = (response: Response): Response => { + incr("gittensory_http_requests_total", { + status: `${Math.floor(response.status / 100)}xx`, + }); + observe( + "gittensory_http_request_duration_seconds", + (Date.now() - startedReq) / 1000, + ); + setCurrentOtelSpanAttributes(selfHostHttpResponseAttributes(response.status)); + return response; + }; + // Webhook delivery dedup: return 204 immediately for already-processed delivery IDs. + // We mark only AFTER a successful response — failed/rejected webhooks must be retryable. + const isWebhook = + webhookCache && + path === "/v1/github/webhook" && + request.method === "POST"; + const deliveryId = isWebhook + ? request.headers.get("x-github-delivery") + : null; + if (deliveryId) { + const seen = await webhookCache!.get(`delivery:${deliveryId}`); + if (seen) { + incr("gittensory_webhook_dedup_total"); + return finish(new Response(null, { status: 204 })); + } + } + const response = await worker.fetch(request, env, ctx); + if (deliveryId && response.ok) { + // Best-effort — never block the response on a cache write failure + void webhookCache! + .set(`delivery:${deliveryId}`, "1", 300) + .catch(() => undefined); + } + return finish(response); + } finally { + clearSelfHostRequestTraceParent(request); } - } - const response = await worker.fetch(request, env, ctx); - if (deliveryId && response.ok) { - // Best-effort — never block the response on a cache write failure - void webhookCache! - .set(`delivery:${deliveryId}`, "1", 300) - .catch(() => undefined); - } - return finish(response); - } finally { - clearSelfHostRequestTraceParent(request); - } - }, + }, + ), + { forceTransaction: true, op: "http.server" }, ); }, port, diff --git a/src/types.ts b/src/types.ts index 73e419d53d..bc4f90d300 100644 --- a/src/types.ts +++ b/src/types.ts @@ -12,6 +12,9 @@ export type JobMessage = redriven?: boolean; /** Self-host OTEL trace context for connecting ingress → queued review work. */ traceParent?: string; + /** Self-host Sentry trace context for connecting ingress → queued review work when tracing is sampled on. */ + sentryTrace?: string; + sentryBaggage?: string; } | { // Delayed self-poll to re-capture a PR's before/after preview once its preview deploy is live — the first diff --git a/test/unit/selfhost-sentry.test.ts b/test/unit/selfhost-sentry.test.ts index a027241de5..d8c48a82fd 100644 --- a/test/unit/selfhost-sentry.test.ts +++ b/test/unit/selfhost-sentry.test.ts @@ -3,10 +3,24 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; // Mock @sentry/node so the dynamic import inside initSentry() resolves to spies. Hoisted so vi.mock can see it. const mocks = vi.hoisted(() => { const scope = { setContext: vi.fn(), setLevel: vi.fn(), setTag: vi.fn(), setFingerprint: vi.fn() }; + let activeSpan: { id: string } | null = null; return { scope, init: vi.fn(), withScope: vi.fn((cb: (s: typeof scope) => void) => cb(scope)), + startSpan: vi.fn(async (_options: unknown, cb: (span: { id: string }) => unknown) => { + const prev = activeSpan; + activeSpan = { id: "active-span" }; + try { + return await cb(activeSpan); + } finally { + activeSpan = prev; + } + }), + continueTrace: vi.fn((_options: unknown, cb: () => unknown) => cb()), + getActiveSpan: vi.fn(() => activeSpan), + spanToTraceHeader: vi.fn(() => "sentry-trace-header"), + spanToBaggageHeader: vi.fn(() => "sentry-baggage-header"), captureException: vi.fn(), captureMessage: vi.fn(), captureCheckIn: vi.fn((checkIn: { checkInId?: string }) => checkIn.checkInId ?? "check-in-id"), @@ -19,6 +33,11 @@ const otelMocks = vi.hoisted(() => ({ vi.mock("@sentry/node", () => ({ init: mocks.init, withScope: mocks.withScope, + startSpan: mocks.startSpan, + continueTrace: mocks.continueTrace, + getActiveSpan: mocks.getActiveSpan, + spanToTraceHeader: mocks.spanToTraceHeader, + spanToBaggageHeader: mocks.spanToBaggageHeader, captureException: mocks.captureException, captureMessage: mocks.captureMessage, captureCheckIn: mocks.captureCheckIn, @@ -41,6 +60,10 @@ import { resetSentryForTest, withSentryMonitor, } from "../../src/selfhost/sentry"; +import { + currentReviewTraceHeaders, + withReviewSpan, +} from "../../src/observability/review-trace"; beforeEach(() => { resetSentryForTest(); @@ -91,6 +114,7 @@ describe("disabled when SENTRY_DSN is unset (modular opt-out → complete no-op) expect(await initSentry({} as unknown as NodeJS.ProcessEnv)).toBe(false); captureError(new Error("x"), { a: 1 }); captureReviewFailure(new Error("y"), { repo: "o/r" }); + await expect(withReviewSpan("off", { repo: "o/r" }, async () => "ok")).resolves.toBe("ok"); await expect( withSentryMonitor( "scheduled-loop", @@ -103,6 +127,7 @@ describe("disabled when SENTRY_DSN is unset (modular opt-out → complete no-op) expect(mocks.captureException).not.toHaveBeenCalled(); expect(mocks.captureCheckIn).not.toHaveBeenCalled(); expect(mocks.flush).not.toHaveBeenCalled(); + expect(mocks.startSpan).not.toHaveBeenCalled(); }); }); @@ -154,6 +179,84 @@ describe("enabled when SENTRY_DSN is set", () => { expect(opts.serverName).toBe("https://self.host"); }); + it("keeps review tracing inert when the Sentry trace sample rate is 0", async () => { + await initSentry({ + SENTRY_DSN: "d", + SENTRY_TRACES_SAMPLE_RATE: "0", + } as unknown as NodeJS.ProcessEnv); + + await expect( + withReviewSpan("selfhost.http.request", { repo: "o/r", pr: 7 }, async () => "ok"), + ).resolves.toBe("ok"); + expect(currentReviewTraceHeaders()).toBeUndefined(); + expect(mocks.startSpan).not.toHaveBeenCalled(); + }); + + it("starts review spans only when the Sentry trace sample rate is non-zero", async () => { + await initSentry({ + SENTRY_DSN: "d", + SENTRY_TRACES_SAMPLE_RATE: "0.25", + } as unknown as NodeJS.ProcessEnv); + + let headers: ReturnType; + await expect( + withReviewSpan( + "selfhost.http.request", + { repo: "o/r", pr: 7, secretToken: "drop", longText: "x".repeat(200) }, + async () => { + headers = currentReviewTraceHeaders(); + return "ok"; + }, + { forceTransaction: true, op: "http.server" }, + ), + ).resolves.toBe("ok"); + + const options = mocks.startSpan.mock.calls[0]?.[0] as { + name: string; + forceTransaction?: boolean; + op?: string; + attributes: Record; + }; + expect(options).toMatchObject({ + name: "selfhost.http.request", + forceTransaction: true, + op: "http.server", + attributes: { repo: "o/r", pr: 7 }, + }); + expect(options.attributes.secretToken).toBeUndefined(); + expect(options.attributes.longText).toHaveLength(160); + expect(headers).toEqual({ + sentryTrace: "sentry-trace-header", + baggage: "sentry-baggage-header", + }); + }); + + it("continues a parent Sentry trace when queued work passes sentry headers", async () => { + await initSentry({ + SENTRY_DSN: "d", + SENTRY_TRACES_SAMPLE_RATE: "0.1", + } as unknown as NodeJS.ProcessEnv); + + await expect( + withReviewSpan( + "selfhost.queue.job", + { repo: "o/r", pr: 7 }, + async () => "ok", + { + parent: { + sentryTrace: "queued-trace", + baggage: "queued-baggage", + }, + }, + ), + ).resolves.toBe("ok"); + + expect(mocks.continueTrace).toHaveBeenCalledWith( + { sentryTrace: "queued-trace", baggage: "queued-baggage" }, + expect.any(Function), + ); + }); + it("uses the image-baked version as the release fallback and ignores blank overrides", async () => { expect( resolveSentryRelease({ diff --git a/test/unit/webhook.test.ts b/test/unit/webhook.test.ts index 42827d74b2..b2d5be66d9 100644 --- a/test/unit/webhook.test.ts +++ b/test/unit/webhook.test.ts @@ -5,6 +5,7 @@ import { getWebhookEvent, recordWebhookEvent } from "../../src/db/repositories"; import { relaySignature } from "../../src/orb/relay"; import { clearSelfHostRequestTraceParent, + setSelfHostRequestReviewTraceHeaders, setSelfHostRequestTraceParent, } from "../../src/selfhost/trace-context"; import { createTestEnv } from "../helpers/d1"; @@ -220,7 +221,7 @@ describe("github webhook queue isolation (#audit-webhook-queue)", () => { expect(jobsSends).toBe(0); // never the shared maintenance queue }); - it("copies the internal self-host traceparent onto queued webhook jobs", async () => { + it("copies the internal self-host trace headers onto queued webhook jobs", async () => { const env = createTestEnv(); const sent: import("../../src/types").JobMessage[] = []; env.WEBHOOKS = { send: async (message: unknown) => void sent.push(message as import("../../src/types").JobMessage) } as unknown as Queue; @@ -228,7 +229,12 @@ describe("github webhook queue isolation (#audit-webhook-queue)", () => { const signature = await signWebhook(rawBody, env.GITHUB_WEBHOOK_SECRET); const request = new Request("https://example.com/webhook", { method: "POST", body: rawBody }); const traceParent = "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-01"; + const sentryTrace = "0123456789abcdef0123456789abcdef-0123456789abcdef-1"; setSelfHostRequestTraceParent(request, traceParent); + setSelfHostRequestReviewTraceHeaders(request, { + sentryTrace, + baggage: "sentry-environment=selfhost", + }); const headers: Record = { "x-github-delivery": "traceparent-1", "x-github-event": "pull_request", @@ -252,7 +258,12 @@ describe("github webhook queue isolation (#audit-webhook-queue)", () => { expect(response.status).toBe(202); expect(sent).toHaveLength(1); - expect(sent[0]).toMatchObject({ type: "github-webhook", traceParent }); + expect(sent[0]).toMatchObject({ + type: "github-webhook", + traceParent, + sentryTrace, + sentryBaggage: "sentry-environment=selfhost", + }); }); it("drops self-authored app comment webhooks before they add queue pressure", async () => {