diff --git a/CHANGELOG.md b/CHANGELOG.md index 13dd5f9949..43fb7d04a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,8 +23,23 @@ and this project adheres to [#5188](https://github.com/OpenFn/lightning/pull/5188) - A "Steps with failures" card on the workflow health page, ranking the jobs the window's failures landed on, heaviest first. - -### Changed +- A band of a bar in the runs volume chart on the workflow health page now opens + the work orders it counted, filtered to that bar's slot and the run states the + band stacked. [#5191](https://github.com/OpenFn/lightning/pull/5191) +- History can now filter on when a run was created and on the state a run + finished in, each as its own chip, so a failure later retried to success is + still reachable from the band that counted it. + [#5191](https://github.com/OpenFn/lightning/pull/5191) + +### Changed + +- The runs volume chart on the workflow health page is now bucketed on your own + timezone rather than UTC, so a daily bar covers your day, the week view's bars + split at your noon, a day the clocks change is still one bar, and the card + names the clock it was drawn on. A browser that sends no timezone, or says it + does not know one, still gets UTC; one that sends a zone the tz database does + not know now gets an error. + [#5191](https://github.com/OpenFn/lightning/pull/5191) ### Fixed diff --git a/assets/js/health/WorkflowHealth.tsx b/assets/js/health/WorkflowHealth.tsx index 6bed9cd67e..2fa9f7d1cb 100644 --- a/assets/js/health/WorkflowHealth.tsx +++ b/assets/js/health/WorkflowHealth.tsx @@ -95,13 +95,17 @@ export const WorkflowHealth = ({ - {({ buckets, window }) => ( + {({ buckets, window, timezone, bucket_hours }) => ( )} diff --git a/assets/js/health/charts/VolumeBars.tsx b/assets/js/health/charts/VolumeBars.tsx index 12eea2aff5..495e3604a6 100644 --- a/assets/js/health/charts/VolumeBars.tsx +++ b/assets/js/health/charts/VolumeBars.tsx @@ -8,6 +8,7 @@ import { YAxis, } from 'recharts'; +import { historyUrl } from '../historyUrl'; import type { FailureState } from '../types'; import { FAILURE_STATES } from '../types'; @@ -20,8 +21,9 @@ import { CANCELLED, FAILED, SUCCESS } from './OutcomesDonut'; * * Counts runs where the donuts beside it count work orders, so the two differ * on purpose and the card carries no total. Buckets arrive already counted and - * zero-filled from `Stats.runs/2` — one row per bar, one key per run state, - * which is the shape Recharts takes as `data`. + * zero-filled from `Stats.runs/3` — one row per bar, one key per run state, + * which is the shape Recharts takes as `data` — on a grid cut to the reader's + * calendar, so every label here follows `timezone` rather than the browser. */ // A run has no `rejected` state — a work order rejected on arrival never @@ -29,7 +31,7 @@ import { CANCELLED, FAILED, SUCCESS } from './OutcomesDonut'; // both. type RunFailureState = Exclude; -const RUN_FAILURE_STATES = FAILURE_STATES.filter( +export const RUN_FAILURE_STATES = FAILURE_STATES.filter( (state): state is RunFailureState => state !== 'rejected' ); @@ -47,23 +49,47 @@ export interface RunBucket { export interface RunVolume { window: { from: string; to: string }; + /** The clock the server cut the grid on. Every label follows it. */ + timezone: string; + /** Bar width, in wall-clock hours on that clock: 2, 12 or 24. */ + bucket_hours: number; buckets: RunBucket[]; } // Top to bottom, as the legend and the spoken totals read. The bars draw from // the reverse of it, since Recharts puts the first `Bar` on the axis. +// +// `states` is what the band actually stacked, and is what its history link +// filters on — the red one folds five, and it is the only place those five can +// be named. const SERIES = [ - { key: 'success', label: 'Success', color: SUCCESS }, - { key: 'cancelled', label: 'Cancelled', color: CANCELLED }, - { key: 'failed', label: 'Failed', color: FAILED }, + { key: 'success', label: 'Success', color: SUCCESS, states: ['success'] }, + { + key: 'cancelled', + label: 'Cancelled', + color: CANCELLED, + states: ['cancelled'], + }, + { key: 'failed', label: 'Failed', color: FAILED, states: RUN_FAILURE_STATES }, ] as const; interface VolumeBarsProps { buckets: RunBucket[]; + timezone: string; + hours: number; emptyMessage: string; + projectId: string; + workflowId: string; } -export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { +export const VolumeBars = ({ + buckets, + timezone, + hours, + emptyMessage, + projectId, + workflowId, +}: VolumeBarsProps) => { const rows = buckets.map(bucket => ({ at: bucket.at, success: bucket.success, @@ -84,8 +110,6 @@ export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { return

{emptyMessage}

; } - const hours = bucketHours(buckets); - return ( <> {/* The donut's `FRAME` is a fixed box and gains nothing from extra @@ -104,7 +128,7 @@ export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { to thin the axis itself Recharts drops named bars too. */} tickLabel(at as string, hours)} + tickFormatter={at => tickLabel(at as string, hours, timezone)} tickLine={false} axisLine={false} interval={hours >= 12 && hours < 24 ? 0 : 'preserveEnd'} @@ -132,22 +156,42 @@ export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { content={ rangeLabel(at, hours)} + formatLabel={at => rangeLabel(at, hours, timezone)} /> } /> {/* Reversed, so failures land on the axis and can be read against - a fixed baseline day to day rather than judged by thickness. */} - {[...totals].reverse().map(({ key, label: name, color }) => ( - - ))} + a fixed baseline day to day rather than judged by thickness. + + Each band links to its own states rather than the chart + carrying one link for the whole column: the red band is the + answer most readers came for, and a column-wide link would + make them filter the failures out again on arrival. Clicking + is a mouse affordance only — the frame is `aria-hidden`, and + thirty bars times three bands is not a link list. */} + {[...totals] + .reverse() + .map(({ key, label: name, color, states }) => ( + { + const href = bucketUrl( + projectId, + workflowId, + buckets, + index, + states + ); + if (href) window.open(href, '_blank'); + }} + /> + ))} @@ -183,69 +227,103 @@ export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { ); }; -// Read off the data rather than passed in, so nothing naming the bucket width -// can disagree with the bars under it. A window too short to measure reads as -// daily, which only ever costs a coarser label. -const bucketHours = (buckets: RunBucket[]) => { - const [first, second] = buckets; - - return first && second - ? (Date.parse(second.at) - Date.parse(first.at)) / 3_600_000 - : 24; -}; +/** + * The card's meta line: the bar width this range draws, and the clock it is + * drawn on. + * + * Names the timezone because the grid is cut on the reader's own clock rather + * than UTC, and a bar labelled `Mar 3` is only unambiguous once the reader + * knows whose `Mar 3` it is. + */ +export const bucketMeta = ({ timezone, bucket_hours: hours }: RunVolume) => + `${hours >= 24 ? 'daily' : `${hours}-hour`} buckets · ${timezone}`; /** - * The card's meta line, measured off the same buckets the chart draws. + * The history link for one band of one bar: the work orders with a run + * created inside the bar's slot that settled in one of the band's states. * - * Names the zone as well as the width: the axis and the tooltip are in UTC, so - * a reader whose own day starts hours either side of it can tell which day a - * bar is counting. + * Half-open, and the slot's end is the *next* bar's start rather than its own + * plus a width: no timezone arithmetic happens here, and a bar spanning a + * clock change is 11 or 13 real hours wide. The newest bar has no upper bound + * — it is still filling. */ -export const bucketMeta = (buckets: RunBucket[]) => { - const hours = bucketHours(buckets); +export const bucketUrl = ( + projectId: string, + workflowId: string, + buckets: RunBucket[], + index: number, + states: readonly string[] +) => { + const bucket = buckets[index]; + + if (!bucket) return null; - return `${hours >= 24 ? 'daily' : `${hours}-hour`} buckets · UTC`; + return historyUrl(projectId, workflowId, { + run_date_after: bucket.at, + run_date_before: buckets[index + 1]?.at, + run_status: states, + }); }; const TICK_FILL = '#6b7280'; -// Rendered in UTC because that is where the buckets are: the server lays the -// grid on the raw epoch, so a day starts at UTC midnight — 03:00 in Nairobi, -// 05:30 in Delhi. The tooltip says UTC for the same reason. -const dayLabel = (date: Date) => +// In the timezone the server cut the grid on, not the browser's own: a bar +// starts at a local whole hour there, so a Delhi bar opening at UTC 18:30 +// labels as 00:00 and no label ever needs minutes. +const dayLabel = (date: Date, timezone: string) => date.toLocaleDateString(undefined, { month: 'short', day: 'numeric', - timeZone: 'UTC', + timeZone: timezone, }); -const hourLabel = (date: Date) => `${date.getUTCHours()}:00`; +// `hourCycle` rather than `hour12: false` because it is the explicit way to +// ask for 00-23 and does not depend on the locale's own preference. Every +// boundary is a local whole hour, so no label ever needs minutes. +const localHour = (date: Date, timezone: string) => + Number( + date.toLocaleString('en-GB', { + hour: '2-digit', + hourCycle: 'h23', + timeZone: timezone, + }) + ); + +const hourLabel = (hour: number) => `${String(hour).padStart(2, '0')}:00`; -export const tickLabel = (at: string, hours: number) => { +export const tickLabel = (at: string, hours: number, timezone: string) => { const date = new Date(at); - if (hours >= 24) return dayLabel(date); + if (hours >= 24) return dayLabel(date, timezone); // Two bars to a day at this width, so the date goes on the morning bar, // where the day starts, and the afternoon is left blank. A window opening // mid-day leaves that first bar unlabelled, which is honest — its morning // is outside the window — and keeps every date two bars from the last. - if (hours >= 12) return date.getUTCHours() < 12 ? dayLabel(date) : ''; + if (hours >= 12) { + return localHour(date, timezone) < 12 ? dayLabel(date, timezone) : ''; + } - return hourLabel(date); + return hourLabel(localHour(date, timezone)); }; // The axis is thinned and its labels name only where a bucket starts, so the // tooltip names the whole slot rather than leaving the reader to add the width -// to the start. -export const rangeLabel = (at: string, hours: number) => { +// to the start. No timezone suffix: the card's meta line names it once, and +// repeating it on every hover is noise. +export const rangeLabel = (at: string, hours: number, timezone: string) => { const start = new Date(at); - // A daily bar is a whole UTC day, and `00:00 – 00:00` invites the question - // of whether the closing midnight is inside the bar or the next one. - if (hours >= 24) return `${dayLabel(start)} UTC`; + // A daily bar is a whole local day, and `00:00 – 00:00` invites the question + // of whether the closing midnight is inside the bar or the next one. It is + // also the branch that keeps a 25-hour clock-change day honest, by printing + // no end at all. + if (hours >= 24) return dayLabel(start, timezone); - const end = new Date(start.getTime() + hours * 3_600_000); + // The end hour is the start's plus the width on the wall clock, not + // `start + hours` in real time: a bar spanning a clock change is 11 or 13 + // real hours, and only the wall clock closes it where the next bar opens. + const from = localHour(start, timezone); - return `${dayLabel(start)}, ${hourLabel(start)} – ${hourLabel(end)} UTC`; + return `${dayLabel(start, timezone)}, ${hourLabel(from)} – ${hourLabel((from + hours) % 24)}`; }; diff --git a/assets/js/health/historyUrl.ts b/assets/js/health/historyUrl.ts index 47fce45e83..8f9fd589b0 100644 --- a/assets/js/health/historyUrl.ts +++ b/assets/js/health/historyUrl.ts @@ -13,7 +13,7 @@ import type { WorkOrderStateCounts } from './types'; export const historyUrl = ( projectId: string, workflowId: string, - filters: Record + filters: Record ) => { const params = new URLSearchParams({ 'filters[workflow_id]': workflowId, @@ -21,9 +21,15 @@ export const historyUrl = ( }); // Absent parts of a filter are skipped, so a caller can hand over an - // optional field without guarding it. + // optional field without guarding it. A list goes over as `key[]` repeated, + // which is what Plug decodes back into a list for a `{:array, _}` field — + // one joined string would fail to cast. for (const [key, value] of Object.entries(filters)) { - if (value) params.set(`filters[${key}]`, value); + if (Array.isArray(value)) { + for (const item of value) params.append(`filters[${key}][]`, item); + } else if (typeof value === 'string' && value) { + params.set(`filters[${key}]`, value); + } } return `/projects/${projectId}/history?${params.toString()}`; diff --git a/assets/js/health/useHealthQuery.ts b/assets/js/health/useHealthQuery.ts index 1709cbc207..c2e25effaf 100644 --- a/assets/js/health/useHealthQuery.ts +++ b/assets/js/health/useHealthQuery.ts @@ -52,7 +52,17 @@ export function useHealthQuery(url: string): Query { setInFlight(true); - fetch(url, { credentials: 'same-origin', signal: controller.signal }) + // Read inside the effect, so a reader whose machine changes timezone + // mid-session picks it up on the next poll. Sent to all three endpoints; + // only `runs` reads it, which is cheaper than threading a per-endpoint + // flag through a hook whose job is to be indifferent to the endpoint. + fetch(url, { + credentials: 'same-origin', + signal: controller.signal, + headers: { + 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone, + }, + }) .then(response => { if (!response.ok) throw new Error('Could not load workflow stats'); diff --git a/assets/test/health/WorkflowHealth.test.tsx b/assets/test/health/WorkflowHealth.test.tsx index dd49065c7a..94a3de4ee8 100644 --- a/assets/test/health/WorkflowHealth.test.tsx +++ b/assets/test/health/WorkflowHealth.test.tsx @@ -34,13 +34,15 @@ const errorSignatures = { ], }; -// Two buckets is the least the chart can measure its own width from. What the -// bars look like is `VolumeBars`'s own test; the page only hands them through. +// Bars as the server cuts them, on the reader's clock. What they look like +// drawn is `VolumeBars`'s own test; the page only hands them through. const runVolume = { window: outcomes.window, + timezone: 'Africa/Nairobi', + bucket_hours: 2, buckets: [ bucket('2026-08-30T00:00:00Z', { success: 40, failed: 3 }), - bucket('2026-08-31T00:00:00Z', { success: 60, failed: 1 }), + bucket('2026-08-30T01:00:00Z', { success: 60, failed: 1 }), ], }; @@ -127,6 +129,23 @@ describe('WorkflowHealth', () => { ); }); + // Nothing server-side records a reader's timezone, so the request has to + // carry it or the chart's buckets are cut on UTC. + test("tells the server the reader's timezone", async () => { + const { fetchMock } = mount(both); + + await screen.findAllByText('Success'); + + expect(fetchMock).toHaveBeenCalledWith( + '/api/projects/proj-1/workflows/wf-1/health/runs?days=30', + expect.objectContaining({ + headers: { + 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone, + }, + }) + ); + }); + test('polls every 30 seconds while the tab is visible, and stops while hidden', async () => { // Mutable so the polled request can answer differently. const responses: Record = { ...both }; diff --git a/assets/test/health/charts/VolumeBars.test.tsx b/assets/test/health/charts/VolumeBars.test.tsx index 5d447d9d0e..782cf8dcf5 100644 --- a/assets/test/health/charts/VolumeBars.test.tsx +++ b/assets/test/health/charts/VolumeBars.test.tsx @@ -3,6 +3,8 @@ import { describe, expect, test } from 'vitest'; import { bucketMeta, + bucketUrl, + RUN_FAILURE_STATES, rangeLabel, tickLabel, VolumeBars, @@ -10,6 +12,8 @@ import { import { bucket } from './counts'; +const links = { projectId: 'proj-1', workflowId: 'wf-1' }; + describe('VolumeBars', () => { test('folds every run failure state into one Failed total', () => { render( @@ -20,7 +24,7 @@ describe('VolumeBars', () => { failed: 3, crashed: 2, }), - bucket('2026-09-09T00:00:00Z', { + bucket('2026-09-08T01:00:00Z', { success: 60, killed: 1, exception: 1, @@ -28,7 +32,10 @@ describe('VolumeBars', () => { cancelled: 4, }), ]} + timezone="Etc/UTC" + hours={2} emptyMessage="No runs" + {...links} /> ); @@ -49,9 +56,12 @@ describe('VolumeBars', () => { ); @@ -67,9 +77,12 @@ describe('VolumeBars', () => { ); @@ -78,54 +91,130 @@ describe('VolumeBars', () => { }); }); -// Measured off the gap between the first two buckets, so the meta line follows -// the bars rather than the range the reader picked. +// Read off the payload, since the server cuts the grid and names its width. describe('bucketMeta', () => { - test('names the bucket width the server actually sent', () => { - const hourly = (hours: number) => [ - bucket('2026-09-09T00:00:00Z'), - bucket( - new Date( - Date.parse('2026-09-09T00:00:00Z') + hours * 3_600_000 - ).toISOString() - ), - ]; - - expect(bucketMeta(hourly(2))).toBe('2-hour buckets · UTC'); - expect(bucketMeta(hourly(12))).toBe('12-hour buckets · UTC'); - expect(bucketMeta(hourly(24))).toBe('daily buckets · UTC'); + const volume = (bucket_hours: number) => ({ + window: { from: '2026-09-08T21:00:00Z', to: '2026-09-09T21:00:00Z' }, + timezone: 'Africa/Nairobi', + bucket_hours, + buckets: [], }); - // One bucket has no width to disagree with, so a coarser label is the worst - // this can cost. - test('falls back to daily when there is nothing to measure', () => { - expect(bucketMeta([bucket('2026-09-09T00:00:00Z')])).toBe( - 'daily buckets · UTC' - ); + test('names the bar width and the clock it is cut on', () => { + expect(bucketMeta(volume(2))).toBe('2-hour buckets · Africa/Nairobi'); + expect(bucketMeta(volume(12))).toBe('12-hour buckets · Africa/Nairobi'); + expect(bucketMeta(volume(24))).toBe('daily buckets · Africa/Nairobi'); }); }); -// The axis names where a bucket starts; the tooltip names the whole slot. The -// day part is left to the locale, so only the clock is asserted literally. +// The axis names where a bar starts; the tooltip names the whole slot. The day +// part is left to the locale, so only the clock is asserted literally. describe('tickLabel and rangeLabel', () => { const at = '2026-09-09T14:00:00Z'; - const day = tickLabel(at, 24); + const day = tickLabel(at, 24, 'Etc/UTC'); - test('clocks the narrow buckets and dates the wide ones', () => { - expect(tickLabel(at, 2)).toBe('14:00'); + test('clocks the narrow bars and dates the wide ones', () => { + expect(tickLabel(at, 2, 'Etc/UTC')).toBe('14:00'); expect(day).not.toBe(''); }); // The date names where a day starts, so it goes on the morning bar and the // afternoon is left blank — a date under the afternoon bar would put the // day's start half a day late. - test('dates a half-day bucket on its morning bar only', () => { - expect(tickLabel('2026-09-09T00:00:00Z', 12)).toBe(day); - expect(tickLabel(at, 12)).toBe(''); + test('dates a half-day bar on its morning bar only', () => { + expect(tickLabel('2026-09-09T00:00:00Z', 12, 'Etc/UTC')).toBe(day); + expect(tickLabel(at, 12, 'Etc/UTC')).toBe(''); + }); + + test('gives the tooltip the whole slot, with no timezone suffix', () => { + expect(rangeLabel(at, 24, 'Etc/UTC')).toBe(day); + expect(rangeLabel(at, 2, 'Etc/UTC')).toBe(`${day}, 14:00 – 16:00`); + }); + + // Closed on the wall clock, not on `start + hours`: the last bar of a local + // day ends at midnight, and a bar spanning a clock change is 11 or 13 real + // hours but still 12 on the clock its labels are drawn on. + test('closes the last bar of a day at midnight', () => { + const lastBar = '2026-10-25T12:00:00Z'; + + expect(rangeLabel('2026-09-09T22:00:00Z', 2, 'Etc/UTC')).toMatch( + /22:00 – 00:00$/ + ); + expect(rangeLabel(lastBar, 12, 'Europe/London')).toMatch(/12:00 – 00:00$/); }); - test('gives the tooltip the whole slot, in UTC', () => { - expect(rangeLabel(at, 24)).toBe(`${day} UTC`); - expect(rangeLabel(at, 2)).toBe(`${day}, 14:00 – 16:00 UTC`); + // A bar opening at 21:00Z is the start of the next day in Nairobi, and it is + // that date the reader has to see. + test("dates a bar on the reader's calendar, not on UTC", () => { + expect(tickLabel('2026-09-08T21:00:00Z', 24, 'Africa/Nairobi')).toBe( + tickLabel('2026-09-09T00:00:00Z', 24, 'Etc/UTC') + ); + expect(tickLabel('2026-09-08T21:00:00Z', 12, 'Africa/Nairobi')).toBe( + tickLabel('2026-09-09T00:00:00Z', 24, 'Etc/UTC') + ); + }); + + // The half- and quarter-hour zones. The grid is floored on the local clock, + // so a boundary is a local whole hour and no label ever needs minutes. + test('clocks the off-the-hour zones as whole local hours', () => { + expect(tickLabel('2026-09-08T18:30:00Z', 2, 'Asia/Kolkata')).toBe('00:00'); + expect(tickLabel('2026-09-08T18:15:00Z', 2, 'Asia/Kathmandu')).toBe( + '00:00' + ); + }); +}); + +// The bar's own link. Its end is the next bar's start, so the boundary a run +// lands on belongs to exactly one bar — the same half-open slot the server +// counted it in. +describe('bucketUrl', () => { + const buckets = [ + bucket('2026-09-08T00:00:00Z'), + bucket('2026-09-08T02:00:00Z'), + bucket('2026-09-08T04:00:00Z'), + ]; + + const params = (index: number, states: readonly string[] = ['success']) => { + const href = bucketUrl( + links.projectId, + links.workflowId, + buckets, + index, + states + ); + + return href ? new URLSearchParams(href.split('?')[1]) : null; + }; + + test('closes a bar on the next one’s start', () => { + expect(params(1)?.get('filters[run_date_after]')).toBe( + '2026-09-08T02:00:00Z' + ); + expect(params(1)?.get('filters[run_date_before]')).toBe( + '2026-09-08T04:00:00Z' + ); + }); + + // Still filling, and an upper bound would be whenever the response was + // computed rather than now. + test('leaves the newest bar open-ended', () => { + expect(params(2)?.get('filters[run_date_after]')).toBe( + '2026-09-08T04:00:00Z' + ); + expect(params(2)?.has('filters[run_date_before]')).toBe(false); + }); + + test('scopes the link to the workflow', () => { + expect(params(0)?.get('filters[workflow_id]')).toBe('wf-1'); + }); + + // The band, not the bar. `run_status` and not `status`, so a failure since + // retried to success still shows up under the band that counted it. + test('filters on the band’s own run states', () => { + expect( + params(1, RUN_FAILURE_STATES)?.getAll('filters[run_status][]') + ).toEqual([...RUN_FAILURE_STATES]); + expect(params(1)?.getAll('filters[run_status][]')).toEqual(['success']); + expect(params(1)?.has('filters[failed]')).toBe(false); }); }); diff --git a/config/config.exs b/config/config.exs index 6c5f0388ec..f093a38c4e 100644 --- a/config/config.exs +++ b/config/config.exs @@ -173,6 +173,11 @@ config :logger, :console, # Use Jason for JSON parsing in Phoenix config :phoenix, :json_library, Jason +# Without this every `DateTime.shift_zone/2` with a zone name returns +# `{:error, :utc_only_time_zone_database}`. Needed by the workflow health +# charts, which bucket on the reader's clock. +config :elixir, :time_zone_database, Tzdata.TimeZoneDatabase + config :lightning, Lightning.Vault, json_library: Jason config :lightning, Lightning.FailureAlerter, diff --git a/lib/lightning/application.ex b/lib/lightning/application.ex index 5c8f1d7370..d5d85e43e8 100644 --- a/lib/lightning/application.ex +++ b/lib/lightning/application.ex @@ -59,6 +59,11 @@ defmodule Lightning.Application do # Workflow health page stats, cached briefly to dedupe bursts on the same # workflow. See `Lightning.Workflows.Stats`. + # + # Unbounded, keyed by workflow, window and timezone. An entry is ~16 KiB + # and lives two minutes, and filling one costs a ~200 ms aggregate, so the + # database is the scarce resource here and a size limit would bound the + # wrong one. workflow_stats_cache_childspec = Supervisor.child_spec({Cachex, name: :workflow_stats}, id: :workflow_stats_cache diff --git a/lib/lightning/invocation.ex b/lib/lightning/invocation.ex index 0c921fda43..97a039930a 100644 --- a/lib/lightning/invocation.ex +++ b/lib/lightning/invocation.ex @@ -657,6 +657,7 @@ defmodule Lightning.Invocation do |> filter_by_wo_date_before(search_params.wo_date_before) |> filter_by_date_after(search_params.date_after) |> filter_by_date_before(search_params.date_before) + |> filter_by_runs(search_params) |> filter_by_error_signature(search_params) |> filter_by_body_or_log_or_id( search_params.search_fields, @@ -753,6 +754,42 @@ defmodule Lightning.Invocation do ) end + defp filter_by_runs(query, %SearchParams{ + run_date_after: nil, + run_date_before: nil, + run_status: [] + }), + do: query + + defp filter_by_runs(query, %SearchParams{ + run_date_after: run_date_after, + run_date_before: run_date_before, + run_status: run_status + }) do + runs = + from(r in Run, where: r.work_order_id == parent_as(:workorder).id) + |> filter_run_inserted_after(run_date_after) + |> filter_run_inserted_before(run_date_before) + |> filter_run_statuses(run_status) + + from([workorder: _workorder] in query, where: exists(runs)) + end + + defp filter_run_inserted_after(query, nil), do: query + + defp filter_run_inserted_after(query, run_date_after), + do: where(query, [r], r.inserted_at >= ^run_date_after) + + defp filter_run_inserted_before(query, nil), do: query + + defp filter_run_inserted_before(query, run_date_before), + do: where(query, [r], r.inserted_at < ^run_date_before) + + defp filter_run_statuses(query, []), do: query + + defp filter_run_statuses(query, states), + do: where(query, [r], r.state in ^states) + # The inverse of `Run.state_reasons/0`, for reading a run-level signature's # `exit_reason` back into the state it came from. `"rejected"` is not a # value in that map — it is `to_signature/2`'s own literal for a work order diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index 2c96b5f736..18b52e6f68 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -92,16 +92,20 @@ defmodule Lightning.Workflows.Stats do end) end - # How the runs chart slices each window: `{bucket_seconds, bucket_count}`. - # Every width divides a day evenly, so a window whose `from` sits on the grid - # keeps every bucket boundary on the clock — 2-hourly, AM/PM, midnight. + # How the runs chart slices each window: `{bucket_hours, bucket_count}`. + # Widths are wall-clock hours on the reader's calendar, not fixed durations — + # a "daily" bar is a local day, which is 23 or 25 hours across a clock change. # # One bucket more than the width divides into: `now` sits mid-bucket, so a - # grid of exactly `days_back / width` bars would start *after* + # grid of exactly `days_back * 24 / width` bars would start *after* # `now - days_back` and leave the oldest hours of the window undrawn while # the donuts beside it counted them. The oldest bar instead reaches back # past `from`, and `window` reports the range actually covered. - @buckets %{1 => {7_200, 13}, 7 => {43_200, 15}, 30 => {86_400, 31}} + # + # ponytail: the counts are wall-clock, so on a spring-forward day the grid + # covers an hour less real time than the donuts beside it. Add a bucket to + # each width if anyone ever notices. + @buckets %{1 => {2, 13}, 7 => {12, 15}, 30 => {24, 31}} @zero_run_counts Map.new(Run.final_states(), &{&1, 0}) @@ -112,69 +116,135 @@ defmodule Lightning.Workflows.Stats do } @doc """ - Final run counts per state, bucketed across the last `days_back` days: - 2-hourly over a day, AM/PM over a week, daily over a month. + Final run counts per state, bucketed across the last `days_back` days on a + grid cut to `timezone`'s clock: local 2-hourly over a day, local half-days + over a week, local days over a month. Bucketed here rather than in the browser, because the alternative is shipping every run in the window — six figures of rows on a busy workflow, cached whole and JSON-encoded — to draw thirty bars. + Bucketed on the local calendar rather than on the epoch, which is what makes + it daylight-saving-correct: every boundary is a local whole hour turned into + the instant it names, so London's 25-hour October day is one bar 25 hours + wide and every daily boundary stays on local midnight. + Buckets are counted on `inserted_at` — when the attempt started, not when it settled — so a run stays in the bar the traffic arrived in. Every bucket and every state is present, zero-filled: the chart draws a flat window without reasoning about which bars are missing. The last bucket is the one `now` falls in, so it is still filling; the first - reaches back past `now - days_back`, so nothing in the window goes undrawn. - `window` is the range the bars actually cover. + reaches back past `now - days_back`, so nothing in the window goes undrawn — + except on a spring-forward day in the reader's zone, where the grid is a + wall-clock hour wide but an hour short in real time. + `window` is the range the bars actually cover, `bucket_hours` is the width + the chart labels, and `timezone` is the clock both were cut on. A bucket is `at` alongside one key per state, flat rather than nested, which is the row shape Recharts takes as `data` — the list goes to the chart untouched, and each `Bar` names the state it draws. """ - @spec runs(Workflow.t(), 1 | 7 | 30) :: %{ + @spec runs(Workflow.t(), 1 | 7 | 30, Calendar.time_zone()) :: %{ window: %{from: DateTime.t(), to: DateTime.t()}, + timezone: Calendar.time_zone(), + bucket_hours: pos_integer(), buckets: [run_bucket()] } - def runs(%Workflow{id: workflow_id}, days_back \\ @default_days_back) + def runs(%Workflow{id: workflow_id}, days_back, timezone) when is_map_key(@buckets, days_back) do # No change marker in this key — a busy workflow moves the marker faster # than the page polls, so every poll would miss (211 ms and 1.4 GiB of # buffer traffic per recompute at 44k work orders). Costs up to # `@runs_ttl` of staleness on a chart whose finest bar is two hours wide. + # + # The timezone *is* in the key: it moves every boundary, and Cachex keys a + # fetch in flight by key alone (see `cached_until_ttl/3` below), so without + # it the first reader to load the page would pin their grid on every other + # timezone for the TTL. cached_until_ttl( - {:runs, workflow_id, days_back}, - fn -> - {seconds, count} = Map.fetch!(@buckets, days_back) - window = bucket_window(seconds, count) - - %{ - window: window, - buckets: bucket_runs(workflow_id, window.from, seconds, count) - } - end, + {:runs, workflow_id, days_back, timezone}, + fn -> bucket_window(workflow_id, days_back, timezone) end, @runs_ttl ) end + # The start of every bar as a real instant, oldest first, in UTC. Resolved + # here and nowhere else: Tzdata and `pg_timezone_names` are versioned apart, + # so a boundary the two databases place differently would be runs tallied + # into a bar that is never drawn. + # + # The list has to be strictly ascending: `width_bucket` reads an unsorted + # array without complaining and returns a plausible number, so a boundary + # that landed at or before its neighbour would be wrong counts per bar and no + # error. The boundary test holds that for the zones it names — London, + # Havana, Troll, Lord Howe, Chatham — not for all of tzdata. + @doc false + @spec boundaries(DateTime.t(), 1 | 7 | 30, Calendar.time_zone()) :: [ + DateTime.t() + ] + def boundaries(to, days_back, timezone) do + {hours, count} = Map.fetch!(@buckets, days_back) + + to + |> bucket_starts(hours, count, timezone) + |> Enum.map(&resolve(&1, timezone)) + end + + defp bucket_window(workflow_id, days_back, timezone) do + {hours, _count} = Map.fetch!(@buckets, days_back) + to = DateTime.utc_now() + starts = boundaries(to, days_back, timezone) + + %{ + window: %{from: hd(starts), to: to}, + timezone: timezone, + bucket_hours: hours, + buckets: bucket_runs(workflow_id, starts) + } + end + + # The local wall-clock start of every bar, oldest first, as naive local + # datetimes, for `boundaries/3` to resolve. + # # Anchored on the bucket `now` is in, not on `now` itself: a window that ends # mid-bucket would put every boundary at whatever minute the request landed - # on, and the labels the chart draws — "2am", "PM", a date — would be lies. - defp bucket_window(seconds, count) do - to = DateTime.utc_now() - current = DateTime.from_unix!(div(DateTime.to_unix(to), seconds) * seconds) + # on, and the labels the chart draws — an hour, a date — would be lies. + defp bucket_starts(to, hours, count, timezone) do + local = to |> DateTime.shift_zone!(timezone) |> DateTime.to_naive() + + current = + local + |> NaiveDateTime.beginning_of_day() + |> NaiveDateTime.add(div(local.hour, hours) * hours, :hour) + |> NaiveDateTime.truncate(:second) + + Enum.map((count - 1)..0//-1, &NaiveDateTime.add(current, -&1 * hours, :hour)) + end - %{from: DateTime.add(current, -(count - 1) * seconds, :second), to: to} + # A local wall-clock time back to a real instant, reported in UTC like every + # other timestamp on this page. A spring-forward gap has no such instant — + # take the one the clock jumps to; an autumn repeat has two — take the first, + # so the bar opens when its label says it does. + defp resolve(naive, timezone) do + case DateTime.from_naive(naive, timezone) do + {:ok, datetime} -> datetime + {:ambiguous, first, _second} -> first + {:gap, _before, after_gap} -> after_gap + end + |> DateTime.shift_zone!("Etc/UTC") end - defp bucket_runs(workflow_id, from, seconds, count) do - tallies = tally_runs(workflow_id, from, seconds) + defp bucket_runs(workflow_id, starts) do + tallies = tally_runs(workflow_id, starts) - Enum.map(0..(count - 1), fn index -> + starts + |> Enum.with_index(1) + |> Enum.map(fn {start, bucket} -> tallies - |> Map.get(index, []) + |> Map.get(bucket, []) |> Enum.into(@zero_run_counts) - |> Map.put(:at, DateTime.add(from, index * seconds, :second)) + |> Map.put(:at, start) end) end @@ -185,11 +255,14 @@ defmodule Lightning.Workflows.Stats do # the window before the nested loop into `runs(work_order_id, inserted_at)`, # instead of probing every work order the workflow ever had. # - # The grid is aligned, so integer division by the bucket width is the whole - # of the bucketing — no `date_trunc` special case per width. `floor` before - # the cast because `extract` yields `numeric` and `numeric::bigint` rounds: - # without it a run at 01:59:59.7 is counted in the 02:00 bar. - defp tally_runs(workflow_id, from, seconds) do + # `inserted_at` is a naked `timestamp` holding UTC, so the boundaries go down + # as naive UTC datetimes to match it, and `width_bucket` assigns each row the + # 1-based slot it falls in — lower edge inclusive. Slot 0 is below the oldest + # boundary, which the `>= from` filter has already excluded; the highest slot + # is the bar `now` is in, still filling. + defp tally_runs(workflow_id, [from | _] = starts) do + edges = Enum.map(starts, &DateTime.to_naive/1) + from(r in Run, join: wo in WorkOrder, on: wo.id == r.work_order_id, @@ -200,10 +273,9 @@ defmodule Lightning.Workflows.Stats do select: { selected_as( fragment( - "div(floor(extract(epoch from ? - ?))::bigint, ?)::int", + "width_bucket(?, ?)", r.inserted_at, - type(^from, :utc_datetime_usec), - type(^seconds, :integer) + ^edges ), :bucket ), @@ -212,9 +284,10 @@ defmodule Lightning.Workflows.Stats do } ) |> Repo.all() - |> Enum.group_by(fn {bucket, _, _} -> bucket end, fn {_, state, count} -> - {state, count} - end) + |> Enum.group_by( + fn {bucket, _, _} -> bucket end, + fn {_, state, count} -> {state, count} end + ) end defp window(days_back) do @@ -229,7 +302,7 @@ defmodule Lightning.Workflows.Stats do # something settles: a poll that finds nothing has moved is a hit on every # pod, because the answer is read from Postgres and not from one node's ETS. # The trade is that a workflow settling work orders faster than the poll - # recomputes on every poll, which is why `runs/2` opts out and takes + # recomputes on every poll, which is why `runs/3` opts out and takes # `@ttl`-bounded staleness instead. # # The marker only moves when a work order settles, so a stat counting unsettled @@ -239,7 +312,7 @@ defmodule Lightning.Workflows.Stats do # flight by key alone and ignores the fallback closure # (`deps/cachex/lib/cachex/services/courier.ex:60-62`), so a second caller # with a different closure for the same key never runs its own and is handed - # the first one's answer. `outcomes/2`, `error_signatures/2` and `runs/2` are + # the first one's answer. `outcomes/2`, `error_signatures/2` and `runs/3` are # safe because their key prefixes differ. defp cached({_slice, workflow_id, _days} = key, fun) do key diff --git a/lib/lightning/workorders/search_params.ex b/lib/lightning/workorders/search_params.ex index 6b3a082247..0ee17f36dd 100644 --- a/lib/lightning/workorders/search_params.ex +++ b/lib/lightning/workorders/search_params.ex @@ -16,6 +16,9 @@ defmodule Lightning.WorkOrders.SearchParams do :date_before, :wo_date_after, :wo_date_before, + :run_date_after, + :run_date_before, + :run_status, :sort_by, :sort_direction, :error_signature_exit_reason, @@ -30,6 +33,9 @@ defmodule Lightning.WorkOrders.SearchParams do @derive {JSON.Encoder, only: @fields} @status_values Lightning.WorkOrder.states() + # Run states, not work order states: the workflow health page's runs chart + # counts runs, and only a *final* run has an outcome to have been counted. + @run_status_values Lightning.Run.final_states() @search_field_values [:id, :body, :log, :dataclip_name] # String forms for the URI/flag params new/1 receives from the UI. @@ -53,6 +59,9 @@ defmodule Lightning.WorkOrders.SearchParams do date_before: DateTime.t(), wo_date_after: DateTime.t(), wo_date_before: DateTime.t(), + run_date_after: DateTime.t(), + run_date_before: DateTime.t(), + run_status: [atom()], sort_by: String.t(), sort_direction: String.t(), error_signature_exit_reason: String.t(), @@ -79,6 +88,15 @@ defmodule Lightning.WorkOrders.SearchParams do field(:sort_by, :string) field(:sort_direction, :string) + # Workflow health page filters + field(:run_date_after, :utc_datetime_usec) + field(:run_date_before, :utc_datetime_usec) + + field(:run_status, {:array, Ecto.Enum}, + values: @run_status_values, + default: [] + ) + # The error signature the workflow health page's triage row draws its # "View" button from. `error_signature_exit_reason` switches the # filter on; a present `error_signature_job_id` is a step-level row, diff --git a/lib/lightning_web/controllers/api/workflow_health_controller.ex b/lib/lightning_web/controllers/api/workflow_health_controller.ex index 9b353a42c5..126ea99072 100644 --- a/lib/lightning_web/controllers/api/workflow_health_controller.ex +++ b/lib/lightning_web/controllers/api/workflow_health_controller.ex @@ -19,6 +19,7 @@ defmodule LightningWeb.API.WorkflowHealthController do plug :authorize_workflow # After :authorize_workflow so a 404 wins over a 400. plug :validate_days + plug :validate_timezone when action in [:runs] def outcomes(conn, _params) do json( @@ -37,16 +38,29 @@ defmodule LightningWeb.API.WorkflowHealthController do ) end + # `vary` because this is the one action whose body depends on a request + # header, and nothing between the browser and here would guess that. def runs(conn, _params) do - json( - conn, - Workflows.Stats.runs(conn.assigns.workflow, conn.assigns.days_back) + conn + |> put_resp_header("vary", "x-timezone") + |> json( + Workflows.Stats.runs( + conn.assigns.workflow, + conn.assigns.days_back, + conn.assigns.timezone + ) ) end # Closed set, string-matched — no free integer, no parse to defend. @days %{"1" => 1, "7" => 7, "30" => 30} @default_days "30" + @default_timezone "Etc/UTC" + + # CLDR's sentinel for a host clock it could not map to an IANA zone. A + # browser sending it is telling us it does not know, which is the same thing + # as not telling us. + @unknown_timezone "Etc/Unknown" defp validate_days(conn, _opts) do case Map.fetch(@days, conn.params["days"] || @default_days) do @@ -61,6 +75,38 @@ defmodule LightningWeb.API.WorkflowHealthController do end end + # The reader's timezone, because nothing in Lightning records one. The only + # place a default is chosen: a browser that sends no header, or says it does + # not know, gets UTC; anything else that is not a zone is a 400, because the + # browser picked it and drawing someone else's clock would hide that. + # + # Validated before it reaches the cache key, so `:workflow_stats` is keyed on + # the tz database rather than on anything a header can carry. + defp validate_timezone(conn, _opts) do + case get_req_header(conn, "x-timezone") do + [] -> + assign(conn, :timezone, @default_timezone) + + [@unknown_timezone] -> + assign(conn, :timezone, @default_timezone) + + [timezone] -> + if Tzdata.zone_exists?(timezone), + do: assign(conn, :timezone, timezone), + else: reject_timezone(conn) + + _ -> + reject_timezone(conn) + end + end + + defp reject_timezone(conn) do + conn + |> put_status(:bad_request) + |> json(%{error: "x-timezone must be an IANA timezone name"}) + |> halt() + end + defp authorize_workflow(conn, _opts) do %{"project_id" => project_id, "workflow_id" => workflow_id} = conn.params diff --git a/lib/lightning_web/live/run_live/index.ex b/lib/lightning_web/live/run_live/index.ex index 79614fd706..d0e2e16c64 100644 --- a/lib/lightning_web/live/run_live/index.ex +++ b/lib/lightning_web/live/run_live/index.ex @@ -36,6 +36,9 @@ defmodule LightningWeb.RunLive.Index do date_before: :utc_datetime, wo_date_after: :utc_datetime, wo_date_before: :utc_datetime, + run_date_after: :utc_datetime, + run_date_before: :utc_datetime, + run_status: {:array, :string}, pending: :boolean, running: :boolean, success: :boolean, @@ -844,6 +847,21 @@ defmodule LightningWeb.RunLive.Index do end end + # The run-date chip's range. + # + # Stamped in UTC and says so: the bars this comes from are cut on the + # reader's clock, nothing in Lightning records what that clock is, and a bare + # "14:00" under a bar labelled "16:00" is worse than a suffix. The suffix + # goes on once, at the end, rather than on both ends of a range. + defp format_run_range(nil, nil), do: "any time" + defp format_run_range(from, nil), do: "after #{run_stamp(from)} UTC" + defp format_run_range(nil, to), do: "before #{run_stamp(to)} UTC" + + defp format_run_range(from, to), + do: "#{run_stamp(from)} – #{run_stamp(to)} UTC" + + defp run_stamp(date), do: Timex.format!(date, "{D}-{Mshort} {h24}:{m}") + defp format_date_range(date_after, date_before) do case {date_after, date_before} do {nil, nil} -> diff --git a/lib/lightning_web/live/run_live/index.html.heex b/lib/lightning_web/live/run_live/index.html.heex index 323403cdf2..a3f85a6df5 100644 --- a/lib/lightning_web/live/run_live/index.html.heex +++ b/lib/lightning_web/live/run_live/index.html.heex @@ -246,6 +246,44 @@ + <%!-- Run status chip. Only when set: a runs-chart band is the only + thing that sets it, and unlike the two date chips above there + is no dropdown behind it to open. Its own chip rather than a + clause on the run-date one, so a reader who wants the whole + slot can drop the band without losing the slot. --%> + <% run_status = get_change(@filters_changeset, :run_status) || [] %> + <%= if run_status != [] do %> + <.filter_chip + id="run-status-filter-chip" + active={true} + clear_fields={[{:run_status, nil}]} + > + Run status: {Enum.map_join( + run_status, + ", ", + &String.capitalize/1 + )} + + <% end %> + + <%!-- Run date chip. "Created", because these bound a run's + `inserted_at` — the chips above bound the work order's + arrival and its last activity. --%> + <% run_date_after = get_change(@filters_changeset, :run_date_after) + run_date_before = get_change(@filters_changeset, :run_date_before) %> + <%= if run_date_after || run_date_before do %> + <.filter_chip + id="run-dates-filter-chip" + active={true} + clear_fields={[ + {:run_date_after, nil}, + {:run_date_before, nil} + ]} + > + Run created {format_run_range(run_date_after, run_date_before)} + + <% end %> + <%!-- Work order ID chip (only when workorder_id filter is active) --%> <%= if workorder_id = get_change(@filters_changeset, :workorder_id) do %> <.filter_chip diff --git a/test/lightning/invocation_test.exs b/test/lightning/invocation_test.exs index 3aa62fcaea..856b6bc3f1 100644 --- a/test/lightning/invocation_test.exs +++ b/test/lightning/invocation_test.exs @@ -1781,6 +1781,140 @@ defmodule Lightning.InvocationTest do assert found_workorder.id == wo_now.id end + # The runs chart's bar link. Not expressible with either date filter above: + # both work orders here arrived and last moved at the same times, and only + # their runs differ. + test "filters workorders by when a run was created, half-open on the end" do + project = insert(:project) + workflow = insert(:workflow, project: project) + trigger = insert(:trigger, workflow: workflow) + + bar_start = ~U[2026-09-08T02:00:00.000000Z] + bar_end = ~U[2026-09-08T04:00:00.000000Z] + + run_at = fn at -> + work_order = + insert(:workorder, + workflow: workflow, + trigger: trigger, + inserted_at: bar_start, + last_activity: bar_end + ) + + insert(:run, + work_order: work_order, + dataclip: build(:dataclip), + starting_trigger: trigger, + inserted_at: at + ) + + work_order.id + end + + before_bar = run_at.(DateTime.add(bar_start, -1, :second)) + inside_bar = run_at.(DateTime.add(bar_start, 30, :minute)) + on_start = run_at.(bar_start) + # The chart puts a run landing exactly on a boundary in the *later* bar, + # so the bar that closes here must not claim it. + on_end = run_at.(bar_end) + + found = + Invocation.search_workorders( + project, + SearchParams.new(%{ + "run_date_after" => bar_start, + "run_date_before" => bar_end + }) + ).entries + |> Enum.map(& &1.id) + + assert Enum.sort(found) == Enum.sort([inside_bar, on_start]) + refute before_bar in found + refute on_end in found + end + + # A band of that bar. `run_status` and not `status`, which is the work + # order's *current* state: the retried work order here is a success now, + # and the red band that counted its failed run still has to reach it. + test "filters workorders by the run's own state, not the work order's" do + project = insert(:project) + workflow = insert(:workflow, project: project) + trigger = insert(:trigger, workflow: workflow) + + bar_start = ~U[2026-09-08T02:00:00.000000Z] + bar_end = ~U[2026-09-08T04:00:00.000000Z] + + run_at = fn wo_state, run_status -> + work_order = + insert(:workorder, + workflow: workflow, + trigger: trigger, + state: wo_state, + last_activity: bar_end + ) + + insert(:run, + work_order: work_order, + dataclip: build(:dataclip), + starting_trigger: trigger, + state: run_status, + inserted_at: DateTime.add(bar_start, 30, :minute) + ) + + work_order.id + end + + retried = run_at.(:success, :failed) + still_failed = run_at.(:failed, :failed) + succeeded = run_at.(:success, :success) + + failures = + Invocation.search_workorders( + project, + SearchParams.new(%{ + "run_date_after" => bar_start, + "run_date_before" => bar_end, + "run_status" => ["failed", "crashed", "killed", "exception", "lost"] + }) + ).entries + |> Enum.map(& &1.id) + + assert Enum.sort(failures) == Enum.sort([retried, still_failed]) + refute succeeded in failures + end + + # The newest bar is still filling, so its link carries no upper bound. + test "filters workorders by an open-ended run window" do + project = insert(:project) + workflow = insert(:workflow, project: project) + trigger = insert(:trigger, workflow: workflow) + + now = DateTime.utc_now() + + work_order = insert(:workorder, workflow: workflow, trigger: trigger) + + # Two runs, one either side of the bound: the work order matches on the + # newer one, and is listed once rather than twice. + for at <- [DateTime.add(now, -1, :hour), now] do + insert(:run, + work_order: work_order, + dataclip: build(:dataclip), + starting_trigger: trigger, + inserted_at: at + ) + end + + assert [found] = + Invocation.search_workorders( + project, + SearchParams.new(%{ + "run_date_after" => DateTime.add(now, -1, :minute) + }) + ).entries + + assert found.id == work_order.id + end + # to be replaced by paginator unit tests @tag :skip test "filters workorders sets timeout" do @@ -2832,7 +2966,7 @@ defmodule Lightning.InvocationTest do %{project: project, workflow: workflow, snapshot: snapshot, run: run} end - defp add_step(ctx, name, opts \\ []) do + defp add_step(ctx, name, opts) do step = insert( :step, diff --git a/test/lightning/workflows/stats_test.exs b/test/lightning/workflows/stats_test.exs index 065da9c927..9aeb03a2b8 100644 --- a/test/lightning/workflows/stats_test.exs +++ b/test/lightning/workflows/stats_test.exs @@ -129,9 +129,9 @@ defmodule Lightning.Workflows.StatsTest do assert %{signatures: []} = Stats.error_signatures(workflow) end - # The review comment this whole unit change is for: retrying a failure until - # it works has to make the page's failure count fall, and only a work order's - # state can fall — the failed run stays failed forever. + # Retrying a failure until it works has to make the page's failure count + # fall, and only a work order's state can fall — the failed run stays failed + # forever. test "counts a work order retried to success once, as a success", %{ workflow: workflow, trigger: trigger @@ -636,7 +636,7 @@ defmodule Lightning.Workflows.StatsTest do assert %{signatures: []} = Stats.error_signatures(workflow) end - # Both render as `unknown`, so ungrouped the table drew two rows under one + # Both render as `unknown`, so ungrouped they are two table rows under one # React key. test "groups an empty error type with a missing one", %{ workflow: workflow, @@ -656,7 +656,7 @@ defmodule Lightning.Workflows.StatsTest do assert %{count: 2, error_type: nil} = signature end - # `""` is truthy, so it won the `||` in `to_signature/2`. + # `""` is truthy, so it wins the `||` in `to_signature/2`. test "does not let an empty step error type mask the run's", %{ workflow: workflow, trigger: trigger @@ -685,7 +685,7 @@ defmodule Lightning.Workflows.StatsTest do end end - describe "runs/2" do + describe "runs/3" do # A run whose `inserted_at` we choose, on a work order active at the same # moment — the shape the window needs and `insert_run/4` can't give. defp run_at(workflow, trigger, state, at) do @@ -710,14 +710,23 @@ defmodule Lightning.Workflows.StatsTest do |> div(DateTime.diff(b.at, a.at, :second)) end + # Every run in the window lands in some bar. A grid whose edges don't line + # up with the tally drops runs silently, and no per-bar assertion catches + # that, so the total is asserted everywhere the bars are. + defp total(buckets) do + Enum.sum_by(buckets, fn bucket -> + bucket |> Map.delete(:at) |> Map.values() |> Enum.sum() + end) + end + test "counts each state into the bucket its run started in", ctx do %{workflow: workflow, trigger: trigger} = ctx older = DateTime.add(DateTime.utc_now(), -5, :hour) newer = DateTime.add(DateTime.utc_now(), -90, :minute) - # A hair inside a bucket, not in the next one: `extract(epoch ...)` is - # `numeric` and casting it rounds, so the query has to floor first. + # A hair inside a bucket, so it stays there rather than opening the next + # one. edge = DateTime.utc_now() |> DateTime.to_unix() @@ -732,7 +741,7 @@ defmodule Lightning.Workflows.StatsTest do run_at(workflow, trigger, :success, older) run_at(workflow, trigger, :crashed, edge) - assert %{buckets: buckets} = result = Stats.runs(workflow, 1) + assert %{buckets: buckets} = result = Stats.runs(workflow, 1, "Etc/UTC") assert %{success: 2, failed: 0} = Enum.at(buckets, bucket_of(result, older)) @@ -741,39 +750,113 @@ defmodule Lightning.Workflows.StatsTest do Enum.at(buckets, bucket_of(result, newer)) assert %{crashed: 1} = Enum.at(buckets, bucket_of(result, edge)) + assert total(buckets) == 4 + end + + # `width_bucket` takes the lower edge, so a run at the boundary instant + # opens that bar. The slot below the oldest boundary is drawn by no bar at + # all, so a run falling into it would leave the chart without a trace. + test "counts a run landing exactly on a boundary into that bar", ctx do + %{workflow: workflow, trigger: trigger} = ctx + + [from | _] = Stats.boundaries(DateTime.utc_now(), 1, "Etc/UTC") + + run_at(workflow, trigger, :success, from) + + assert %{buckets: [first | _] = buckets} = + Stats.runs(workflow, 1, "Etc/UTC") + + assert %{success: 1} = first + assert total(buckets) == 1 end - # Boundaries on the clock, so the chart can label a bar "2am" or "Tuesday" - # and be telling the truth. And a bar chart with holes in it is a different - # chart, so every bucket carries every final state, zero-filled. + # Boundaries on the reader's clock, so the chart can label a bar with an + # hour or a date and be telling the truth. And a bar chart with holes in it + # is a different chart, so every bucket carries every final state, + # zero-filled. + # + # Kolkata and Kathmandu are the zones that prove the grid really moved: + # their local whole hour is UTC :30 and :15. Europe/London is here because + # a 30 day window may straddle a clock change, and every boundary has to + # stay on a local midnight when it does. test "cuts each window into clock-aligned, zero-filled buckets", ctx do %{workflow: workflow} = ctx zeroed = Map.new(Run.final_states(), &{&1, 0}) - for {days, seconds, count} <- [ - {1, 7_200, 13}, - {7, 43_200, 15}, - {30, 86_400, 31} - ] do - assert %{buckets: buckets, window: window} = Stats.runs(workflow, days) + for timezone <- [ + "Etc/UTC", + "Africa/Nairobi", + "Asia/Kolkata", + "Asia/Kathmandu", + "Europe/London" + ], + {days, hours, count} <- [{1, 2, 13}, {7, 12, 15}, {30, 24, 31}] do + assert %{buckets: buckets, window: window} = + Stats.runs(workflow, days, timezone) assert length(buckets) == count - assert rem(DateTime.to_unix(window.from), seconds) == 0 + assert DateTime.compare(window.from, hd(buckets).at) == :eq + + # Every bar opens on a local whole hour that the width divides — local + # midnight for a daily bar, local noon or midnight for a half-day. + for bucket <- buckets do + local = DateTime.shift_zone!(bucket.at, timezone) + assert local.minute == 0 and local.second == 0 + assert rem(local.hour, hours) == 0 + end + + # An hour's grace: the grid is `count` wall-clock steps wide, and a + # spring-forward day makes that an hour less in real time. assert DateTime.compare( window.from, - DateTime.add(window.to, -days, :day) + window.to |> DateTime.add(-days, :day) |> DateTime.add(1, :hour) ) != :gt - assert DateTime.diff(Enum.at(buckets, 1).at, hd(buckets).at) == seconds - for bucket <- buckets, do: assert(Map.delete(bucket, :at) == zeroed) # The window ends inside the last bucket, which is still filling. last = List.last(buckets).at assert DateTime.compare(last, window.to) == :lt - assert DateTime.diff(window.to, last, :second) < seconds + assert DateTime.diff(window.to, last, :second) < hours * 3_600 + end + end + + # The half- and quarter-hour timezones. A whole-hour one cannot prove the + # grid moved, because at these widths its boundaries are the same list of + # instants as UTC's — the offset cancels out of the arithmetic. These two + # can: a local whole hour is not a UTC whole hour. + test "anchors the grid off the hour for offsets that are", ctx do + %{workflow: workflow} = ctx + + for {timezone, minute} <- [{"Asia/Kolkata", 30}, {"Asia/Kathmandu", 15}] do + assert %{window: window} = Stats.runs(workflow, 1, timezone) + + assert window.from.minute == minute + assert window.from.second == 0 + end + end + + # Without the timezone in the key, the first reader to load the page would + # pin their grid on every other timezone for the TTL — and on one machine + # that is invisible. + test "caches each timezone's grid separately", ctx do + %{workflow: workflow} = ctx + + utc = Stats.runs(workflow, 1, "Etc/UTC") + kolkata = Stats.runs(workflow, 1, "Asia/Kolkata") + + assert utc.window.from != kolkata.window.from + assert utc.timezone == "Etc/UTC" + assert kolkata.timezone == "Asia/Kolkata" + + for timezone <- ["Etc/UTC", "Asia/Kolkata"] do + assert {:ok, true} = + Cachex.exists?( + :workflow_stats, + {:runs, workflow.id, 1, timezone} + ) end end @@ -782,14 +865,17 @@ defmodule Lightning.Workflows.StatsTest do %{workflow: workflow, trigger: trigger} = ctx run_at(workflow, trigger, :success, DateTime.utc_now()) - first = Stats.runs(workflow, 1) + first = Stats.runs(workflow, 1, "Etc/UTC") run_at(workflow, trigger, :failed, DateTime.utc_now()) - assert Stats.runs(workflow, 1) == first + assert Stats.runs(workflow, 1, "Etc/UTC") == first assert {:ok, true} = - Cachex.exists?(:workflow_stats, {:runs, workflow.id, 1}) + Cachex.exists?( + :workflow_stats, + {:runs, workflow.id, 1, "Etc/UTC"} + ) end test "skips runs outside the window, in flight, or on another workflow", @@ -802,11 +888,103 @@ defmodule Lightning.Workflows.StatsTest do other = insert(:simple_workflow) run_at(other, hd(other.triggers), :success, DateTime.utc_now()) - assert %{buckets: buckets} = Stats.runs(workflow, 1) + assert %{buckets: buckets} = Stats.runs(workflow, 1, "Etc/UTC") + + assert total(buckets) == 0 + end + + # Tzdata knows this zone and PG 15 does not. Nothing sends a zone name to + # Postgres, so the two databases disagreeing about it cannot reach the + # chart. + test "draws a zone this Postgres has never heard of", ctx do + %{workflow: workflow, trigger: trigger} = ctx + + run_at(workflow, trigger, :success, DateTime.utc_now()) + + run_at( + workflow, + trigger, + :failed, + DateTime.add(DateTime.utc_now(), -5, :hour) + ) - assert Enum.sum_by(buckets, fn bucket -> - bucket |> Map.delete(:at) |> Map.values() |> Enum.sum() - end) == 0 + assert %{buckets: buckets, timezone: "America/Coyhaique"} = + Stats.runs(workflow, 1, "America/Coyhaique") + + assert total(buckets) == 2 + end + end + + # `runs/3` always ends its window at `DateTime.utc_now()` and nothing here can + # move that clock, so the clock changes are exercised against the grid itself + # at fixed dates. + describe "boundaries/3" do + defp spans(boundaries) do + boundaries + |> Enum.chunk_every(2, 1, :discard) + |> Enum.map(fn [a, b] -> DateTime.diff(b, a, :hour) end) + end + + # The claim the local-calendar grid rests on, and the one a UTC grid gets + # wrong: London's October Sunday is 25 hours long, and both sides of the + # change belong to the same bar. + test "gives a 25-hour day one 24-hour-labelled bar" do + boundaries = + Stats.boundaries(~U[2025-10-27 12:00:00Z], 30, "Europe/London") + + assert Enum.count(spans(boundaries), &(&1 == 25)) == 1 + assert Enum.all?(spans(boundaries), &(&1 in [24, 25])) + assert ~U[2025-10-25 23:00:00Z] in boundaries + assert ~U[2025-10-27 00:00:00Z] in boundaries + end + + # Havana springs forward at local midnight, so the daily boundary itself is + # the hour that never happened. It opens when the clock reaches 01:00. + test "opens a bar whose local start never happened at the jump" do + boundaries = + Stats.boundaries(~U[2026-03-09 12:00:00Z], 30, "America/Havana") + + assert Enum.count(spans(boundaries), &(&1 == 23)) == 1 + assert ~U[2026-03-08 05:00:00Z] in boundaries + end + + # The same midnight twice in November. The bar opens at the first, so it + # opens when its label says it does. + test "opens a bar on the first of two identical local starts" do + boundaries = + Stats.boundaries(~U[2025-11-02 12:00:00Z], 30, "America/Havana") + + assert ~U[2025-11-02 04:00:00Z] in boundaries + refute ~U[2025-11-02 05:00:00Z] in boundaries + end + + # `width_bucket` reads an unsorted array without complaining and returns a + # plausible slot for every row, so a clock change that moved a boundary past + # its neighbour would be silently wrong counts rather than an error. Troll + # shifts two hours, Lord Howe thirty minutes, and Havana's daily boundary + # lands in the change itself. + test "never places a boundary at or before the one before it" do + dates = + for day <- 0..364, + do: DateTime.add(~U[2026-01-01 12:00:00Z], day, :day) + + for zone <- [ + "Europe/London", + "America/Havana", + "Antarctica/Troll", + "Australia/Lord_Howe", + "Pacific/Chatham" + ], + days <- [1, 7, 30], + to <- dates do + boundaries = Stats.boundaries(to, days, zone) + where = "#{zone}, #{days}d, ending #{to}" + + assert boundaries == Enum.sort(boundaries, DateTime), + "out of order: #{where}" + + assert boundaries == Enum.uniq(boundaries), "repeated boundary: #{where}" + end end end end diff --git a/test/lightning_web/controllers/api/workflow_health_controller_test.exs b/test/lightning_web/controllers/api/workflow_health_controller_test.exs index 9a5f9ba167..e741355486 100644 --- a/test/lightning_web/controllers/api/workflow_health_controller_test.exs +++ b/test/lightning_web/controllers/api/workflow_health_controller_test.exs @@ -33,9 +33,17 @@ defmodule LightningWeb.API.WorkflowHealthControllerTest do ) end - defp get_runs(conn, user, project_id, workflow_id, params \\ %{}) do - conn - |> log_in_user(user) + defp get_runs( + conn, + user, + project_id, + workflow_id, + params \\ %{}, + headers \\ [] + ) do + Enum.reduce(headers, log_in_user(conn, user), fn {name, value}, conn -> + put_req_header(conn, name, value) + end) |> get( ~p"/api/projects/#{project_id}/workflows/#{workflow_id}/health/runs", params @@ -395,6 +403,97 @@ defmodule LightningWeb.API.WorkflowHealthControllerTest do end end + # Nothing in Lightning records a reader's timezone, so the browser says. No + # header, or one saying the browser does not know, is UTC; anything else the + # tz database does not know is a 400, because the browser chose it. + describe "x-timezone" do + test "cuts the grid on the timezone the reader sent", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + response = + conn + |> get_runs(user, project.id, workflow.id, %{"days" => "1"}, [ + {"x-timezone", "Africa/Nairobi"} + ]) + |> json_response(200) + + assert response["timezone"] == "Africa/Nairobi" + + {:ok, from, 0} = DateTime.from_iso8601(response["window"]["from"]) + local = DateTime.shift_zone!(from, "Africa/Nairobi") + + assert local.minute == 0 and local.second == 0 + end + + test "falls back to UTC without the header", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + response = + conn + |> get_runs(user, project.id, workflow.id, %{"days" => "1"}) + |> json_response(200) + + assert response["timezone"] == "Etc/UTC" + end + + # A host clock CLDR could not map to a zone. The browser is saying it does + # not know, not naming one we failed to recognise. + test "falls back to UTC when the browser says it does not know", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + response = + conn + |> get_runs(user, project.id, workflow.id, %{"days" => "1"}, [ + {"x-timezone", "Etc/Unknown"} + ]) + |> json_response(200) + + assert response["timezone"] == "Etc/UTC" + end + + test "tells caches the body turns on the header", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + conn = get_runs(conn, user, project.id, workflow.id, %{"days" => "1"}) + + assert get_resp_header(conn, "vary") == ["x-timezone"] + end + + # Unknown, empty, and long — the cache key has to stay bounded to the tz + # database whatever arrives. + test "rejects a value it cannot use", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + for value <- ["Mars/Olympus", "", String.duplicate("x", 5_000)] do + response = + conn + |> get_runs(user, project.id, workflow.id, %{"days" => "1"}, [ + {"x-timezone", value} + ]) + |> json_response(400) + + assert response == %{ + "error" => "x-timezone must be an IANA timezone name" + } + end + end + end + describe "?days=" do @accepted_days [1, 7, 30] diff --git a/test/lightning_web/live/run_live/index_test.exs b/test/lightning_web/live/run_live/index_test.exs index 6305e5c43f..848d89fdf8 100644 --- a/test/lightning_web/live/run_live/index_test.exs +++ b/test/lightning_web/live/run_live/index_test.exs @@ -1049,6 +1049,85 @@ defmodule LightningWeb.RunLive.IndexTest do refute has_element?(view, "#error-signature-filter-chip") end + + # A band of a bar on the workflow health page's runs chart links here, and + # sets both. Two chips, so either can be dropped without the other: the + # band without the slot is every failure in the range, the slot without + # the band is the whole bar. + test "a runs-chart band renders a run status chip and a run date chip", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{ + run_date_after: "2026-09-08T02:00:00Z", + run_date_before: "2026-09-08T04:00:00Z", + run_status: ["failed", "crashed"] + } + ) + ) + + assert render(element(view, "#run-status-filter-chip")) =~ + "Run status: Failed, Crashed" + + dates = render(element(view, "#run-dates-filter-chip")) + + # The stamp is UTC and says so: the bars are cut on the reader's clock + # and nothing here records what that clock is. + assert dates =~ "Run created 8-Sep 02:00" + assert dates =~ "8-Sep 04:00 UTC" + end + + test "a run status filter renders no run date chip", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{run_status: ["success"]} + ) + ) + + assert render(element(view, "#run-status-filter-chip")) =~ + "Run status: Success" + + refute has_element?(view, "#run-dates-filter-chip") + end + + # The newest bar is still filling, so its link carries no upper bound. + test "run date chip reads open-ended without a run_date_before", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{run_date_after: "2026-09-08T02:00:00Z"} + ) + ) + + assert render(element(view, "#run-dates-filter-chip")) =~ + "Run created after 8-Sep 02:00 UTC" + + refute has_element?(view, "#run-status-filter-chip") + end + + test "run chips are absent when neither filter is set", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async(conn, Routes.project_run_index_path(conn, :index, project.id)) + + refute has_element?(view, "#run-status-filter-chip") + refute has_element?(view, "#run-dates-filter-chip") + end end describe "cancel work orders" do