diff --git a/README.md b/README.md index 7a60b2f..65c986c 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,8 @@ flag alone. | `github_repo_pulls_draft` | gauge | `org`, `repo`, `author_kind` | | `github_pull_created_timestamp_seconds` | gauge | `org`, `repo`, `number`, `author`, `author_kind`, `draft` | | `github_workflow_run_status` | gauge | `org`, `repo`, `workflow`, `event`, `conclusion` | +| `github_workflow_run_stale` | gauge | `org`, `repo`, `workflow` | +| `github_workflow_enabled` | gauge | `org`, `repo`, `workflow`, `state` | | `github_workflow_run_timestamp_seconds` | gauge | `org`, `repo`, `workflow` | | `github_workflow_last_success_timestamp_seconds` | gauge | `org`, `repo`, `workflow` | | `github_workflow_expected_interval_seconds` | gauge | `org`, `repo`, `workflow` | @@ -114,6 +116,19 @@ activity for visibility while alert rules select only `author_kind="human"`. workflow. An in-flight run reports `conclusion="running"` and does not clear a previous failure. +## What counts as the current CI state + +Getting this right matters more than it sounds. Naively taking the newest run +per workflow produced 38 "failures", of which fewer than half were real: + +| Filter | Why | +| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Workflow must still exist | GitHub keeps run history after a workflow file is deleted. A removed `Update pre-commit hooks` reported a permanent failure across 10 repositories. | +| Identity is the file path | Runs predating a workflow's `name:` report the path instead, splitting one workflow into two series. | +| Branch-state events only | The API's `branch=` filter matches a pull request's *head* branch, so PR runs leak in. A merged PR's last pre-merge failure would otherwise be the branch's CI state forever. | +| Runs age out after 90 days | Some workflows only fire on `pull_request`, leaving a branch-state run many months old. Those report `conclusion="stale"` and set `github_workflow_run_stale`, rather than an unclearable failure. | +| Disabled workflows are kept | A workflow auto-disabled by GitHub after 60 days of inactivity has stopped running silently. That is the fault worth alerting on, so it is reported rather than filtered out. | + ## Configuration TOML file, with `GHCI_`-prefixed environment overrides: diff --git a/github-ci-exporter/src/collector.rs b/github-ci-exporter/src/collector.rs index 6a01911..d06d58f 100644 --- a/github-ci-exporter/src/collector.rs +++ b/github-ci-exporter/src/collector.rs @@ -9,8 +9,8 @@ use crate::{ config::Config, github::{Client, client::RateLimitResource, graphql, rest}, metrics::{ - AuthorLabels, Metrics, PullLabels, RepoLabels, ResourceLabels, SkipLabels, WorkflowLabels, - WorkflowStateLabels, author_label, + AuthorLabels, Metrics, PullLabels, RepoLabels, ResourceLabels, SkipLabels, + WorkflowEnabledLabels, WorkflowLabels, WorkflowStateLabels, author_label, }, model::{Repo, SkipReason}, }; @@ -45,8 +45,12 @@ const fn estimate_core_requests(monitored: u64) -> u64 { pub struct WorkflowCache { /// `owner/name` -> workflow name -> expected interval in seconds. intervals: HashMap>, - /// `owner/name` -> whether the repo has any workflow files at all. - has_workflows: HashMap, + /// `owner/name` -> the workflow files currently present in the repo. + /// + /// Retained so run history can be intersected against it: GitHub keeps + /// runs of deleted workflows forever, and reporting them shows failures + /// for CI that no longer exists. + workflows: HashMap>, /// Repositories monitored by the previous cycle, used to size the /// budget pre-flight check. monitored_count: u64, @@ -139,36 +143,7 @@ pub async fn collect( let (candidates, mut skipped) = graphql::partition_repos(discovered, &|name| config.is_denylisted(name), max_age, now); - // Workflow presence. Repos with no workflow files are content-hosting - // repos with no CI signal; they are dropped so the dashboard is not - // padded with permanently-empty rows. - let mut monitored = Vec::with_capacity(candidates.len()); - for repo in candidates { - let key = repo.full_name(); - let workflows = match rest::list_workflows(client, &repo).await { - Ok(workflows) => workflows, - Err(error) => { - warn!(repo = %repo, %error, "failed to list workflows; keeping repository"); - monitored.push(repo); - continue; - } - }; - - let has_workflows = !workflows.is_empty(); - cache.has_workflows.insert(key.clone(), has_workflows); - - if !has_workflows && config.skip_repos_without_workflows { - skipped.push((repo, SkipReason::NoWorkflows)); - continue; - } - - // Resolve cron schedules once per workflow set; they change rarely. - if let std::collections::hash_map::Entry::Vacant(entry) = cache.intervals.entry(key) { - entry.insert(resolve_cron_intervals(client, &repo, &workflows).await); - } - - monitored.push(repo); - } + let monitored = resolve_monitored(client, config, cache, candidates, &mut skipped).await; info!( monitored = monitored.len(), @@ -189,8 +164,25 @@ pub async fn collect( // Actions runs, one request per repository. for repo in &monitored { - match rest::fetch_runs(client, repo).await { - Ok(runs) => record_runs(metrics, repo, &runs, cache), + let live = cache + .workflows + .get(&repo.full_name()) + .cloned() + .unwrap_or_default(); + // Without a workflow set every run would be discarded as orphaned, + // publishing no series at all and making a listing failure look like + // "this repository's CI vanished". Skip the fetch rather than spend a + // request that cannot produce a result. + if live.is_empty() { + warn!( + repo = %repo, + "no workflow set available; skipping run fetch for this cycle" + ); + continue; + } + record_workflow_states(metrics, repo, &live); + match rest::fetch_runs(client, repo, &live).await { + Ok(runs) => record_runs(metrics, repo, &runs, cache, now), Err(error) => warn!(repo = %repo, %error, "failed to fetch workflow runs"), } } @@ -281,6 +273,70 @@ fn record_repo_inventory(metrics: &Metrics, monitored: &[Repo], skipped: &[(Repo } } +/// Determines which candidates actually have CI, caching their workflow sets. +/// +/// Repositories with no workflow files are content-hosting repos with no CI +/// signal; they are dropped so the dashboard is not padded with permanently +/// empty rows. +async fn resolve_monitored( + client: &Client, + config: &Config, + cache: &mut WorkflowCache, + candidates: Vec, + skipped: &mut Vec<(Repo, SkipReason)>, +) -> Vec { + let mut monitored = Vec::with_capacity(candidates.len()); + + for repo in candidates { + let key = repo.full_name(); + let workflows = match rest::list_workflows(client, &repo).await { + Ok(workflows) => workflows, + Err(error) => { + // A listing failure is not evidence of absent CI, so the + // repository is kept rather than silently dropped. + warn!(repo = %repo, %error, "failed to list workflows; keeping repository"); + monitored.push(repo); + continue; + } + }; + + if workflows.is_empty() && config.skip_repos_without_workflows { + cache.workflows.remove(&key); + skipped.push((repo, SkipReason::NoWorkflows)); + continue; + } + cache.workflows.insert(key.clone(), workflows.clone()); + + // Cron schedules change rarely, so they are resolved once per set. + if let std::collections::hash_map::Entry::Vacant(entry) = cache.intervals.entry(key) { + entry.insert(resolve_cron_intervals(client, &repo, &workflows).await); + } + + monitored.push(repo); + } + + monitored +} + +/// Publishes whether GitHub will actually run each workflow. +/// +/// A workflow auto-disabled for inactivity has silently stopped running; the +/// `state` label lets an alert distinguish that from a deliberate manual +/// disable. +fn record_workflow_states(metrics: &Metrics, repo: &Repo, workflows: &[rest::Workflow]) { + for workflow in workflows { + metrics + .workflow_enabled + .get_or_create(&WorkflowEnabledLabels { + org: repo.owner.clone(), + repo: repo.name.clone(), + workflow: workflow.name.clone(), + state: workflow.state.as_str().to_owned(), + }) + .set(i64::from(workflow.state == rest::WorkflowState::Active)); + } +} + /// Resolves each workflow's expected run interval from its cron schedule. async fn resolve_cron_intervals( client: &Client, @@ -360,10 +416,36 @@ fn record_activity( } } -fn record_runs(metrics: &Metrics, repo: &Repo, runs: &rest::RepoRuns, cache: &WorkflowCache) { +fn record_runs( + metrics: &Metrics, + repo: &Repo, + runs: &rest::RepoRuns, + cache: &WorkflowCache, + now: DateTime, +) { let intervals = cache.intervals.get(&repo.full_name()); for run in &runs.latest { + // A run older than the staleness horizon says nothing about the + // current code. Reporting `stale` instead of its original conclusion + // keeps an ancient failure from producing an alert that cannot be + // cleared without an artificial push. `workflow_run_stale` carries the + // fact separately so it stays visible on the dashboard. + let stale = run.is_stale(now); + metrics + .workflow_run_stale + .get_or_create(&WorkflowLabels { + org: repo.owner.clone(), + repo: repo.name.clone(), + workflow: run.workflow.clone(), + }) + .set(i64::from(stale)); + + let conclusion = if stale { + "stale" + } else { + run.conclusion.as_str() + }; metrics .workflow_run_status .get_or_create(&WorkflowStateLabels { @@ -371,7 +453,7 @@ fn record_runs(metrics: &Metrics, repo: &Repo, runs: &rest::RepoRuns, cache: &Wo repo: repo.name.clone(), workflow: run.workflow.clone(), event: run.event.clone(), - conclusion: run.conclusion.as_str().to_owned(), + conclusion: conclusion.to_owned(), }) .set(1); diff --git a/github-ci-exporter/src/github/client.rs b/github-ci-exporter/src/github/client.rs index 264d2fe..7ec4280 100644 --- a/github-ci-exporter/src/github/client.rs +++ b/github-ci-exporter/src/github/client.rs @@ -370,6 +370,30 @@ impl Client { path: &str, project: F, ) -> Result<(R, CacheOutcome), ClientError> + where + T: DeserializeOwned, + R: Serialize + DeserializeOwned, + F: FnOnce(T) -> R, + { + self.get_cached_as(path, path, project).await + } + + /// As [`Self::get_cached`], but with the cache key decoupled from the + /// request path. + /// + /// Needed when the projection depends on inputs beyond the response body: + /// the cached value must be invalidated when those inputs change, even + /// though the request itself is unchanged. + /// + /// # Errors + /// Returns [`ClientError`] on transport failure, a non-success status, or + /// a body that does not match `T`. + pub async fn get_cached_as( + &self, + cache_key: &str, + path: &str, + project: F, + ) -> Result<(R, CacheOutcome), ClientError> where T: DeserializeOwned, R: Serialize + DeserializeOwned, @@ -380,12 +404,17 @@ impl Client { } else { format!("{}{}", self.api_url, path) }; + let cache_key = if cache_key == path { + url.clone() + } else { + cache_key.to_owned() + }; let cached_etag = self .cache .lock() .ok() - .and_then(|cache| cache.get(&url).map(|entry| entry.etag.clone())); + .and_then(|cache| cache.get(&cache_key).map(|entry| entry.etag.clone())); // A conditional request answered 304 is free, but that is only known // after the fact; the budget must be checked as though it will cost. @@ -409,7 +438,7 @@ impl Client { .cache .lock() .ok() - .and_then(|cache| cache.get(&url).map(|entry| entry.body.clone())); + .and_then(|cache| cache.get(&cache_key).map(|entry| entry.body.clone())); if let Some(cached) = cached { let value = serde_json::from_str(&cached).map_err(|source| ClientError::Decode { @@ -423,7 +452,7 @@ impl Client { // retry unconditionally. warn!(%url, "304 with no cached body; refetching unconditionally"); if let Ok(mut cache) = self.cache.lock() { - cache.remove(&url); + cache.remove(&cache_key); } let response = self.http.get(&url).send().await?; self.requests_total.fetch_add(1, Ordering::Relaxed); @@ -464,7 +493,7 @@ impl Client { && let Ok(mut cache) = self.cache.lock() { cache.insert( - url, + cache_key, CacheEntry { etag, body: encoded, @@ -726,6 +755,96 @@ mod tests { ); } + #[tokio::test] + async fn distinct_cache_keys_do_not_share_an_entry() { + // A projection that depends on more than the response body must be + // invalidated when those inputs change, even though the URL is + // identical and the server would answer 304. + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/runs")) + .respond_with( + ResponseTemplate::new(200) + .insert_header("etag", "\"same\"") + .set_body_json(json!({"n": 1})), + ) + .mount(&server) + .await; + + let client = client_for(&server); + let (first, _): (String, _) = client + .get_cached_as("/runs#v1", "/runs", |v: serde_json::Value| { + format!("v1:{}", v["n"]) + }) + .await + .expect("first"); + assert_eq!(first, "v1:1"); + + // Same URL, different key: must re-project rather than replay. + let (second, outcome): (String, _) = client + .get_cached_as("/runs#v2", "/runs", |v: serde_json::Value| { + format!("v2:{}", v["n"]) + }) + .await + .expect("second"); + assert_eq!( + second, "v2:1", + "a new cache key must not replay the old projection" + ); + assert_eq!(outcome, CacheOutcome::Modified); + } + + #[tokio::test] + async fn custom_cache_key_is_reused_on_repeat() { + // Regression guard: entries were read by cache key but written by URL, + // so every `get_cached_as` caller refetched and reprojected. That + // silently disabled ETag revalidation for the runs endpoint, the + // single largest consumer of the rate-limit budget. + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/runs")) + .respond_with( + ResponseTemplate::new(200) + .insert_header("etag", "\"e1\"") + .set_body_json(json!({"n": 7})), + ) + .up_to_n_times(1) + .mount(&server) + .await; + + let client = client_for(&server); + let (first, outcome): (i64, _) = client + .get_cached_as("/runs#fp=abc", "/runs", |v: serde_json::Value| { + v["n"].as_i64().unwrap_or_default() + }) + .await + .expect("first"); + assert_eq!(first, 7); + assert_eq!(outcome, CacheOutcome::Modified); + + // The same key must now revalidate and be answered from cache. + Mock::given(method("GET")) + .and(path("/runs")) + .and(header("if-none-match", "\"e1\"")) + .respond_with(ResponseTemplate::new(304)) + .expect(1) + .mount(&server) + .await; + + let (second, outcome): (i64, _) = client + .get_cached_as("/runs#fp=abc", "/runs", |v: serde_json::Value| { + v["n"].as_i64().unwrap_or_default() + }) + .await + .expect("second"); + assert_eq!(second, 7, "cached projection must be replayed"); + assert_eq!( + outcome, + CacheOutcome::NotModified, + "a repeated custom key must revalidate rather than refetch" + ); + } + #[tokio::test] async fn records_rate_limit_headers() { let server = MockServer::start().await; diff --git a/github-ci-exporter/src/github/rest.rs b/github-ci-exporter/src/github/rest.rs index 2d50687..4a3f933 100644 --- a/github-ci-exporter/src/github/rest.rs +++ b/github-ci-exporter/src/github/rest.rs @@ -23,6 +23,15 @@ use crate::model::{Repo, RunConclusion}; /// the most recent run of every workflow for these repositories. const RUNS_PER_PAGE: usize = 100; +/// Age beyond which a run no longer describes the current code. +/// +/// Some workflows only fire on `pull_request`, so their newest branch-state +/// run can be many months old while the workflow itself is healthy. Reporting +/// that ancient conclusion produces an alert nobody can clear without an +/// artificial push. Past this age the run is marked stale and stops +/// contributing a conclusion. +pub const STALE_RUN_AGE: chrono::TimeDelta = chrono::TimeDelta::days(90); + #[derive(Debug, Deserialize)] struct WorkflowsResponse { workflows: Vec, @@ -35,6 +44,40 @@ struct WorkflowEntry { state: String, } +/// Whether GitHub will currently run a workflow. +/// +/// The distinction between the two disabled states is the point: GitHub +/// automatically disables scheduled workflows in a repository with no activity +/// for 60 days, which silently stops CI. That is a fault worth alerting on, +/// whereas a manually disabled workflow is a deliberate choice. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum WorkflowState { + Active, + /// Auto-disabled by GitHub after 60 days of repository inactivity. + DisabledInactivity, + /// Switched off by a human, or disabled because the repo is a fork. + DisabledManually, +} + +impl WorkflowState { + fn from_api(state: &str) -> Self { + match state { + "active" => Self::Active, + "disabled_inactivity" => Self::DisabledInactivity, + _ => Self::DisabledManually, + } + } + + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::Active => "active", + Self::DisabledInactivity => "disabled_inactivity", + Self::DisabledManually => "disabled_manually", + } + } +} + /// A workflow definition that exists in the repository. /// /// Serialisable because this is what gets stored in the `ETag` cache, rather @@ -43,9 +86,15 @@ struct WorkflowEntry { pub struct Workflow { pub name: String, pub path: String, + pub state: WorkflowState, } -/// Lists active workflows defined by files in `.github/workflows`. +/// Lists workflows defined by files in `.github/workflows`. +/// +/// Disabled workflows are **included**, with their state, because a workflow +/// auto-disabled for inactivity still has meaningful run history and its +/// disablement is itself worth reporting. Filtering them out here caused a +/// failing `update-flakes` to disappear from the metrics entirely. /// /// GitHub also reports "dynamic" workflows (Dependabot updates, Copilot /// reviewers) that have no file in the repository. Those are excluded: they @@ -64,9 +113,10 @@ pub async fn list_workflows(client: &Client, repo: &Repo) -> Result>() @@ -83,7 +133,9 @@ struct RunsResponse { #[derive(Debug, Deserialize)] struct RunEntry { - name: Option, + // The display name is deliberately not read: it is unreliable (older runs + // report the file path instead) and the authoritative name comes from the + // live workflow list, keyed by `path`. #[serde(default)] path: Option, status: String, @@ -103,6 +155,14 @@ pub struct LatestRun { pub html_url: String, } +impl LatestRun { + /// Whether this run is too old to describe the current code. + #[must_use] + pub fn is_stale(&self, now: DateTime) -> bool { + now - self.created_at > STALE_RUN_AGE + } +} + /// Most recent run per workflow, plus the most recent *successful* run. /// /// This reduced form is what the `ETag` cache stores. The raw runs listing is @@ -118,47 +178,125 @@ pub struct RepoRuns { /// Fetches recent runs on the default branch and reduces them to the latest /// run per workflow. /// +/// `live` is the current workflow set from [`list_workflows`]; runs belonging +/// to workflows absent from it are discarded as orphaned history. +/// /// # Errors /// Returns [`ClientError`] if the request fails. -pub async fn fetch_runs(client: &Client, repo: &Repo) -> Result { +pub async fn fetch_runs( + client: &Client, + repo: &Repo, + live: &[Workflow], +) -> Result { let path = format!( "/repos/{}/{}/actions/runs?per_page={RUNS_PER_PAGE}&branch={}", repo.owner, repo.name, repo.default_branch ); + // The cached value is a *reduction*, and the reduction depends on `live` + // as well as on the response. Deleting or renaming a workflow does not + // change the runs listing, so the request would answer 304 and replay a + // reduction computed against the previous workflow set -- reviving the + // orphaned-run bug this reduction exists to prevent. Folding a fingerprint + // of `live` into the cache key invalidates the entry whenever the workflow + // set changes, at the cost of one uncached fetch after such a change. + let cache_key = format!("{path}#wf={}", fingerprint_workflows(live)); let (runs, outcome) = client - .get_cached(&path, |response: RunsResponse| { - reduce_runs(response.workflow_runs) + .get_cached_as(&cache_key, &path, |response: RunsResponse| { + reduce_runs(response.workflow_runs, live) }) .await?; debug!(repo = %repo, ?outcome, workflows = runs.latest.len(), "fetched runs"); Ok(runs) } -/// Reduces a run list to the newest run per workflow. +/// A stable fingerprint of the workflow set's identities and display names. +/// +/// Order-independent so an API reordering cannot spuriously invalidate the +/// cache, and it covers names as well as paths because a rename must change +/// the reduction's output labels. +fn fingerprint_workflows(live: &[Workflow]) -> u64 { + use std::hash::{Hash as _, Hasher as _}; + + let mut pairs: Vec<(&str, &str)> = live + .iter() + .map(|w| (w.path.as_str(), w.name.as_str())) + .collect(); + pairs.sort_unstable(); + + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + for (path, name) in pairs { + path.hash(&mut hasher); + name.hash(&mut hasher); + } + hasher.finish() +} + +/// Whether a run's trigger reflects the state of the default branch. +/// +/// Only these events describe "is the branch healthy right now": +/// +/// * `push` -- code landed on the branch. +/// * `schedule` / `workflow_dispatch` -- ran against the branch as it stands. +/// +/// `pull_request` runs are excluded even when the API's `branch=` filter +/// matches them, because that filter matches the PR's *head* branch. A merged +/// PR's last pre-merge failure would otherwise be reported as the branch's +/// current CI state forever, which was observed on both +/// `fredsystems/pre-commit-checks` and `sdr-enthusiasts/docker-planefence`. +/// Post-merge health is covered by the `push` run that merging produces. +/// +/// `dynamic` is Dependabot's generated security-update runs, each with a +/// unique name; keeping them would make cardinality unbounded. +fn is_branch_state_event(event: &str) -> bool { + matches!(event, "push" | "schedule" | "workflow_dispatch") +} + +/// Reduces a run list to the newest run per workflow, keeping only workflows +/// that still exist in the repository. +/// +/// Two classes of stale data must be discarded, both observed in the wild: +/// +/// * **Orphaned runs.** Run history outlives the workflow file. A deleted +/// `Update pre-commit hooks` workflow kept reporting its final failure +/// indefinitely -- 18 of 38 observed failures were this. Runs are therefore +/// intersected with the live workflow list. +/// * **Path-named runs.** Runs created before a workflow gained a `name:` +/// field report the file path where the name should be, so `CI` and +/// `.github/workflows/ci.yml` appear as two distinct workflows. Keying on +/// `path` and resolving the display name from the live workflow list +/// collapses them. /// -/// Dependabot's security-update runs are excluded: each one has a unique -/// generated name (`npm_and_yarn in /. for ...`), so keeping them would -/// produce unbounded metric cardinality. -fn reduce_runs(runs: Vec) -> RepoRuns { +/// Dependabot's security-update runs are excluded: each has a unique generated +/// name, which would make cardinality unbounded. +/// +/// Note that while *lookup* is by path, the output is keyed by display name, +/// because that is the label operators recognise on a dashboard. Two workflow +/// files declaring the same `name:` therefore collapse into one series. That +/// is accepted: it does not occur in the monitored organisations, and keying +/// metrics by file path would make every dashboard and alert harder to read. +fn reduce_runs(runs: Vec, live: &[Workflow]) -> RepoRuns { + // Workflow identity is the file path, not the display name: a run created + // before the workflow gained a `name:` reports the path in the name field, + // and a renamed workflow would otherwise split into two series. + let live_by_path: HashMap<&str, &Workflow> = + live.iter().map(|w| (w.path.as_str(), w)).collect(); + let mut latest: HashMap = HashMap::new(); let mut last_success: HashMap> = HashMap::new(); for run in runs { - if run.event == "dynamic" { + if !is_branch_state_event(&run.event) { continue; } - // A run whose workflow file was deleted still appears in history; - // without a path there is no stable identity to key on. - let Some(name) = run.name.filter(|n| !n.is_empty()) else { + // Runs of a since-deleted workflow linger in history forever. Only + // workflows still present in the repository are reported. + let Some(path) = run.path.as_deref() else { continue; }; - if run - .path - .as_ref() - .is_some_and(|p| !p.starts_with(".github/workflows")) - { + let Some(workflow) = live_by_path.get(path) else { continue; - } + }; + let name = workflow.name.clone(); let conclusion = RunConclusion::from_api(&run.status, run.conclusion.as_deref()); @@ -305,16 +443,34 @@ fn parse_crons(yaml: &str) -> Vec { mod tests { use super::*; + /// A run of a workflow whose file is `.github/workflows/.yml`. fn run( name: &str, status: &str, conclusion: Option<&str>, event: &str, created: &str, + ) -> RunEntry { + run_at( + name, + &format!(".github/workflows/{name}.yml"), + status, + conclusion, + event, + created, + ) + } + + fn run_at( + name: &str, + path: &str, + status: &str, + conclusion: Option<&str>, + event: &str, + created: &str, ) -> RunEntry { RunEntry { - name: Some(name.to_owned()), - path: Some(".github/workflows/x.yaml".to_owned()), + path: Some(path.to_owned()), status: status.to_owned(), conclusion: conclusion.map(str::to_owned), event: event.to_owned(), @@ -323,6 +479,18 @@ mod tests { } } + /// Declares workflows as currently present in the repository. + fn live(names: &[&str]) -> Vec { + names + .iter() + .map(|n| Workflow { + name: (*n).to_owned(), + path: format!(".github/workflows/{n}.yml"), + state: WorkflowState::Active, + }) + .collect() + } + #[test] fn keeps_only_the_newest_run_per_workflow() { let runs = vec![ @@ -348,7 +516,7 @@ mod tests { "2026-08-05T00:00:00Z", ), ]; - let reduced = reduce_runs(runs); + let reduced = reduce_runs(runs, &live(&["CI", "Deploy"])); assert_eq!(reduced.latest.len(), 2); let ci = reduced @@ -382,7 +550,7 @@ mod tests { "2026-08-01T00:00:00Z", ), ]; - let reduced = reduce_runs(runs); + let reduced = reduce_runs(runs, &live(&["CI", "Deploy"])); assert_eq!(reduced.latest[0].conclusion, RunConclusion::Success); } @@ -404,7 +572,7 @@ mod tests { "2026-08-09T00:00:00Z", ), ]; - let reduced = reduce_runs(runs); + let reduced = reduce_runs(runs, &live(&["CI", "Deploy"])); assert_eq!(reduced.latest[0].conclusion, RunConclusion::Failure); assert_eq!( @@ -414,6 +582,63 @@ mod tests { ); } + #[test] + fn excludes_pull_request_runs() { + // Regression guard: the API's `branch=` filter matches a PR's HEAD + // branch, so PR runs leak through. A merged PR's failing pre-merge run + // was being reported as the default branch's current CI state. + let runs = vec![ + run( + "Lint", + "completed", + Some("failure"), + "pull_request", + "2026-08-03T16:51:03Z", + ), + run( + "Lint", + "completed", + Some("success"), + "push", + "2026-08-01T00:00:00Z", + ), + ]; + let reduced = reduce_runs(runs, &live(&["Lint"])); + + assert_eq!(reduced.latest.len(), 1); + assert_eq!( + reduced.latest[0].conclusion, + RunConclusion::Success, + "a newer pull_request run must not override the branch's push state" + ); + } + + #[test] + fn keeps_schedule_and_dispatch_runs() { + for event in ["push", "schedule", "workflow_dispatch"] { + let runs = vec![run( + "CI", + "completed", + Some("failure"), + event, + "2026-08-09T00:00:00Z", + )]; + let reduced = reduce_runs(runs, &live(&["CI"])); + assert_eq!(reduced.latest.len(), 1, "{event} must be kept"); + } + for event in ["pull_request", "dynamic", "pull_request_target"] { + let runs = vec![run( + "CI", + "completed", + Some("failure"), + event, + "2026-08-09T00:00:00Z", + )]; + let reduced = reduce_runs(runs, &live(&["CI"])); + assert!(reduced.latest.is_empty(), "{event} must be excluded"); + } + } + #[test] fn excludes_dependabot_dynamic_runs() { // These have unique generated names and would explode cardinality. @@ -433,7 +658,7 @@ mod tests { "2026-08-09T00:00:00Z", ), ]; - let reduced = reduce_runs(runs); + let reduced = reduce_runs(runs, &live(&["CI", "Deploy"])); assert_eq!(reduced.latest.len(), 1); assert_eq!(reduced.latest[0].workflow, "CI"); @@ -448,11 +673,266 @@ mod tests { "push", "2026-08-09T00:00:00Z", )]; - let reduced = reduce_runs(runs); + let reduced = reduce_runs(runs, &live(&["CI", "Deploy"])); assert_eq!(reduced.latest[0].conclusion, RunConclusion::Running); assert!(!reduced.latest[0].conclusion.is_failure()); } + #[test] + fn discards_runs_of_deleted_workflows() { + // Regression guard: GitHub keeps run history after a workflow file is + // removed. Observed in the wild as a long-deleted "Update pre-commit + // hooks" reporting a permanent failure -- 18 of 38 failures were this. + let runs = vec![ + run( + "CI", + "completed", + Some("success"), + "push", + "2026-08-09T00:00:00Z", + ), + run( + "Update pre-commit hooks", + "completed", + Some("failure"), + "schedule", + "2025-12-14T00:54:24Z", + ), + ]; + let reduced = reduce_runs(runs, &live(&["CI"])); + + assert_eq!(reduced.latest.len(), 1); + assert_eq!(reduced.latest[0].workflow, "CI"); + assert!( + !reduced.latest.iter().any(|r| r.conclusion.is_failure()), + "a deleted workflow must not report a failure" + ); + } + + #[test] + fn collapses_path_named_runs_onto_the_workflow_name() { + // Runs created before a workflow gained a `name:` report the file path + // in the name field. Keying on path prevents "CI" and + // ".github/workflows/ci.yml" becoming two series for one workflow. + let runs = vec![ + run_at( + ".github/workflows/CI.yml", + ".github/workflows/CI.yml", + "completed", + Some("failure"), + "push", + "2026-07-08T03:36:04Z", + ), + run( + "CI", + "completed", + Some("success"), + "push", + "2026-08-09T00:00:00Z", + ), + ]; + let reduced = reduce_runs(runs, &live(&["CI"])); + + assert_eq!(reduced.latest.len(), 1, "one workflow, one series"); + assert_eq!(reduced.latest[0].workflow, "CI"); + assert_eq!( + reduced.latest[0].conclusion, + RunConclusion::Success, + "newest run wins after collapsing" + ); + } + + #[test] + fn keeps_workflows_regardless_of_yml_or_yaml_extension() { + // Both spellings are in active use across the fleet. + let workflows = vec![ + Workflow { + name: "Deploy".to_owned(), + path: ".github/workflows/deploy.yml".to_owned(), + state: WorkflowState::Active, + }, + Workflow { + name: "Lint".to_owned(), + path: ".github/workflows/lint.yaml".to_owned(), + state: WorkflowState::Active, + }, + ]; + let runs = vec![ + run_at( + "Deploy", + ".github/workflows/deploy.yml", + "completed", + Some("success"), + "push", + "2026-08-09T00:00:00Z", + ), + run_at( + "Lint", + ".github/workflows/lint.yaml", + "completed", + Some("failure"), + "push", + "2026-08-09T00:00:00Z", + ), + ]; + let reduced = reduce_runs(runs, &workflows); + + assert_eq!(reduced.latest.len(), 2, "both extensions must be kept"); + } + + #[test] + fn disabled_workflows_still_report_their_runs() { + // Regression guard: filtering to state=="active" made a failing + // `update-flakes` vanish from the metrics after GitHub auto-disabled + // it for repository inactivity -- the exact condition worth alerting + // on was the one being hidden. + let workflows = vec![Workflow { + name: "update-flakes".to_owned(), + path: ".github/workflows/update-flakes.yaml".to_owned(), + state: WorkflowState::DisabledInactivity, + }]; + let runs = vec![run_at( + "update-flakes", + ".github/workflows/update-flakes.yaml", + "completed", + Some("failure"), + "schedule", + "2026-08-01T00:47:26Z", + )]; + let reduced = reduce_runs(runs, &workflows); + + assert_eq!(reduced.latest.len(), 1); + assert!(reduced.latest[0].conclusion.is_failure()); + } + + #[test] + fn workflow_state_maps_the_two_disabled_kinds_apart() { + assert_eq!(WorkflowState::from_api("active"), WorkflowState::Active); + assert_eq!( + WorkflowState::from_api("disabled_inactivity"), + WorkflowState::DisabledInactivity + ); + assert_eq!( + WorkflowState::from_api("disabled_manually"), + WorkflowState::DisabledManually + ); + // Unknown future states are treated as a deliberate disable rather + // than as an inactivity fault, so they cannot cause a false page. + assert_eq!( + WorkflowState::from_api("something_new"), + WorkflowState::DisabledManually + ); + } + + #[test] + fn ancient_runs_are_marked_stale() { + // Regression guard: `pre-commit-checks` Lint last ran on a push to + // main in Dec 2025 and failed; every run since has been a passing + // pull_request. Reporting that failure forever is not actionable. + let run = run( + "Lint", + "completed", + Some("failure"), + "push", + "2025-12-13T13:42:49Z", + ); + let reduced = reduce_runs(vec![run], &live(&["Lint"])); + let latest = reduced.latest.first().expect("run retained"); + + let now: DateTime = "2026-08-10T00:00:00Z".parse().expect("timestamp"); + assert!(latest.is_stale(now), "an 8-month-old run must be stale"); + } + + #[test] + fn recent_runs_are_not_stale() { + let run = run( + "CI", + "completed", + Some("failure"), + "push", + "2026-08-01T00:00:00Z", + ); + let reduced = reduce_runs(vec![run], &live(&["CI"])); + let latest = reduced.latest.first().expect("run retained"); + + let now: DateTime = "2026-08-10T00:00:00Z".parse().expect("timestamp"); + assert!(!latest.is_stale(now), "a 9-day-old failure is actionable"); + } + + #[test] + fn staleness_boundary_is_exclusive() { + let run = run( + "CI", + "completed", + Some("failure"), + "push", + "2026-05-12T00:00:00Z", + ); + let reduced = reduce_runs(vec![run], &live(&["CI"])); + let latest = reduced.latest.first().expect("run retained"); + + // Exactly 90 days is not yet stale; a second past it is. + let at_horizon = latest.created_at + STALE_RUN_AGE; + assert!(!latest.is_stale(at_horizon)); + assert!(latest.is_stale(at_horizon + chrono::TimeDelta::seconds(1))); + } + + #[test] + fn fingerprint_changes_when_a_workflow_is_deleted() { + // Regression guard: deleting a workflow does not change the runs + // listing, so the request answers 304. Without the fingerprint in the + // cache key the stale reduction is replayed and the deleted + // workflow's runs come back. + let before = live(&["CI", "Deploy"]); + let after = live(&["CI"]); + assert_ne!( + fingerprint_workflows(&before), + fingerprint_workflows(&after) + ); + } + + #[test] + fn fingerprint_changes_when_a_workflow_is_renamed() { + let before = vec![Workflow { + name: "CI".to_owned(), + path: ".github/workflows/ci.yml".to_owned(), + state: WorkflowState::Active, + }]; + let after = vec![Workflow { + name: "Build".to_owned(), + path: ".github/workflows/ci.yml".to_owned(), + state: WorkflowState::Active, + }]; + assert_ne!( + fingerprint_workflows(&before), + fingerprint_workflows(&after), + "a rename changes the output labels and must invalidate the cache" + ); + } + + #[test] + fn fingerprint_is_order_independent() { + // An API reordering must not spuriously invalidate the cache and + // force a full uncached sweep. + let a = live(&["CI", "Deploy", "Lint"]); + let mut b = a.clone(); + b.reverse(); + assert_eq!(fingerprint_workflows(&a), fingerprint_workflows(&b)); + } + + #[test] + fn run_without_a_path_is_discarded() { + let mut orphan = run( + "CI", + "completed", + Some("failure"), + "push", + "2026-08-09T00:00:00Z", + ); + orphan.path = None; + assert!(reduce_runs(vec![orphan], &live(&["CI"])).latest.is_empty()); + } + #[test] fn parses_cron_from_workflow_yaml() { let yaml = r#" diff --git a/github-ci-exporter/src/metrics.rs b/github-ci-exporter/src/metrics.rs index 35d27db..d1ff43a 100644 --- a/github-ci-exporter/src/metrics.rs +++ b/github-ci-exporter/src/metrics.rs @@ -46,6 +46,14 @@ pub struct WorkflowLabels { pub workflow: String, } +#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)] +pub struct WorkflowEnabledLabels { + pub org: String, + pub repo: String, + pub workflow: String, + pub state: String, +} + #[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)] pub struct WorkflowStateLabels { pub org: String, @@ -76,6 +84,8 @@ pub struct Metrics { pub workflow_run_timestamp: Family, pub workflow_last_success_timestamp: Family, pub workflow_expected_interval: Family, + pub workflow_enabled: Family, + pub workflow_run_stale: Family, pub repo_monitored: Family, pub repos_skipped: Family, pub rate_limit_remaining: Family, @@ -155,6 +165,20 @@ impl Metrics { workflow_expected_interval.clone(), ); + let workflow_enabled = Family::::default(); + registry.register( + "workflow_enabled", + "1 if GitHub will run this workflow; state distinguishes an inactivity auto-disable from a manual one", + workflow_enabled.clone(), + ); + + let workflow_run_stale = Family::::default(); + registry.register( + "workflow_run_stale", + "1 if the latest branch-state run is too old to describe current code", + workflow_run_stale.clone(), + ); + let repo_monitored = Family::::default(); registry.register( "repo_monitored", @@ -263,6 +287,8 @@ impl Metrics { workflow_run_timestamp, workflow_last_success_timestamp, workflow_expected_interval, + workflow_enabled, + workflow_run_stale, repo_monitored, repos_skipped, rate_limit_remaining, @@ -295,6 +321,8 @@ impl Metrics { self.workflow_run_timestamp.clear(); self.workflow_last_success_timestamp.clear(); self.workflow_expected_interval.clear(); + self.workflow_enabled.clear(); + self.workflow_run_stale.clear(); self.repo_monitored.clear(); self.repos_skipped.clear(); }