From 931698a66e120749bfe1837e8b9d50362607cebe Mon Sep 17 00:00:00 2001 From: Vuk <129513318+vuks19@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:48:23 +0200 Subject: [PATCH 1/2] gh close issue when resolved / archived --- CLAUDE.md | 5 +- .../exception_stack_trace.controller.go | 69 ++-- backend/app/controllers/page.controller.go | 22 +- .../pg/0143_create_github_issues.up.sql | 11 + .../0144_create_github_issues_open_idx.up.sql | 1 + .../sqlite/0077_create_github_issues.up.sql | 13 + backend/app/models/escalation.model.go | 9 +- backend/app/models/github_issue.model.go | 27 ++ backend/app/models/models.go | 1 + .../app/models/notification_message.model.go | 26 ++ backend/app/models/outbox.model.go | 3 + backend/app/notifications/adapter_github.go | 120 ++++++- .../app/notifications/adapter_github_test.go | 154 +++++++++ backend/app/notifications/dispatch.go | 4 + backend/app/notifications/github_issues.go | 155 +++++++++ .../github_issues_sqlite_test.go | 295 ++++++++++++++++++ backend/app/notifications/outbox_hooks.go | 6 + .../pg/github_issue.repository.go | 45 +++ .../sqlite/github_issue.repository.go | 45 +++ .../transactional/transactional_pg.go | 1 + .../transactional/transactional_sqlite.go | 1 + docs/pages/learn/alerts.mdx | 14 +- 22 files changed, 987 insertions(+), 40 deletions(-) create mode 100644 backend/app/migrations/pg/0143_create_github_issues.up.sql create mode 100644 backend/app/migrations/pg/0144_create_github_issues_open_idx.up.sql create mode 100644 backend/app/migrations/sqlite/0077_create_github_issues.up.sql create mode 100644 backend/app/models/github_issue.model.go create mode 100644 backend/app/notifications/adapter_github_test.go create mode 100644 backend/app/notifications/github_issues.go create mode 100644 backend/app/notifications/github_issues_sqlite_test.go create mode 100644 backend/app/repositories/transactional/pg/github_issue.repository.go create mode 100644 backend/app/repositories/transactional/sqlite/github_issue.repository.go diff --git a/CLAUDE.md b/CLAUDE.md index 500ff152b..f69cb8dab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -292,6 +292,8 @@ NOTIFICATION_POLL_SECONDS=60 # polled rule evaluation interval; minimum ONCALL_POLL_SECONDS=30 # on-call escalation worker interval; minimum 5, invalid values fall back to 30. Kept separate from NOTIFICATION_POLL_SECONDS so raising rule-evaluation intervals never delays paging. A buffered Wake() channel makes freshly opened pages notify L1 near-instantly regardless of this interval. OUTBOX_POLL_SECONDS=15 # notification outbox drain interval; minimum 5, invalid values fall back to 15. The outbox (backend/app/outbox, notification_outbox table in the main DB) is the persist-then-send layer for ALL notifications: rule dispatch and the escalator only enqueue (with an adapter-config snapshot) inside their transactions; the drain worker sends with retries (backoff 1m/5m/15m/60m, 5 attempts, then terminal failed + CaptureException). Crash-safe at-least-once: stale 'sending' rows are reclaimed after 5 min, cancelled rows can never resurrect (guarded status transitions), ack/resolve cancels queued page deliveries via outbox.CancelByKey. Cooldown and event-rule dedup record at enqueue commit (the durable promise), and fired_notifications is written at the terminal outcome. /api/health/deep exposes an `outbox` block; `traceway.outbox.*` metrics are emitted when monitoring is on; terminal rows are pruned daily (sent/cancelled 7d, failed 30d). +**GitHub channel round trip.** The `github` channel is the one adapter with a lifecycle: it opens an issue when a rule fires and closes it again when the exception is archived. A `new_error`/`error_regression` delivery is tagged in `dispatch` (`trackGitHubIssue`, gated on `models.IsIssueRuleType` — an issue opened by a latency or metric rule has no exception to archive, so it is never tracked), and `AdapterSend` routes github deliveries to `sendGitHubIssue`, which records the created issue number in `github_issues` (main DB, keyed project + `issue_key` = exception hash + channel, `closed_at` doubles as the state). Every archive path — `/exception-stack-traces/archive` plus the two on-call page resolves that archive — then calls `notifications.CloseGitHubIssuesForArchived` in its transaction, which marks the rows closed and enqueues an `OutboxKindGithubClose` delivery per open issue. The rows are marked closed at enqueue (the durable promise, as with cooldowns) so a second archive cannot queue a second close; the close PATCHes `state=closed` using the **recorded** owner/repo rather than the channel's current config, treats 404/410 as already closed, and posts its explanatory comment after the close so a retry never doubles it. A channel retyped away from github drops the row instead of queueing a send that can only fail; a disabled one still closes, since this is one-shot cleanup of an issue that channel itself opened. Deleting the channel (or the project) cascades the rows away, which is also the only way a tracked issue is abandoned. A regression opens a fresh issue under the same hash, so an `issue_key` carries several rows over time. + # Synthetics (synthetic uptime monitoring: backend/app/synthetics, /synthetics frontend route) SYNTHETICS_POLL_SECONDS=15 # scheduler tick for due checks; minimum 5, invalid values fall back to 15. The scheduler enqueues due checks into the check_runs queue (main DB, outbox-style guarded claims, advisory lock 824737004) and records expired queued runs as `missed` in telemetry — a probe is never executed late. In-process executors claim http/tcp runs always, browser runs only when mode=embedded. SYNTHETICS_BROWSER_MODE=off # off | embedded | remote. Browser checks are real @playwright/test specs executed by spawning Node against a harness dir (no npm/npx at runtime; allowlisted env so user scripts never see server secrets). `embedded` requires the :browser image (Dockerfile.browser, DuckDB base + Node + Chromium) and fails fast at startup otherwise; `remote` queues browser runs for traceway-runner binaries that long-poll /api/runners/poll authenticating with SYNTHETICS_RUNNER_SECRET. Hard-blocked in cloud mode (startup panic + 422 at check creation). @@ -911,6 +913,7 @@ func (c *ReportController) Report(ctx *gin.Context) { | `invitations` | Team invitations with token, role, expiry | | `source_maps` | Uploaded source map files (project, version, storage key) | | `metric_registry` | Custom metric definitions (type, unit, description) | +| `github_issues` | Issues the GitHub channel opened, so archiving the exception can close them (`closed_at` = state) | | `dashboards` | Org-owned dashboards (name, JSONB definition with widgets, template provenance) | | `project_dashboards` | Which projects show a dashboard, and tab order | | `dashboard_templates` | Marketplace templates (key, category, definition), seeded by migrations | @@ -944,7 +947,7 @@ Both embedded builds (SQLite and DuckDB telemetry) are single-instance: the data **Main DB tables** (`db.DB` — transactional, uses lit with `*sql.Tx`): - `users`, `organizations`, `organization_users`, `projects`, `invitations` - `source_maps`, `metric_registry`, `dashboards`, `project_dashboards`, `dashboard_templates`, `starred_dashboard_widgets` (plus the legacy `widget_groups`/`widget_group_widgets`/`starred_widgets`) -- `notification_channels`, `notification_rules` +- `notification_channels`, `notification_rules`, `github_issues` - `synthetic_checks`, `check_runs`, `check_incidents`, `incident_updates`, `post_mortems`, `post_mortem_events`, `synthetic_runners`, `status_pages` **Telemetry DB tables** (`db.TelemetryDB` — non-transactional, uses lit with `db.TelemetryDB` directly): diff --git a/backend/app/controllers/exception_stack_trace.controller.go b/backend/app/controllers/exception_stack_trace.controller.go index 5f08369bd..b2d76e6dc 100644 --- a/backend/app/controllers/exception_stack_trace.controller.go +++ b/backend/app/controllers/exception_stack_trace.controller.go @@ -14,7 +14,9 @@ import ( "github.com/tracewayapp/traceway/backend/app/db" "github.com/tracewayapp/traceway/backend/app/middleware" "github.com/tracewayapp/traceway/backend/app/models" + "github.com/tracewayapp/traceway/backend/app/notifications" "github.com/tracewayapp/traceway/backend/app/oncall" + "github.com/tracewayapp/traceway/backend/app/outbox" "github.com/tracewayapp/traceway/backend/app/repositories/telemetry" "github.com/tracewayapp/traceway/backend/app/repositories/transactional" "github.com/tracewayapp/traceway/backend/app/storage" @@ -45,6 +47,13 @@ type ArchiveRequest struct { ResolvePages bool `json:"resolvePages"` } +// archiveOutcome is the main-DB follow-up to an archive: the on-call pages it +// resolved and the GitHub issues it queued a close for. +type archiveOutcome struct { + ResolvedPages int + ClosedGithubIssues int +} + type ExceptionDetailRequest struct { Pagination PaginationParams `json:"pagination"` } @@ -243,36 +252,50 @@ func (e exceptionStackTraceController) ArchiveExceptions(c *gin.Context) { return } - resolvedPages := 0 - if request.ResolvePages { - userId := middleware.GetUserId(c) - now := time.Now().UTC() - resolvedPages, err = db.ExecuteTransaction(func(tx *sql.Tx) (int, error) { - resolved := 0 - for _, hash := range request.Hashes { - pages, err := transactional.PageRepository.FindUnresolvedByIssueHash(tx, projectId, hash) + // The archive itself is telemetry and already landed; what follows is main-DB + // bookkeeping for the same issues, so it runs in one transaction after it. + userId := middleware.GetUserId(c) + now := time.Now().UTC() + outcome, err := db.ExecuteTransaction(func(tx *sql.Tx) (archiveOutcome, error) { + var result archiveOutcome + closed, err := notifications.CloseGitHubIssuesForArchived(tx, projectId, request.Hashes) + if err != nil { + return result, err + } + result.ClosedGithubIssues = closed + if !request.ResolvePages { + return result, nil + } + for _, hash := range request.Hashes { + pages, err := transactional.PageRepository.FindUnresolvedByIssueHash(tx, projectId, hash) + if err != nil { + return result, err + } + for _, page := range pages { + ok, err := oncall.ResolvePage(tx, page.Id, userId, now) if err != nil { - return 0, err + return result, err } - for _, page := range pages { - ok, err := oncall.ResolvePage(tx, page.Id, userId, now) - if err != nil { - return 0, err - } - if ok { - resolved++ - } + if ok { + result.ResolvedPages++ } } - return resolved, nil - }) - if err != nil { - c.AbortWithError(500, traceway.NewStackTraceErrorf("error resolving pages for archived issues %s: %w", strings.Join(request.Hashes, ","), err)) - return } + return result, nil + }) + if err != nil { + c.AbortWithError(500, traceway.NewStackTraceErrorf("error closing out archived issues %s: %w", strings.Join(request.Hashes, ","), err)) + return + } + if outcome.ClosedGithubIssues > 0 { + outbox.Wake() } - c.JSON(http.StatusOK, gin.H{"archived": len(request.Hashes), "resolvedPages": resolvedPages}) + c.JSON(http.StatusOK, gin.H{ + "archived": len(request.Hashes), + "resolvedPages": outcome.ResolvedPages, + "closedGithubIssues": outcome.ClosedGithubIssues, + }) } func (e exceptionStackTraceController) UnarchiveExceptions(c *gin.Context) { diff --git a/backend/app/controllers/page.controller.go b/backend/app/controllers/page.controller.go index 952623be6..2d73cb181 100644 --- a/backend/app/controllers/page.controller.go +++ b/backend/app/controllers/page.controller.go @@ -10,7 +10,9 @@ import ( "github.com/tracewayapp/traceway/backend/app/db" "github.com/tracewayapp/traceway/backend/app/middleware" "github.com/tracewayapp/traceway/backend/app/models" + "github.com/tracewayapp/traceway/backend/app/notifications" "github.com/tracewayapp/traceway/backend/app/oncall" + "github.com/tracewayapp/traceway/backend/app/outbox" "github.com/tracewayapp/traceway/backend/app/repositories/telemetry" "github.com/tracewayapp/traceway/backend/app/repositories/transactional" @@ -210,6 +212,7 @@ func (c *pageController) Resolve(ctx *gin.Context) { } archived := false + closedIssues := 0 if request.ArchiveIssue { hash := page.IssueHash() if hash == "" { @@ -233,8 +236,15 @@ func (c *pageController) Resolve(ctx *gin.Context) { return } archived = true + if closedIssues, err = notifications.CloseGitHubIssuesForArchived(tx, page.ProjectId, []string{hash}); err != nil { + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("failed to close GitHub issues for archived issue %s: %w", hash, err)) + return + } + if closedIssues > 0 { + middleware.OnCommit(ctx, outbox.Wake) + } } - ctx.JSON(http.StatusOK, gin.H{"message": "Page resolved", "archivedIssue": archived}) + ctx.JSON(http.StatusOK, gin.H{"message": "Page resolved", "archivedIssue": archived, "closedGithubIssues": closedIssues}) } type bulkAcknowledgeRequest struct { @@ -343,6 +353,7 @@ func (c *pageController) BulkResolve(ctx *gin.Context) { } archivedIssues := 0 + closedIssues := 0 if len(hashSet) > 0 { // Resolving deliberately has no write gate, but archiving issues is a // write: enforce the effective project role in-handler. Aborting rolls @@ -365,8 +376,15 @@ func (c *pageController) BulkResolve(ctx *gin.Context) { return } archivedIssues = len(hashes) + if closedIssues, err = notifications.CloseGitHubIssuesForArchived(tx, projectId, hashes); err != nil { + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("failed to close GitHub issues for archived issues: %w", err)) + return + } + if closedIssues > 0 { + middleware.OnCommit(ctx, outbox.Wake) + } } - ctx.JSON(http.StatusOK, gin.H{"resolved": resolved, "archivedIssues": archivedIssues}) + ctx.JSON(http.StatusOK, gin.H{"resolved": resolved, "archivedIssues": archivedIssues, "closedGithubIssues": closedIssues}) } type pagesForIssuesRequest struct { diff --git a/backend/app/migrations/pg/0143_create_github_issues.up.sql b/backend/app/migrations/pg/0143_create_github_issues.up.sql new file mode 100644 index 000000000..eb49ee37a --- /dev/null +++ b/backend/app/migrations/pg/0143_create_github_issues.up.sql @@ -0,0 +1,11 @@ +CREATE TABLE IF NOT EXISTS github_issues ( + id SERIAL PRIMARY KEY, + project_id UUID NOT NULL, + channel_id INT NOT NULL REFERENCES notification_channels(id) ON DELETE CASCADE, + issue_key VARCHAR(200) NOT NULL DEFAULT '', + owner VARCHAR(200) NOT NULL DEFAULT '', + repo VARCHAR(200) NOT NULL DEFAULT '', + issue_number INT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + closed_at TIMESTAMPTZ +) diff --git a/backend/app/migrations/pg/0144_create_github_issues_open_idx.up.sql b/backend/app/migrations/pg/0144_create_github_issues_open_idx.up.sql new file mode 100644 index 000000000..2a0b88ad0 --- /dev/null +++ b/backend/app/migrations/pg/0144_create_github_issues_open_idx.up.sql @@ -0,0 +1 @@ +CREATE INDEX IF NOT EXISTS github_issues_open_idx ON github_issues (project_id, issue_key) WHERE closed_at IS NULL diff --git a/backend/app/migrations/sqlite/0077_create_github_issues.up.sql b/backend/app/migrations/sqlite/0077_create_github_issues.up.sql new file mode 100644 index 000000000..cefe756b5 --- /dev/null +++ b/backend/app/migrations/sqlite/0077_create_github_issues.up.sql @@ -0,0 +1,13 @@ +CREATE TABLE IF NOT EXISTS github_issues ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + project_id TEXT NOT NULL, + channel_id INTEGER NOT NULL REFERENCES notification_channels(id) ON DELETE CASCADE, + issue_key TEXT NOT NULL DEFAULT '', + owner TEXT NOT NULL DEFAULT '', + repo TEXT NOT NULL DEFAULT '', + issue_number INTEGER NOT NULL, + created_at DATETIME NOT NULL, + closed_at DATETIME +); + +CREATE INDEX IF NOT EXISTS github_issues_open_idx ON github_issues (project_id, issue_key) WHERE closed_at IS NULL; diff --git a/backend/app/models/escalation.model.go b/backend/app/models/escalation.model.go index e9a5eceb3..e09d28da5 100644 --- a/backend/app/models/escalation.model.go +++ b/backend/app/models/escalation.model.go @@ -54,11 +54,18 @@ type Page struct { UpdatedAt time.Time `json:"updatedAt" lit:"updated_at"` } +// IsIssueRuleType reports whether a rule fires for one exception group, and so +// carries that group's hash as its dedup token. Pages and GitHub issues both +// use it to decide whether they are issue-linked. +func IsIssueRuleType(ruleType string) bool { + return ruleType == "new_error" || ruleType == "error_regression" +} + // IssueHash returns the exception hash a page was opened for, or "" when the // page is not issue-linked. New-error and regression rules carry the hash as // the dedup token after the "ruleId|" prefix (see oncall.pageDedupKey). func (p *Page) IssueHash() string { - if p.RuleType != "new_error" && p.RuleType != "error_regression" { + if !IsIssueRuleType(p.RuleType) { return "" } _, hash, found := strings.Cut(p.DedupKey, "|") diff --git a/backend/app/models/github_issue.model.go b/backend/app/models/github_issue.model.go new file mode 100644 index 000000000..0bf069e09 --- /dev/null +++ b/backend/app/models/github_issue.model.go @@ -0,0 +1,27 @@ +package models + +import ( + "time" + + "github.com/google/uuid" +) + +// GithubIssue records an issue the GitHub channel opened, so archiving the +// exception it tracks can close it again. ClosedAt doubles as the state: an +// issue still open in GitHub has none. +// +// Rows are keyed per channel rather than per rule: several rules can point at +// the same GitHub channel, and the issue belongs to the repository it landed +// in, which is the channel's configuration. A regression opens a second issue +// under the same IssueKey, so a key can carry more than one row over time. +type GithubIssue struct { + Id int `json:"id" lit:"id"` + ProjectId uuid.UUID `json:"projectId" lit:"project_id"` + ChannelId int `json:"channelId" lit:"channel_id"` + IssueKey string `json:"issueKey" lit:"issue_key"` + Owner string `json:"owner" lit:"owner"` + Repo string `json:"repo" lit:"repo"` + IssueNumber int `json:"issueNumber" lit:"issue_number"` + CreatedAt time.Time `json:"createdAt" lit:"created_at"` + ClosedAt *time.Time `json:"closedAt" lit:"closed_at"` +} diff --git a/backend/app/models/models.go b/backend/app/models/models.go index 2182b026e..4c81d0343 100644 --- a/backend/app/models/models.go +++ b/backend/app/models/models.go @@ -54,6 +54,7 @@ func Init(driver lit.Driver) { lit.RegisterModel[NotificationChannel](driver) lit.RegisterModel[NotificationRule](driver) lit.RegisterModel[NotificationRuleWithChannel](driver) + lit.RegisterModel[GithubIssue](driver) lit.RegisterModel[Team](driver) lit.RegisterModel[TeamWithCounts](driver) lit.RegisterModel[TeamProjectRow](driver) diff --git a/backend/app/models/notification_message.model.go b/backend/app/models/notification_message.model.go index 77c59c4cc..0e3837875 100644 --- a/backend/app/models/notification_message.model.go +++ b/backend/app/models/notification_message.model.go @@ -23,6 +23,12 @@ type NotificationMessage struct { Email *NotificationEmail `json:",omitempty"` + // GitHub is the GitHub adapter's payload: on a rule delivery it names what + // the created issue tracks, so archiving that exception can close the issue + // again; on a close delivery it names the issue to close. Only the GitHub + // adapter reads it. + GitHub *NotificationGitHub `json:",omitempty"` + // DedupToken is the stable identity of what fired within the rule // (exception hash, endpoint, task or metric name; empty for rule-level // conditions). Page dedup keys are built from it — never from URL, which @@ -42,6 +48,26 @@ const ( EmailTemplateTest = "test" ) +// NotificationGitHub is carried by messages a GitHub channel can act on. Like +// the rest of NotificationMessage it is persisted in notification_outbox, so +// the shape is a wire format: add fields, never repurpose them. +type NotificationGitHub struct { + // IssueKey is the exception hash a created issue tracks, and ProjectId the + // project it belongs to. Empty when the firing rule is not issue-shaped: + // nothing would ever archive that issue, so nothing is recorded for it. + IssueKey string `json:",omitempty"` + ProjectId string `json:",omitempty"` + ChannelId int `json:",omitempty"` + + // CloseNumber makes the delivery close that issue in Owner/Repo instead of + // creating a new one. Owner and Repo come from the recorded issue rather + // than the channel's current config, so a repository changed since the + // issue was opened still closes the right one. + CloseNumber int `json:",omitempty"` + Owner string `json:",omitempty"` + Repo string `json:",omitempty"` +} + type NotificationEmail struct { Template string Exception *EmailException `json:",omitempty"` diff --git a/backend/app/models/outbox.model.go b/backend/app/models/outbox.model.go index 6b8ecbfb7..8ce898c65 100644 --- a/backend/app/models/outbox.model.go +++ b/backend/app/models/outbox.model.go @@ -10,6 +10,9 @@ const ( OutboxKindRule = "rule" OutboxKindPage = "page" OutboxKindVerification = "verification" + // OutboxKindGithubClose closes a GitHub issue whose exception was archived. + // It is not a rule fire, so it records no fired_notifications audit row. + OutboxKindGithubClose = "github_close" OutboxPending = "pending" OutboxSending = "sending" diff --git a/backend/app/notifications/adapter_github.go b/backend/app/notifications/adapter_github.go index db5fdc037..ef3e47c98 100644 --- a/backend/app/notifications/adapter_github.go +++ b/backend/app/notifications/adapter_github.go @@ -7,7 +7,17 @@ import ( "fmt" "io" "net/http" + "net/url" "time" + + traceway "go.tracewayapp.com" +) + +const ( + githubAPIBase = "https://api.github.com" + // githubMaxResponseBytes caps what is read back from an API response; only + // the created issue's number is ever needed from it. + githubMaxResponseBytes = 1 << 20 ) type GitHubAdapter struct { @@ -15,6 +25,10 @@ type GitHubAdapter struct { Owner string `json:"owner"` Repo string `json:"repo"` Labels []string `json:"labels,omitempty"` + + // baseURL replaces api.github.com in tests. Unexported, so a channel + // config can never redirect deliveries somewhere else. + baseURL string } func (a *GitHubAdapter) Type() string { return "github" } @@ -32,9 +46,17 @@ func (a *GitHubAdapter) Validate() error { return nil } +// Send opens an issue for the message. Deliveries that close a previously +// opened issue go through AdapterSend, which routes them to CloseIssue. func (a *GitHubAdapter) Send(ctx context.Context, msg Message) error { - url := fmt.Sprintf("https://api.github.com/repos/%s/%s/issues", a.Owner, a.Repo) + _, err := a.CreateIssue(ctx, msg) + return err +} +// CreateIssue opens the issue and returns its number, so the caller can record +// it against the exception it tracks. A number of 0 means the issue was +// created but its number could not be read back. +func (a *GitHubAdapter) CreateIssue(ctx context.Context, msg Message) (int, error) { payload := map[string]interface{}{ "title": msg.Subject, "body": msg.Body, @@ -43,14 +65,90 @@ func (a *GitHubAdapter) Send(ctx context.Context, msg Message) error { payload["labels"] = a.Labels } + resp, err := a.do(ctx, http.MethodPost, a.issuesURL(a.Owner, a.Repo), payload) + if err != nil { + return 0, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + io.Copy(io.Discard, io.LimitReader(resp.Body, githubMaxResponseBytes)) + return 0, fmt.Errorf("GitHub returned status %d", resp.StatusCode) + } + + var created struct { + Number int `json:"number"` + } + if err := json.NewDecoder(io.LimitReader(resp.Body, githubMaxResponseBytes)).Decode(&created); err != nil { + // The issue exists. Failing the delivery over a body we cannot read + // would retry the create and open a duplicate, so report success and + // let the caller skip recording it. + return 0, nil + } + return created.Number, nil +} + +// CloseIssue closes an issue this channel opened and comments why. An issue +// that is gone (deleted, or in a repository the token no longer reaches) +// counts as closed: retrying cannot bring it back. +func (a *GitHubAdapter) CloseIssue(ctx context.Context, owner, repo string, number int, comment string) error { + target := fmt.Sprintf("%s/%d", a.issuesURL(owner, repo), number) + resp, err := a.do(ctx, http.MethodPatch, target, map[string]interface{}{ + "state": "closed", + "state_reason": "completed", + }) + if err != nil { + return err + } + drainResponse(resp) + + if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusGone { + return nil + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("GitHub returned status %d", resp.StatusCode) + } + + // The close is what the delivery promised; the comment is commentary. It is + // posted after the close so a retry never doubles it up, and its failure is + // reported rather than returned so the retry does not either. + if comment != "" { + if err := a.comment(ctx, target, comment); err != nil { + traceway.CaptureException(fmt.Errorf("closed GitHub issue %s/%s#%d but failed to comment: %w", owner, repo, number, err)) + } + } + return nil +} + +func (a *GitHubAdapter) comment(ctx context.Context, issueURL, body string) error { + resp, err := a.do(ctx, http.MethodPost, issueURL+"/comments", map[string]interface{}{"body": body}) + if err != nil { + return err + } + drainResponse(resp) + if resp.StatusCode != http.StatusCreated { + return fmt.Errorf("GitHub returned status %d", resp.StatusCode) + } + return nil +} + +func (a *GitHubAdapter) issuesURL(owner, repo string) string { + base := a.baseURL + if base == "" { + base = githubAPIBase + } + return fmt.Sprintf("%s/repos/%s/%s/issues", base, url.PathEscape(owner), url.PathEscape(repo)) +} + +func (a *GitHubAdapter) do(ctx context.Context, method, target string, payload interface{}) (*http.Response, error) { body, err := json.Marshal(payload) if err != nil { - return fmt.Errorf("failed to marshal GitHub payload: %w", err) + return nil, fmt.Errorf("failed to marshal GitHub payload: %w", err) } - req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(body)) + req, err := http.NewRequestWithContext(ctx, method, target, bytes.NewReader(body)) if err != nil { - return fmt.Errorf("failed to create GitHub request: %w", err) + return nil, fmt.Errorf("failed to create GitHub request: %w", err) } req.Header.Set("Content-Type", "application/json") @@ -60,14 +158,12 @@ func (a *GitHubAdapter) Send(ctx context.Context, msg Message) error { client := &http.Client{Timeout: 10 * time.Second} resp, err := client.Do(req) if err != nil { - return fmt.Errorf("GitHub request failed: %w", err) - } - defer resp.Body.Close() - io.Copy(io.Discard, resp.Body) - - if resp.StatusCode != 201 { - return fmt.Errorf("GitHub returned status %d", resp.StatusCode) + return nil, fmt.Errorf("GitHub request failed: %w", err) } + return resp, nil +} - return nil +func drainResponse(resp *http.Response) { + io.Copy(io.Discard, io.LimitReader(resp.Body, githubMaxResponseBytes)) + resp.Body.Close() } diff --git a/backend/app/notifications/adapter_github_test.go b/backend/app/notifications/adapter_github_test.go new file mode 100644 index 000000000..4aa6a37e9 --- /dev/null +++ b/backend/app/notifications/adapter_github_test.go @@ -0,0 +1,154 @@ +package notifications + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" +) + +type githubCall struct { + Method string + Path string + Body map[string]interface{} +} + +func githubServer(t *testing.T, handler func(w http.ResponseWriter, r *http.Request)) (*httptest.Server, *[]githubCall) { + t.Helper() + calls := &[]githubCall{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + var body map[string]interface{} + json.Unmarshal(raw, &body) + *calls = append(*calls, githubCall{Method: r.Method, Path: r.URL.Path, Body: body}) + handler(w, r) + })) + t.Cleanup(server.Close) + return server, calls +} + +func TestCreateIssueReturnsIssueNumber(t *testing.T) { + server, calls := githubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"number":17,"html_url":"https://github.com/acme/backend/issues/17"}`)) + }) + + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "backend", Labels: []string{"bug"}, baseURL: server.URL} + number, err := adapter.CreateIssue(context.Background(), Message{Subject: "New error", Body: "trace"}) + if err != nil { + t.Fatalf("create: %v", err) + } + if number != 17 { + t.Errorf("number = %d, expected 17", number) + } + if len(*calls) != 1 { + t.Fatalf("expected 1 call, got %d", len(*calls)) + } + call := (*calls)[0] + if call.Method != http.MethodPost || call.Path != "/repos/acme/backend/issues" { + t.Errorf("wrong request: %s %s", call.Method, call.Path) + } + if call.Body["title"] != "New error" || call.Body["body"] != "trace" { + t.Errorf("wrong payload: %v", call.Body) + } +} + +func TestCreateIssueSucceedsWithUnreadableBody(t *testing.T) { + server, _ := githubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`not json`)) + }) + + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "backend", baseURL: server.URL} + number, err := adapter.CreateIssue(context.Background(), Message{Subject: "New error"}) + // The issue exists: a retry would open a duplicate, so this must not fail. + if err != nil { + t.Fatalf("create: %v", err) + } + if number != 0 { + t.Errorf("number = %d, expected 0 so the caller skips recording", number) + } +} + +func TestCreateIssueFailsOnErrorStatus(t *testing.T) { + server, _ := githubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }) + + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "backend", baseURL: server.URL} + if _, err := adapter.CreateIssue(context.Background(), Message{Subject: "New error"}); err == nil { + t.Fatal("expected an error so the outbox retries") + } +} + +func TestCloseIssueClosesThenComments(t *testing.T) { + server, calls := githubServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + w.WriteHeader(http.StatusCreated) + return + } + w.WriteHeader(http.StatusOK) + }) + + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "backend", baseURL: server.URL} + if err := adapter.CloseIssue(context.Background(), "acme", "backend", 17, "archived in Traceway"); err != nil { + t.Fatalf("close: %v", err) + } + if len(*calls) != 2 { + t.Fatalf("expected a close and a comment, got %d calls", len(*calls)) + } + closeCall, commentCall := (*calls)[0], (*calls)[1] + if closeCall.Method != http.MethodPatch || closeCall.Path != "/repos/acme/backend/issues/17" { + t.Errorf("wrong close request: %s %s", closeCall.Method, closeCall.Path) + } + if closeCall.Body["state"] != "closed" || closeCall.Body["state_reason"] != "completed" { + t.Errorf("wrong close payload: %v", closeCall.Body) + } + if commentCall.Method != http.MethodPost || commentCall.Path != "/repos/acme/backend/issues/17/comments" { + t.Errorf("wrong comment request: %s %s", commentCall.Method, commentCall.Path) + } + if commentCall.Body["body"] != "archived in Traceway" { + t.Errorf("wrong comment payload: %v", commentCall.Body) + } +} + +func TestCloseIssueUsesTheRecordedRepository(t *testing.T) { + server, calls := githubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + // The channel has been repointed since the issue was opened; the close must + // still reach the repository the issue actually lives in. + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "new-repo", baseURL: server.URL} + if err := adapter.CloseIssue(context.Background(), "acme", "old-repo", 17, ""); err != nil { + t.Fatalf("close: %v", err) + } + if path := (*calls)[0].Path; path != "/repos/acme/old-repo/issues/17" { + t.Errorf("closed the wrong issue: %s", path) + } +} + +func TestCloseIssueTreatsMissingIssueAsClosed(t *testing.T) { + server, _ := githubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "backend", baseURL: server.URL} + // Retrying cannot bring a deleted issue back, so this must not keep failing. + if err := adapter.CloseIssue(context.Background(), "acme", "backend", 17, "archived"); err != nil { + t.Fatalf("close: %v", err) + } +} + +func TestCloseIssueFailsOnErrorStatus(t *testing.T) { + server, _ := githubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "backend", baseURL: server.URL} + if err := adapter.CloseIssue(context.Background(), "acme", "backend", 17, "archived"); err == nil { + t.Fatal("expected an error so the outbox retries") + } +} diff --git a/backend/app/notifications/dispatch.go b/backend/app/notifications/dispatch.go index 09d434362..0661939ca 100644 --- a/backend/app/notifications/dispatch.go +++ b/backend/app/notifications/dispatch.go @@ -69,6 +69,10 @@ func dispatch(rule *models.NotificationRuleWithChannel, msg Message) bool { return true } + if channel.ChannelType == "github" { + trackGitHubIssue(&msg, rule, channel.Id) + } + ruleId := rule.Id projectId := rule.ProjectId _, err := db.ExecuteTransaction(func(tx *sql.Tx) (int, error) { diff --git a/backend/app/notifications/github_issues.go b/backend/app/notifications/github_issues.go new file mode 100644 index 000000000..c8cb27445 --- /dev/null +++ b/backend/app/notifications/github_issues.go @@ -0,0 +1,155 @@ +package notifications + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" + "strings" + "time" + + "github.com/google/uuid" + "github.com/tracewayapp/traceway/backend/app/config" + "github.com/tracewayapp/traceway/backend/app/db" + "github.com/tracewayapp/traceway/backend/app/models" + "github.com/tracewayapp/traceway/backend/app/outbox" + "github.com/tracewayapp/traceway/backend/app/repositories/transactional" + traceway "go.tracewayapp.com" +) + +// sendGitHubIssue performs one GitHub delivery: a close when the message names +// an issue to close, otherwise a create whose issue number is remembered so +// archiving the exception can close it later. +func sendGitHubIssue(ctx context.Context, adapter *GitHubAdapter, msg Message) error { + if msg.GitHub != nil && msg.GitHub.CloseNumber > 0 { + return adapter.CloseIssue(ctx, msg.GitHub.Owner, msg.GitHub.Repo, msg.GitHub.CloseNumber, msg.Body) + } + number, err := adapter.CreateIssue(ctx, msg) + if err != nil { + return err + } + recordGitHubIssue(adapter, msg, number) + return nil +} + +// recordGitHubIssue remembers a created issue so archiving the exception it +// tracks closes it. The issue already exists, so a failure here is reported and +// swallowed: failing the delivery would retry the create and open a duplicate. +func recordGitHubIssue(adapter *GitHubAdapter, msg Message, number int) { + if msg.GitHub == nil || msg.GitHub.IssueKey == "" || number == 0 { + return + } + projectId, err := uuid.Parse(msg.GitHub.ProjectId) + if err != nil { + traceway.CaptureException(fmt.Errorf("GitHub issue %s/%s#%d has an unusable project id %q: %w", adapter.Owner, adapter.Repo, number, msg.GitHub.ProjectId, err)) + return + } + _, err = db.ExecuteTransaction(func(tx *sql.Tx) (int, error) { + return transactional.GithubIssueRepository.Create(tx, &models.GithubIssue{ + ProjectId: projectId, + ChannelId: msg.GitHub.ChannelId, + IssueKey: msg.GitHub.IssueKey, + Owner: adapter.Owner, + Repo: adapter.Repo, + IssueNumber: number, + CreatedAt: time.Now().UTC(), + }) + }) + if err != nil { + traceway.CaptureException(fmt.Errorf("failed to record GitHub issue %s/%s#%d for exception %s: %w", adapter.Owner, adapter.Repo, number, msg.GitHub.IssueKey, err)) + } +} + +// trackGitHubIssue marks a rule delivery so the issue it opens is remembered. +// Only issue-shaped rules qualify: an issue opened for a latency or metric rule +// has no exception to archive, so nothing would ever close it. +func trackGitHubIssue(msg *Message, rule *models.NotificationRuleWithChannel, channelId int) { + if !models.IsIssueRuleType(rule.RuleType) || msg.DedupToken == "" { + return + } + msg.GitHub = &models.NotificationGitHub{ + IssueKey: msg.DedupToken, + ProjectId: rule.ProjectId.String(), + ChannelId: channelId, + } +} + +// CloseGitHubIssuesForArchived queues a close for every GitHub issue still open +// for the given exception hashes, in the caller's transaction. That commit is +// the durable promise, so the issues stop being tracked here rather than when +// the close lands: the outbox already retries the send, and a row left open +// would queue a second close on the next archive. Callers Wake the outbox after +// their commit. Returns how many closes were queued. +func CloseGitHubIssuesForArchived(tx *sql.Tx, projectId uuid.UUID, hashes []string) (int, error) { + queued := 0 + now := time.Now().UTC() + for _, hash := range hashes { + issues, err := transactional.GithubIssueRepository.FindOpenByIssueKey(tx, projectId, hash) + if err != nil { + return queued, err + } + for _, issue := range issues { + closed, err := transactional.GithubIssueRepository.MarkClosed(tx, issue.Id, now) + if err != nil { + return queued, err + } + if !closed { + continue + } + channel, err := transactional.NotificationChannelRepository.FindById(tx, issue.ChannelId) + if err != nil { + return queued, err + } + // A channel retyped away from GitHub has no credentials left to + // close with; stop tracking rather than queueing a send that can + // only fail. A disabled one still closes: this is one-shot cleanup + // of an issue that channel itself opened, not a new notification. + if channel == nil || channel.ChannelType != "github" { + continue + } + if _, err := outbox.Enqueue(tx, outbox.Delivery{ + Kind: models.OutboxKindGithubClose, + AdapterType: channel.ChannelType, + AdapterConfig: json.RawMessage(channel.Config), + Message: buildGitHubCloseMessage(issue), + ProjectId: &projectId, + ChannelName: channel.Name, + }); err != nil { + return queued, err + } + queued++ + } + } + return queued, nil +} + +func buildGitHubCloseMessage(issue *models.GithubIssue) Message { + var body strings.Builder + body.WriteString("Closed automatically by Traceway: the issue this tracks was archived.") + if link := dashboardURL("/issues/" + issue.IssueKey); link != "" { + fmt.Fprintf(&body, "\n\n%s", link) + } + return Message{ + Subject: "Issue archived in Traceway", + Body: body.String(), + Severity: SeverityInfo, + URL: "/issues/" + issue.IssueKey, + GitHub: &models.NotificationGitHub{ + IssueKey: issue.IssueKey, + CloseNumber: issue.IssueNumber, + Owner: issue.Owner, + Repo: issue.Repo, + }, + } +} + +// dashboardURL builds an absolute link to a dashboard path, or "" when the +// deployment has not published an origin. A link is left out rather than +// guessed: the comment it goes into is public, and a localhost URL there helps +// nobody. +func dashboardURL(path string) string { + if config.Config == nil || config.Config.AppBaseURL == "" { + return "" + } + return strings.TrimRight(config.Config.AppBaseURL, "/") + path +} diff --git a/backend/app/notifications/github_issues_sqlite_test.go b/backend/app/notifications/github_issues_sqlite_test.go new file mode 100644 index 000000000..e05d2bb9b --- /dev/null +++ b/backend/app/notifications/github_issues_sqlite_test.go @@ -0,0 +1,295 @@ +//go:build !transactional_pg && !telemetry_ch && !telemetry_duckdb + +package notifications + +import ( + "context" + "database/sql" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/google/uuid" + "github.com/tracewayapp/traceway/backend/app/db" + "github.com/tracewayapp/traceway/backend/app/dbtest" + "github.com/tracewayapp/traceway/backend/app/models" + "github.com/tracewayapp/traceway/backend/app/repositories/transactional" +) + +type githubFixture struct { + ProjectId uuid.UUID + Channel *models.NotificationChannel + Rule *models.NotificationRuleWithChannel +} + +func setupGitHubDB(t *testing.T) *githubFixture { + t.Helper() + + dbtest.SetupSQLite(t) + t.Cleanup(func() { + cooldowns.mu.Lock() + cooldowns.fired = make(map[int]time.Time) + cooldowns.mu.Unlock() + }) + + fixture := &githubFixture{} + _, err := db.ExecuteTransaction(func(tx *sql.Tx) (struct{}, error) { + org, err := transactional.OrganizationRepository.Create(tx, "Acme", "UTC") + if err != nil { + return struct{}{}, err + } + project, err := transactional.ProjectRepository.CreateWithOrganization(tx, "api", "gin", org.Id) + if err != nil { + return struct{}{}, err + } + fixture.ProjectId = project.Id + now := time.Now().UTC() + channel := &models.NotificationChannel{ + ProjectId: project.Id, + Name: "Backlog", + ChannelType: "github", + Config: []byte(`{"token":"ghp_x","owner":"acme","repo":"backend"}`), + Enabled: true, + CreatedAt: now, + UpdatedAt: now, + } + channelId, err := transactional.NotificationChannelRepository.Create(tx, channel) + if err != nil { + return struct{}{}, err + } + channel.Id = channelId + fixture.Channel = channel + fixture.Rule = &models.NotificationRuleWithChannel{ + Id: 7, ProjectId: project.Id, ChannelId: channelId, + Name: "New errors", RuleType: "new_error", CooldownMinutes: 15, + ChannelType: "github", ChannelName: "Backlog", + } + return struct{}{}, nil + }) + if err != nil { + t.Fatalf("seed: %v", err) + } + return fixture +} + +func recordIssue(t *testing.T, fixture *githubFixture, issueKey string, number int) int { + t.Helper() + id, err := db.ExecuteTransaction(func(tx *sql.Tx) (int, error) { + return transactional.GithubIssueRepository.Create(tx, &models.GithubIssue{ + ProjectId: fixture.ProjectId, + ChannelId: fixture.Channel.Id, + IssueKey: issueKey, + Owner: "acme", + Repo: "backend", + IssueNumber: number, + CreatedAt: time.Now().UTC(), + }) + }) + if err != nil { + t.Fatalf("record issue: %v", err) + } + return id +} + +func closeForArchived(t *testing.T, projectId uuid.UUID, hashes []string) int { + t.Helper() + queued, err := db.ExecuteTransaction(func(tx *sql.Tx) (int, error) { + return CloseGitHubIssuesForArchived(tx, projectId, hashes) + }) + if err != nil { + t.Fatalf("close for archived: %v", err) + } + return queued +} + +func decodeMessage(t *testing.T, row *models.OutboxDelivery) Message { + t.Helper() + var msg Message + if err := json.Unmarshal(row.Message, &msg); err != nil { + t.Fatalf("decode outbox message: %v", err) + } + return msg +} + +func TestDispatchTagsGitHubIssueDeliveries(t *testing.T) { + fixture := setupGitHubDB(t) + + if !dispatch(fixture.Rule, Message{Subject: "New error", Body: "b", DedupToken: "hash-1"}) { + t.Fatal("dispatch should report a durable enqueue") + } + rows := outboxRows(t) + if len(rows) != 1 { + t.Fatalf("expected 1 outbox row, got %d", len(rows)) + } + msg := decodeMessage(t, rows[0]) + if msg.GitHub == nil { + t.Fatal("github rule delivery should carry the tracking payload") + } + if msg.GitHub.IssueKey != "hash-1" { + t.Errorf("issue key = %q, expected hash-1", msg.GitHub.IssueKey) + } + if msg.GitHub.ProjectId != fixture.ProjectId.String() { + t.Errorf("project id = %q, expected %s", msg.GitHub.ProjectId, fixture.ProjectId) + } + if msg.GitHub.ChannelId != fixture.Channel.Id { + t.Errorf("channel id = %d, expected %d", msg.GitHub.ChannelId, fixture.Channel.Id) + } +} + +func TestDispatchDoesNotTagNonIssueRules(t *testing.T) { + fixture := setupGitHubDB(t) + fixture.Rule.RuleType = "error_rate" + + if !dispatch(fixture.Rule, Message{Subject: "Error rate high", DedupToken: "GET /users"}) { + t.Fatal("dispatch should report a durable enqueue") + } + // An issue opened for an endpoint has no exception to archive, so tracking + // it would leave a row nothing ever closes. + if msg := decodeMessage(t, outboxRows(t)[0]); msg.GitHub != nil { + t.Errorf("non-issue rule should not be tracked, got %+v", msg.GitHub) + } +} + +func TestCloseGitHubIssuesForArchivedQueuesClose(t *testing.T) { + fixture := setupGitHubDB(t) + recordIssue(t, fixture, "hash-1", 42) + + if queued := closeForArchived(t, fixture.ProjectId, []string{"hash-1"}); queued != 1 { + t.Fatalf("queued = %d, expected 1", queued) + } + + rows := outboxRows(t) + if len(rows) != 1 { + t.Fatalf("expected 1 outbox row, got %d", len(rows)) + } + row := rows[0] + if row.Kind != models.OutboxKindGithubClose || row.AdapterType != "github" { + t.Errorf("wrong delivery shape: kind=%s adapter=%s", row.Kind, row.AdapterType) + } + if string(row.AdapterConfig) != string(fixture.Channel.Config) { + t.Errorf("close should snapshot the channel config, got %s", row.AdapterConfig) + } + msg := decodeMessage(t, row) + if msg.GitHub == nil || msg.GitHub.CloseNumber != 42 || msg.GitHub.Owner != "acme" || msg.GitHub.Repo != "backend" { + t.Fatalf("close payload wrong: %+v", msg.GitHub) + } + + // The issue stops being tracked at enqueue, so archiving again is a no-op + // rather than a second close. + if queued := closeForArchived(t, fixture.ProjectId, []string{"hash-1"}); queued != 0 { + t.Errorf("second archive queued %d closes, expected 0", queued) + } + if len(outboxRows(t)) != 1 { + t.Errorf("second archive enqueued another delivery") + } + + open, err := db.ExecuteTransaction(func(tx *sql.Tx) ([]*models.GithubIssue, error) { + return transactional.GithubIssueRepository.FindOpenByIssueKey(tx, fixture.ProjectId, "hash-1") + }) + if err != nil { + t.Fatalf("reload issue: %v", err) + } + if len(open) != 0 { + t.Errorf("issue should no longer be open, got %+v", open[0]) + } +} + +func TestCloseGitHubIssuesIgnoresOtherProjectsAndHashes(t *testing.T) { + fixture := setupGitHubDB(t) + recordIssue(t, fixture, "hash-1", 42) + + if queued := closeForArchived(t, fixture.ProjectId, []string{"hash-2"}); queued != 0 { + t.Errorf("unrelated hash queued %d closes", queued) + } + if queued := closeForArchived(t, uuid.New(), []string{"hash-1"}); queued != 0 { + t.Errorf("another project queued %d closes", queued) + } +} + +func TestCloseGitHubIssuesSkipsRetypedChannel(t *testing.T) { + fixture := setupGitHubDB(t) + recordIssue(t, fixture, "hash-1", 42) + + _, err := db.ExecuteTransaction(func(tx *sql.Tx) (struct{}, error) { + fixture.Channel.ChannelType = "slack" + fixture.Channel.Config = []byte(`{"webhookUrl":"https://hooks.example.com/x"}`) + return struct{}{}, transactional.NotificationChannelRepository.Update(tx, fixture.Channel) + }) + if err != nil { + t.Fatalf("retype channel: %v", err) + } + + if queued := closeForArchived(t, fixture.ProjectId, []string{"hash-1"}); queued != 0 { + t.Errorf("retyped channel queued %d closes", queued) + } + if len(outboxRows(t)) != 0 { + t.Error("a channel that is no longer GitHub must not be sent to") + } + if queued := closeForArchived(t, fixture.ProjectId, []string{"hash-1"}); queued != 0 { + t.Error("the untrackable issue should have been dropped, not retried") + } +} + +func TestSendGitHubIssueRecordsCreatedIssue(t *testing.T) { + fixture := setupGitHubDB(t) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"number":99}`)) + })) + defer server.Close() + + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "backend", baseURL: server.URL} + msg := Message{ + Subject: "New error", + GitHub: &models.NotificationGitHub{ + IssueKey: "hash-1", + ProjectId: fixture.ProjectId.String(), + ChannelId: fixture.Channel.Id, + }, + } + if err := sendGitHubIssue(context.Background(), adapter, msg); err != nil { + t.Fatalf("send: %v", err) + } + + open, err := db.ExecuteTransaction(func(tx *sql.Tx) ([]*models.GithubIssue, error) { + return transactional.GithubIssueRepository.FindOpenByIssueKey(tx, fixture.ProjectId, "hash-1") + }) + if err != nil { + t.Fatalf("load issues: %v", err) + } + if len(open) != 1 { + t.Fatalf("expected the created issue to be tracked, got %d rows", len(open)) + } + if open[0].IssueNumber != 99 || open[0].Owner != "acme" || open[0].Repo != "backend" { + t.Errorf("recorded issue wrong: %+v", open[0]) + } +} + +func TestSendGitHubIssueSkipsRecordingUntrackedDeliveries(t *testing.T) { + setupGitHubDB(t) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"number":99}`)) + })) + defer server.Close() + + adapter := &GitHubAdapter{Token: "ghp_x", Owner: "acme", Repo: "backend", baseURL: server.URL} + if err := sendGitHubIssue(context.Background(), adapter, Message{Subject: "Error rate high"}); err != nil { + t.Fatalf("send: %v", err) + } + + count, err := db.ExecuteTransaction(func(tx *sql.Tx) (int, error) { + var n int + return n, tx.QueryRow("SELECT COUNT(*) FROM github_issues").Scan(&n) + }) + if err != nil { + t.Fatalf("count issues: %v", err) + } + if count != 0 { + t.Errorf("untracked delivery recorded %d rows", count) + } +} diff --git a/backend/app/notifications/outbox_hooks.go b/backend/app/notifications/outbox_hooks.go index e955e9cc4..2633a75e6 100644 --- a/backend/app/notifications/outbox_hooks.go +++ b/backend/app/notifications/outbox_hooks.go @@ -18,6 +18,12 @@ func AdapterSend(ctx context.Context, adapterType string, adapterConfig json.Raw if err != nil { return err } + // GitHub deliveries are two-way: they open an issue and later close it, + // and an opened one is recorded so the close can find it. Adapter.Send + // only covers the open half. + if github, ok := adapter.(*GitHubAdapter); ok { + return sendGitHubIssue(ctx, github, msg) + } return adapter.Send(ctx, msg) } diff --git a/backend/app/repositories/transactional/pg/github_issue.repository.go b/backend/app/repositories/transactional/pg/github_issue.repository.go new file mode 100644 index 000000000..2c45df660 --- /dev/null +++ b/backend/app/repositories/transactional/pg/github_issue.repository.go @@ -0,0 +1,45 @@ +//go:build transactional_pg + +package pg + +import ( + "database/sql" + "time" + + "github.com/tracewayapp/traceway/backend/app/models" + + "github.com/google/uuid" + "github.com/tracewayapp/lit/v2" +) + +type githubIssueRepository struct{} + +const githubIssueColumns = "id, project_id, channel_id, issue_key, owner, repo, issue_number, created_at, closed_at" + +// Create records an issue the GitHub channel just opened. +func (r *githubIssueRepository) Create(tx *sql.Tx, issue *models.GithubIssue) (int, error) { + return lit.Insert[models.GithubIssue](tx, issue) +} + +// FindOpenByIssueKey returns every issue still open for one exception hash. +// A hash can hold more than one: several GitHub channels can watch the same +// project, and a regression opens a fresh issue after the last one closed. +func (r *githubIssueRepository) FindOpenByIssueKey(tx *sql.Tx, projectId uuid.UUID, issueKey string) ([]*models.GithubIssue, error) { + return lit.SelectNamed[models.GithubIssue]( + tx, + "SELECT "+githubIssueColumns+" FROM github_issues WHERE project_id = :project_id AND issue_key = :issue_key AND closed_at IS NULL ORDER BY id ASC", + lit.P{"project_id": projectId, "issue_key": issueKey}, + ) +} + +// MarkClosed stops tracking an issue. The closed_at guard makes a second +// archive of the same hash a no-op instead of a duplicate close. +func (r *githubIssueRepository) MarkClosed(tx *sql.Tx, id int, closedAt time.Time) (bool, error) { + return guardedStatusUpdate( + tx, + "UPDATE github_issues SET closed_at = :closed_at WHERE id = :id AND closed_at IS NULL", + lit.P{"closed_at": closedAt.UTC(), "id": id}, + ) +} + +var GithubIssueRepository = githubIssueRepository{} diff --git a/backend/app/repositories/transactional/sqlite/github_issue.repository.go b/backend/app/repositories/transactional/sqlite/github_issue.repository.go new file mode 100644 index 000000000..674c332d2 --- /dev/null +++ b/backend/app/repositories/transactional/sqlite/github_issue.repository.go @@ -0,0 +1,45 @@ +//go:build !transactional_pg + +package sqlite + +import ( + "database/sql" + "time" + + "github.com/tracewayapp/traceway/backend/app/models" + + "github.com/google/uuid" + "github.com/tracewayapp/lit/v2" +) + +type githubIssueRepository struct{} + +const githubIssueColumns = "id, project_id, channel_id, issue_key, owner, repo, issue_number, created_at, closed_at" + +// Create records an issue the GitHub channel just opened. +func (r *githubIssueRepository) Create(tx *sql.Tx, issue *models.GithubIssue) (int, error) { + return lit.Insert[models.GithubIssue](tx, issue) +} + +// FindOpenByIssueKey returns every issue still open for one exception hash. +// A hash can hold more than one: several GitHub channels can watch the same +// project, and a regression opens a fresh issue after the last one closed. +func (r *githubIssueRepository) FindOpenByIssueKey(tx *sql.Tx, projectId uuid.UUID, issueKey string) ([]*models.GithubIssue, error) { + return lit.SelectNamed[models.GithubIssue]( + tx, + "SELECT "+githubIssueColumns+" FROM github_issues WHERE project_id = :project_id AND issue_key = :issue_key AND closed_at IS NULL ORDER BY id ASC", + lit.P{"project_id": projectId, "issue_key": issueKey}, + ) +} + +// MarkClosed stops tracking an issue. The closed_at guard makes a second +// archive of the same hash a no-op instead of a duplicate close. +func (r *githubIssueRepository) MarkClosed(tx *sql.Tx, id int, closedAt time.Time) (bool, error) { + return guardedStatusUpdate( + tx, + "UPDATE github_issues SET closed_at = :closed_at WHERE id = :id AND closed_at IS NULL", + lit.P{"closed_at": closedAt.UTC(), "id": id}, + ) +} + +var GithubIssueRepository = githubIssueRepository{} diff --git a/backend/app/repositories/transactional/transactional_pg.go b/backend/app/repositories/transactional/transactional_pg.go index bc5ad3a9d..f76cf8476 100644 --- a/backend/app/repositories/transactional/transactional_pg.go +++ b/backend/app/repositories/transactional/transactional_pg.go @@ -12,6 +12,7 @@ var ( DashboardTemplateRepository = pgrepo.DashboardTemplateRepository DeviceAuthorizationRepository = pgrepo.DeviceAuthorizationRepository EscalationPolicyRepository = pgrepo.EscalationPolicyRepository + GithubIssueRepository = pgrepo.GithubIssueRepository IncidentUpdateRepository = pgrepo.IncidentUpdateRepository InvitationRepository = pgrepo.InvitationRepository MetricRegistryRepository = pgrepo.MetricRegistryRepository diff --git a/backend/app/repositories/transactional/transactional_sqlite.go b/backend/app/repositories/transactional/transactional_sqlite.go index 6787fa7d8..f6e9d978c 100644 --- a/backend/app/repositories/transactional/transactional_sqlite.go +++ b/backend/app/repositories/transactional/transactional_sqlite.go @@ -12,6 +12,7 @@ var ( DashboardTemplateRepository = sqliterepo.DashboardTemplateRepository DeviceAuthorizationRepository = sqliterepo.DeviceAuthorizationRepository EscalationPolicyRepository = sqliterepo.EscalationPolicyRepository + GithubIssueRepository = sqliterepo.GithubIssueRepository IncidentUpdateRepository = sqliterepo.IncidentUpdateRepository InvitationRepository = sqliterepo.InvitationRepository MetricRegistryRepository = sqliterepo.MetricRegistryRepository diff --git a/docs/pages/learn/alerts.mdx b/docs/pages/learn/alerts.mdx index 02f0831d2..d5176f15b 100644 --- a/docs/pages/learn/alerts.mdx +++ b/docs/pages/learn/alerts.mdx @@ -20,7 +20,7 @@ A channel represents a notification destination. Traceway supports the following | Email | Sends an email to one or more recipients | List of email addresses | | Slack | Posts a message to a Slack channel via incoming webhook | Webhook URL | | Webhook | Sends an HTTP POST with a JSON payload to any URL | URL, optional headers, optional HMAC secret | -| GitHub | Creates a GitHub issue in a repository | Personal access token, repository owner/name, optional labels | +| GitHub | Opens a GitHub issue in a repository, and closes it again when the error is archived | Personal access token, repository owner/name, optional labels | | Pushover | Sends push notifications to your mobile devices | User Key, App Token | | Telegram | Sends a message via a Telegram bot to a user or group chat | Bot Token, Chat ID | | Escalation policy | Opens an on-call [page](/learn/on-call) and runs the policy's escalation chain until somebody acknowledges, instead of sending a message | Escalation policy | @@ -76,6 +76,18 @@ Then configure the GitHub channel in Traceway: GitHub channel configuration dialog +#### Closing Issues Automatically + +When a **New Error** or **Error Regression** rule opens a GitHub issue, Traceway remembers which issue tracks which error. Archiving that error — from the issues list, the issue page, or by resolving its on-call page with **Archive issue** ticked — closes the GitHub issue and leaves a comment linking back to Traceway. Nothing to configure: it applies to every GitHub channel. + +A few details worth knowing: + +- Only issues opened by error rules are tracked. An issue opened by a latency or metric rule has no error to archive, so it stays open for you to close by hand. +- The issue is closed in the repository it was created in, even if the channel has since been pointed at a different one. +- If the error comes back, an Error Regression rule opens a **new** issue rather than reopening the closed one. Archiving again closes that one too. +- Deleting the channel forgets its issues. Any that are still open stay open, since the token that could close them is gone. +- Set `APP_BASE_URL` on the server for the closing comment to carry a link back to the error in Traceway. + ### Setting Up Telegram To send alerts to Telegram, you need a **Bot Token** and a **Chat ID**: From 7294d05583d6a6e312a713a62f96c1231c08856c Mon Sep 17 00:00:00 2001 From: Vuk <129513318+vuks19@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:09:38 +0200 Subject: [PATCH 2/2] use already existing dashboardUrl function --- backend/app/notifications/github_issues.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/backend/app/notifications/github_issues.go b/backend/app/notifications/github_issues.go index c8cb27445..40900ab2f 100644 --- a/backend/app/notifications/github_issues.go +++ b/backend/app/notifications/github_issues.go @@ -126,8 +126,11 @@ func CloseGitHubIssuesForArchived(tx *sql.Tx, projectId uuid.UUID, hashes []stri func buildGitHubCloseMessage(issue *models.GithubIssue) Message { var body strings.Builder body.WriteString("Closed automatically by Traceway: the issue this tracks was archived.") - if link := dashboardURL("/issues/" + issue.IssueKey); link != "" { - fmt.Fprintf(&body, "\n\n%s", link) + // Unlike dashboardURL's normal fallback (a bare relative path), the link is + // left out entirely when unset: the comment it goes into is public, and a + // relative or localhost path there helps nobody outside Traceway. + if config.Config.PublicBaseURL() != "" { + fmt.Fprintf(&body, "\n\n%s", dashboardURL("/issues/"+issue.IssueKey)) } return Message{ Subject: "Issue archived in Traceway", @@ -142,14 +145,3 @@ func buildGitHubCloseMessage(issue *models.GithubIssue) Message { }, } } - -// dashboardURL builds an absolute link to a dashboard path, or "" when the -// deployment has not published an origin. A link is left out rather than -// guessed: the comment it goes into is public, and a localhost URL there helps -// nobody. -func dashboardURL(path string) string { - if config.Config == nil || config.Config.AppBaseURL == "" { - return "" - } - return strings.TrimRight(config.Config.AppBaseURL, "/") + path -}