Skip to content

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

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.

Sign up for GitHub

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

Closed
jonathanchang31 wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
597f0d9
feat(observability): add sampled Sentry review spans
jonathanchang31 Jun 30, 2026
d76e8d9
Merge branch 'main' into feat-1734-sentry-review-traces
jonathanchang31 Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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@<version>, so do
Expand All @@ -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
Expand Down
133 changes: 70 additions & 63 deletions src/github/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<CheckRunOutcome> => {
Expand Down Expand Up @@ -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 {
Expand Down
67 changes: 51 additions & 16 deletions src/github/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -274,22 +288,43 @@ async function replayableResponse(response: Response): Promise<CachedGitHubRespo
}

async function fetchWithGitHubRetry(input: RequestInfo | URL, init?: GitHubTimeoutFetchInit): Promise<Response> {
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(
Expand Down
95 changes: 49 additions & 46 deletions src/github/comments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 {
Expand Down
Loading
Loading