From 126a215892b48b7328de3b0cf5eaa1598875be06 Mon Sep 17 00:00:00 2001 From: Maris Popens Date: Sun, 27 Sep 2026 09:16:11 +0300 Subject: [PATCH] chore: trim comments --- .github/workflows/build.yml | 3 +-- Dockerfile | 4 +-- internal/collector/collector.go | 18 +++---------- internal/config/config.go | 10 +++----- internal/fetch/fetch.go | 24 +++++------------- internal/github/client.go | 45 ++++++++------------------------- internal/github/types.go | 10 +++----- internal/orgstats/orgstats.go | 34 ++++++------------------- internal/runners/runners.go | 6 +---- 9 files changed, 38 insertions(+), 116 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8160010..153198a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,8 +8,7 @@ on: - '**.md' - 'LICENSE' - '.gitignore' - # CI-config-only changes skip the build; this file is re-included so edits - # to it still run. + # CI-config-only changes skip the build; this file re-included so its own edits run. - '.github/**' - '!.github/workflows/build.yml' workflow_dispatch: diff --git a/Dockerfile b/Dockerfile index 91ff4af..7777e9e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,4 @@ -# Two-stage build. Pure Go, CGO disabled - a single static binary, no -# libc needed. distroless/static bundles CA certificates (HTTPS to the -# GitHub API) and nothing else. +# Static CGO-free binary; distroless/static brings CA certs and nothing else. FROM golang:1.27-trixie AS builder WORKDIR /src COPY go.mod go.sum ./ diff --git a/internal/collector/collector.go b/internal/collector/collector.go index 4695256..15cecaa 100644 --- a/internal/collector/collector.go +++ b/internal/collector/collector.go @@ -1,6 +1,4 @@ -// Package collector adapts runners.Summary and orgstats.Summary into a -// prometheus.Collector, so metrics are computed fresh from the shared -// caches on every scrape rather than accumulated/pushed. +// Package collector computes metrics from the shared caches on every scrape. package collector import ( @@ -36,10 +34,7 @@ type Collector struct { repoDependabot *prometheus.Desc } -// New builds the collector. runnerCacheTTL/orgCacheTTL are only used to -// document each metric's caching behavior in its HELP text, per -// Prometheus's own guidance - they don't change the actual caching -// (that's each Fetcher's job). +// New builds the collector. The TTLs only feed HELP text; caching is the Fetchers' job. func New(runnerFetcher *fetch.Fetcher[runners.Summary], orgFetcher *fetch.Fetcher[orgstats.Summary], runnerCacheTTL, orgCacheTTL time.Duration) *Collector { runnerNote := fmt.Sprintf(" Cached for up to %s.", runnerCacheTTL) orgNote := fmt.Sprintf(" Cached for up to %s - not real-time by design, see internal/orgstats.", orgCacheTTL) @@ -69,13 +64,8 @@ func New(runnerFetcher *fetch.Fetcher[runners.Summary], orgFetcher *fetch.Fetche repoOpenPRs: desc("repo", "open_prs", "Number of open pull requests."+orgNote, []string{"repo"}), - // Always 1 - an "info" style metric. The conclusion label - // carries GitHub's own conclusion string verbatim (success, - // failure, cancelled, skipped, neutral, timed_out, - // action_required, stale) rather than this exporter collapsing - // it to pass/fail - what counts as "actually broken" is a - // dashboard-level judgment call, not this exporter's to make. - // Absent if the workflow has never run. + // info metric, always 1. conclusion is GitHub's raw string: what counts as + // broken is the dashboard's call. Absent if the workflow never ran. repoCILastRunConclusion: desc("repo", "ci_last_run_conclusion", "Always 1; the workflow's latest completed run outcome is in the conclusion label."+orgNote, []string{"repo", "workflow", "url", "conclusion"}), repoCILastRunAt: desc("repo", "ci_last_run_timestamp_seconds", diff --git a/internal/config/config.go b/internal/config/config.go index dd31366..2329770 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,6 +1,4 @@ -// Package config loads settings from environment variables. Env vars -// only (no flags/files) - this is meant to run as a container, where -// env vars are the natural configuration surface. +// Package config reads env vars only; it runs as a container. package config import ( @@ -15,13 +13,11 @@ type Config struct { ListenAddr string RequestTimeout time.Duration - // Runner status is genuinely real-time - short TTL. + // runner status is real-time: short TTL RunnerCacheTTL time.Duration RunnerCacheMaxStale time.Duration - // CI/PR/Dependabot/org stats are not real-time, and cost ~3 API - // calls per repo per refresh - long TTL to stay well inside - // GitHub's rate limit across a few dozen repos. + // ~3 API calls per repo per refresh: long TTL to stay inside the rate limit OrgCacheTTL time.Duration OrgCacheMaxStale time.Duration } diff --git a/internal/fetch/fetch.go b/internal/fetch/fetch.go index ced761c..c6074d1 100644 --- a/internal/fetch/fetch.go +++ b/internal/fetch/fetch.go @@ -1,7 +1,5 @@ -// Package fetch caches the result of a build function so bursts of -// concurrent Prometheus scrapes don't each trigger their own round trip, -// and - more importantly for this exporter - so a scrape cadence tighter -// than GitHub's rate-limit budget doesn't burn through it. +// Package fetch caches build results so scrape bursts and a tight scrape +// interval don't burn GitHub's rate limit. package fetch import ( @@ -12,20 +10,10 @@ import ( "time" ) -// Fetcher caches whatever T a build function produces. Get never blocks -// a caller on an upstream round trip once it has anything cached at -// all - a stale cache triggers a refresh in the background and the -// stale value is returned immediately instead. This matters -// specifically because Get is called from a Prometheus scrape handler: -// blocking on the upstream API (as an earlier version of this did) -// coupled scrape response time to that API's latency, and once the TTL -// was close to the scrape interval, most scrapes ended up doing a live -// fetch inline - slow enough, often enough, to blow past Prometheus's -// own scrape timeout and show up as real gaps in the data. -// -// The one exception is the very first call ever, before anything has -// been fetched at all - there's nothing to serve yet, so that one has -// to block. +// Fetcher caches a build function's T. Once anything is cached, Get never +// blocks: a stale value is returned and refreshed in the background. Blocking +// in the scrape handler used to blow Prometheus's scrape timeout. Only the very +// first call blocks. type Fetcher[T any] struct { build func(context.Context) (T, error) ttl time.Duration diff --git a/internal/github/client.go b/internal/github/client.go index a18dcd1..0ace0ee 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -1,6 +1,4 @@ -// Package github is a minimal client for the GitHub REST endpoints this -// exporter needs. It deliberately does not try to be a general-purpose -// GitHub API client. +// Package github is a minimal client for the GitHub endpoints this exporter uses. package github import ( @@ -61,11 +59,7 @@ func (c *Client) get(ctx context.Context, url string, out interface{}) error { return nil } -// Runners returns every self-hosted runner registered at the -// organization level. GitHub paginates this endpoint at 30 per page by -// default; a homelab-sized org with a handful of runners never needs a -// second page, so pagination is deliberately not implemented here - -// add it if this ever needs to scale past 100 runners. +// Runners returns the org's self-hosted runners. No pagination; add it past 100. func (c *Client) Runners(ctx context.Context) ([]Runner, error) { var out listRunnersResponse url := fmt.Sprintf("%s/orgs/%s/actions/runners?per_page=100", apiBase, c.org) @@ -75,10 +69,7 @@ func (c *Client) Runners(ctx context.Context) ([]Runner, error) { return out.Runners, nil } -// Repos returns every non-forked repo in the org. Capped at 100 - this -// org has a few dozen, well under GitHub's page size, so pagination is -// deliberately not implemented; add it if the org ever grows past 100 -// repos. +// Repos returns the org's non-fork repos. No pagination; add it past 100. func (c *Client) Repos(ctx context.Context) ([]Repo, error) { var out []Repo url := fmt.Sprintf("%s/orgs/%s/repos?per_page=100&type=all", apiBase, c.org) @@ -88,9 +79,7 @@ func (c *Client) Repos(ctx context.Context) ([]Repo, error) { return out, nil } -// Workflows returns a repo's workflow definitions. Capped at 100 - no -// repo here plausibly has more than that many workflow files; add -// pagination if that ever changes. +// Workflows returns a repo's workflow definitions. No pagination; add it past 100. func (c *Client) Workflows(ctx context.Context, repo string) ([]Workflow, error) { var out listWorkflowsResponse url := fmt.Sprintf("%s/repos/%s/%s/actions/workflows?per_page=100", apiBase, c.org, repo) @@ -100,14 +89,9 @@ func (c *Client) Workflows(ctx context.Context, repo string) ([]Workflow, error) return out.Workflows, nil } -// LatestRunForWorkflow returns the most recently created run of one -// specific workflow. Checking per-workflow, rather than the single -// most recent run across a repo's whole Actions history, matters: a -// repo with several workflows (e.g. a Validate that runs on PRs and a -// Build that runs on push) would otherwise report whichever one -// happened to run last as if it spoke for all of them - a failing -// Validate can sit hidden behind a later, unrelated, successful Build. -// Returns ok=false if this workflow has never run. +// LatestRunForWorkflow returns one workflow's most recent run, ok=false if it +// never ran. Per workflow, so a failing Validate can't hide behind a later +// green Build in the same repo. func (c *Client) LatestRunForWorkflow(ctx context.Context, repo string, workflowID int64) (run WorkflowRun, ok bool, err error) { var out listWorkflowRunsResponse url := fmt.Sprintf("%s/repos/%s/%s/actions/workflows/%d/runs?per_page=1", apiBase, c.org, repo, workflowID) @@ -122,11 +106,8 @@ func (c *Client) LatestRunForWorkflow(ctx context.Context, repo string, workflow var lastPageRegexp = regexp.MustCompile(`[?&]page=(\d+)>;\s*rel="last"`) -// OpenPRCount returns the number of open pull requests in a repo, -// without paginating through them: it reads the page count off the -// Link response header of a per_page=1 request instead. If there's no -// "last" rel (0 or 1 open PRs), the length of the single returned page -// is the count. +// OpenPRCount reads the count off the Link header's "last" page of a +// per_page=1 request; with no "last" rel it's the page length. func (c *Client) OpenPRCount(ctx context.Context, repo string) (int, error) { url := fmt.Sprintf("%s/repos/%s/%s/pulls?state=open&per_page=1", apiBase, c.org, repo) resp, err := c.request(ctx, url) @@ -154,10 +135,7 @@ func (c *Client) OpenPRCount(ctx context.Context, repo string) (int, error) { return len(page), nil } -// DependabotAlerts returns every open Dependabot alert for a repo. -// Capped at 100 - a homelab-scale repo realistically never has more -// open alerts than that; a repo that somehow did would just undercount -// here rather than error. +// DependabotAlerts returns a repo's open alerts. Capped at 100; undercounts past that. func (c *Client) DependabotAlerts(ctx context.Context, repo string) ([]DependabotAlert, error) { var out []DependabotAlert url := fmt.Sprintf("%s/repos/%s/%s/dependabot/alerts?state=open&per_page=100", apiBase, c.org, repo) @@ -167,8 +145,7 @@ func (c *Client) DependabotAlerts(ctx context.Context, repo string) ([]Dependabo return out, nil } -// RateLimit returns the core API rate limit budget this client itself -// draws from. +// RateLimit returns the core rate-limit budget this client draws from. func (c *Client) RateLimit(ctx context.Context) (RateLimit, error) { var out rateLimitResponse if err := c.get(ctx, apiBase+"/rate_limit", &out); err != nil { diff --git a/internal/github/types.go b/internal/github/types.go index 1bd047a..d1e9af4 100644 --- a/internal/github/types.go +++ b/internal/github/types.go @@ -1,8 +1,6 @@ package github -// Runner is the subset of GitHub's self-hosted runner object this -// exporter cares about. See: -// https://docs.github.com/en/rest/actions/self-hosted-runners#list-self-hosted-runners-for-an-organization +// Runner is the subset of GitHub's self-hosted runner object we use. type Runner struct { ID int64 `json:"id"` Name string `json:"name"` @@ -41,8 +39,7 @@ type listWorkflowsResponse struct { Workflows []Workflow `json:"workflows"` } -// WorkflowRun is the subset of a workflow run this exporter needs to -// derive "is CI currently green" for one workflow. +// WorkflowRun is the subset of a workflow run we use. type WorkflowRun struct { Status string `json:"status"` // "completed" | "in_progress" | ... Conclusion string `json:"conclusion"` // "success" | "failure" | ... (empty until completed) @@ -64,8 +61,7 @@ type DependabotAlert struct { } `json:"security_advisory"` } -// RateLimit is the "core" resource from GET /rate_limit - the budget -// every other call in this client draws from. +// RateLimit is the "core" resource from GET /rate_limit. type RateLimit struct { Limit int `json:"limit"` Remaining int `json:"remaining"` diff --git a/internal/orgstats/orgstats.go b/internal/orgstats/orgstats.go index 2ec21ab..32d70b3 100644 --- a/internal/orgstats/orgstats.go +++ b/internal/orgstats/orgstats.go @@ -1,9 +1,5 @@ -// Package orgstats builds repo/CI health, PR counts, Dependabot alert -// counts and rate-limit status for the org. Fetched on a slow cache TTL -// (see internal/fetch and main.go) - unlike runner status, none of this -// needs to be near-real-time, and polling ~3 endpoints per repo across -// a few dozen repos on a short TTL would burn through GitHub's rate -// limit for no benefit. +// Package orgstats builds repo/CI, PR, Dependabot and rate-limit stats on a +// slow TTL; none of it needs to be real-time. package orgstats import ( @@ -13,21 +9,13 @@ import ( "github.com/drumandbytes/github-actions-runner-exporter/internal/github" ) -// WorkflowCI is one active workflow's latest-run status - tracked per -// workflow, not per repo, so a failing workflow can't hide behind a -// later, unrelated, successful one in the same repo. +// WorkflowCI is one active workflow's latest-run status. type WorkflowCI struct { Name string HasRun bool // false if this workflow has never run - // The raw GitHub conclusion string: "success", "failure", - // "cancelled", "skipped", "neutral", "timed_out", - // "action_required" or "stale". Exposed as-is rather than - // collapsed to a pass/fail bool here - what counts as "actually - // broken" (e.g. whether "neutral" or "cancelled" should read as a - // problem) is a dashboard-level judgment call, not this exporter's - // to make. + // raw GitHub conclusion; pass/fail is the dashboard's call LastConclusion string LastRunAt time.Time @@ -41,14 +29,10 @@ type RepoStats struct { OpenPRs int - // Only active workflows (state == "active") are included - a - // disabled workflow's stale last run isn't a meaningful signal. + // active workflows only: a disabled one's last run means nothing Workflows []WorkflowCI - // severity -> open alert count. Only severities that actually occur - // are present - a repo with zero open "critical" alerts simply has - // no "critical" key, same convention as pve-metrics-exporter's - // optional per-sensor critical-threshold series. + // severity -> open count; absent severities have no key DependabotAlertsBySeverity map[string]int } @@ -82,10 +66,8 @@ func Build(ctx context.Context, client *github.Client) (Summary, error) { stats.Visibility = "public" } - // Per-repo call failures below are swallowed deliberately (not - // propagated as a Build error): a repo the token can't see, or - // one with Dependabot disabled (404), shouldn't blank out every - // other repo's data for this scrape. + // per-repo errors are swallowed: one repo the token can't see (or with + // Dependabot off, 404) mustn't blank the rest of the scrape if n, err := client.OpenPRCount(ctx, r.Name); err == nil { stats.OpenPRs = n } diff --git a/internal/runners/runners.go b/internal/runners/runners.go index 392c4e2..ab92085 100644 --- a/internal/runners/runners.go +++ b/internal/runners/runners.go @@ -1,8 +1,4 @@ -// Package runners builds the self-hosted-runner status summary - kept -// separate from orgstats because it's fetched on a much shorter cache -// TTL (runner up/busy is genuinely real-time; CI/PR/Dependabot signals -// are not, and polling them that often would blow through GitHub's -// rate limit across two dozen repos). +// Package runners builds the runner status summary, on a much shorter TTL than orgstats. package runners import (