From 82da7be547e4943089db7bd39ac5918f23de686f Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Sun, 2 Aug 2026 05:41:08 +0200 Subject: [PATCH 1/2] fix(dashboards): bound TF queries during page render, degrade to client fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dashboard page render ran TimeFusion queries inline with no timeout anywhere on the path: processConstant, processVariable and processEagerWidget all had to finish before the shell shipped, and mkHasqlPool sets only a 30s *acquisition* timeout (chart queries don't even use that pool — they go through the postgresql-simple timefusionPgPool). So when TF wedged on 2026-08-02, every render thread parked on it, the three replicas grew into their 24GB cap, and Swarm OOM-killed them in a loop while /status timed out. log_explorer stayed up throughout because it already splits chrome (apiLogH) from data (logExplorerDataH); dashboards had no such split. Wrap each render-time query in a 4s budget and fall back to the un-prefilled value. That is exactly the state the client already recovers from: table shells always emit hx-trigger="load" and a spinner when html is absent, stat content includes `load` whenever it has no data, and chartWidget shows "Loading chart…" and fetches on intersection when dataset.source is empty. A slow TF now degrades dashboards from server-rendered to client-fetched instead of from working to down. lazyWidget must clear `eager` as well as html/dataset: renderStatContent reads the flag itself as "data is present" and drops `load` from the trigger, so a widget left flagged-but-empty renders a spinner nothing ever resolves. WidgetLazySpec pins both directions. WTAnomalies is excluded deliberately — it reads Postgres, not TF, and widget_ renders nothing at all without html. Also move the container HEALTHCHECK from /status to /ping. statusH runs `select version()`, which made every replica's liveness depend on one shared Postgres, so a single DB hiccup failed all three probes at once and Swarm killed the whole service. A probe whose failure action is "kill the container" must test liveness only; dependency health already alerts via InfraHealthCheck. --- Dockerfile | 16 +++++-- monoscope.cabal | 5 +++ package.yaml | 5 +++ src/Pages/Dashboards.hs | 78 ++++++++++++++++++++++++++++++--- test/unit/Pkg/WidgetLazySpec.hs | 64 +++++++++++++++++++++++++++ 5 files changed, 157 insertions(+), 11 deletions(-) create mode 100644 test/unit/Pkg/WidgetLazySpec.hs diff --git a/Dockerfile b/Dockerfile index 0d77bae28..c0eea89b6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -129,11 +129,19 @@ ENV GHCRTS="-p -poprofiles/monoscope -l-a --eventlog-flush-interval=30 -hc -i60" # Liveness of the web listener itself (not just the process): a wedged warp # accept-loop leaves the container "running" but refusing connections, so Swarm -# keeps routing 1/N of VIP traffic to a black-hole. Probe /status (bare 200, no -# auth redirect) over bash /dev/tcp — the image has no curl/wget. ${PORT:-8080} -# matches prod (PORT=80) and the exposed default. +# keeps routing 1/N of VIP traffic to a black-hole. Probe over bash /dev/tcp — +# the image has no curl/wget. ${PORT:-8080} matches prod (PORT=80) and the +# exposed default. +# +# /ping, NOT /status: this probe's failure action is "kill the container", so it +# must test liveness only. /status runs `select version()`, which makes every +# replica's liveness depend on one shared Postgres — a single DB hiccup fails all +# three probes at once and Swarm kills the whole service. /ping is a pure handler, +# so it still proves the accept loop and handler threads are alive (the thing this +# probe exists to catch) without coupling liveness to a dependency. Dependency +# health belongs in the InfraHealthCheck job, which alerts instead of killing. HEALTHCHECK --interval=15s --timeout=5s --start-period=90s --retries=3 \ - CMD ["bash","-c","exec 3<>/dev/tcp/127.0.0.1/${PORT:-8080}; printf 'GET /status HTTP/1.0\\r\\nHost: localhost\\r\\n\\r\\n' >&3; head -1 <&3 | grep -q '200 OK'"] + CMD ["bash","-c","exec 3<>/dev/tcp/127.0.0.1/${PORT:-8080}; printf 'GET /ping HTTP/1.0\\r\\nHost: localhost\\r\\n\\r\\n' >&3; head -1 <&3 | grep -q '200 OK'"] # Timestamped eventlog per boot: a fixed -ol path would be truncated by the # restart right after an OOM kill, destroying exactly the samples we want. diff --git a/monoscope.cabal b/monoscope.cabal index 81f381d04..0c35c758e 100644 --- a/monoscope.cabal +++ b/monoscope.cabal @@ -1157,6 +1157,7 @@ test-suite unit-tests Pkg.Parser.ExprSpec Pkg.ParserSpec Pkg.QueryCacheSpec + Pkg.WidgetLazySpec RequestMessagesSpec Spec System.ServerSpec @@ -1194,9 +1195,13 @@ test-suite unit-tests , async , base , containers + , data-default , effectful-core , extra + , generic-lens , hasql + , lens + , lucid , hasql-pool , hs-opentelemetry-instrumentation-auto , hspec diff --git a/package.yaml b/package.yaml index 439f2166c..0520bf0e3 100644 --- a/package.yaml +++ b/package.yaml @@ -284,6 +284,11 @@ tests: - hspec - relude >= 1.2.0.0 - aeson + # Pkg.WidgetLazySpec builds a Widget and renders it + - data-default + - generic-lens + - lens + - lucid - async - containers - text diff --git a/src/Pages/Dashboards.hs b/src/Pages/Dashboards.hs index f828e6bcf..db8983f39 100644 --- a/src/Pages/Dashboards.hs +++ b/src/Pages/Dashboards.hs @@ -26,6 +26,7 @@ module Pages.Dashboards ( dashboardWidgetExpandGetH, visTypes, processEagerWidget, + lazyWidget, fetchWidgetData, dashboardBulkActionPostH, TabRenameForm (..), @@ -105,6 +106,7 @@ import System.Logging qualified as Log import System.Tracing (Tracing) import System.Types import Text.Slugify (slugify) +import UnliftIO qualified import UnliftIO.Exception (try) import Utils import Web.FormUrlEncoded (FromForm) @@ -629,9 +631,13 @@ processVariable pid now (sinceStr, fromDStr, toDStr) allParams variableBase = do Dashboards.VTQuery | Just sqlQuery <- variable.sql -> do -- SECURITY: Use secured query execution with project_id filtering useTf <- (.env.enableTimefusionReads) <$> ask @AuthContext - LogQueries.executeSecuredQuery useTf pid sqlQuery 1000 <&> \case - Right queryResults -> variable{Dashboards.options = Just $ queryRowsToText queryResults} - Left _ -> variable -- Return unchanged on error + -- Budgeted for the same reason as the facet/dependent short-circuits above: + -- an option list is never worth blocking the shell on. + withRenderBudget ("variable:" <> variable.key) variable + $ LogQueries.executeSecuredQuery useTf pid sqlQuery 1000 + <&> \case + Right queryResults -> variable{Dashboards.options = Just $ queryRowsToText queryResults} + Left _ -> variable -- Return unchanged on error _ -> pure variable @@ -745,6 +751,44 @@ variablePickerModal_ pid dashId activeTabSlug allParams var useOob = do forM_ keys $ kbd_ [class_ "kbd kbd-xs"] . toHtml +-- | Wall-clock budget for a single TimeFusion-backed query run *during page render* +-- (a constant, a variable's option list, an eager widget prefill). These all used to +-- be unbounded, so a slow or wedged TF held the dashboard shell hostage: on +-- 2026-08-02 every render thread parked on TF, the three replicas grew to their 24GB +-- cap and Swarm OOM-killed them in a loop while /status timed out. +-- +-- The prefill is an optimisation, never a requirement — every widget kind can fetch +-- its own data client-side (see 'withRenderBudget'). So we cap the wait and ship the +-- shell; a blown budget costs a spinner, not a page. +-- +-- This is per query, and render runs three sequential phases (constants, then +-- variables, then widgets), each internally concurrent — so a total TF stall bounds +-- the shell at ~3x this, not 1x. Healthy TF is unaffected: these queries return in +-- milliseconds and the timeout never fires. +renderQueryBudgetMicros :: Int +renderQueryBudgetMicros = 4_000_000 + + +-- | Run a render-time query under 'renderQueryBudgetMicros', falling back to +-- @fallback@ when the budget is blown. +-- +-- Every caller's fallback leaves the widget/variable/constant in its *un-prefilled* +-- state, which is exactly the state the client already knows how to recover from: +-- tables render their spinner and fire @hx-trigger=\"load\"@, stats include @load@ in +-- their trigger whenever they have no data, and charts show \"Loading chart…\" and +-- fetch on intersection when @dataset.source@ is empty. So a slow TF degrades the +-- dashboard from server-rendered to client-fetched rather than from working to down. +-- +-- Abandoning the query is safe: resource-pool destroys a connection whose borrower +-- died, so we drop the connection rather than return a half-read one to the pool. +withRenderBudget :: Text -> a -> ATAuthCtx a -> ATAuthCtx a +withRenderBudget label fallback action = + UnliftIO.timeout renderQueryBudgetMicros action >>= \case + Just a -> pure a + Nothing -> + Log.logWarn "Dashboard render query exceeded budget; degrading to client-side fetch" label $> fallback + + -- | Process a single dashboard constant by executing its SQL or KQL query and populating the result. -- Constants are executed once and their results are made available to all widgets. processConstant :: Projects.ProjectId -> UTCTime -> (Maybe Text, Maybe Text, Maybe Text) -> [(Text, Maybe Text)] -> Dashboards.Constant -> ATAuthCtx Dashboards.Constant @@ -776,16 +820,33 @@ processWidget :: Projects.ProjectId -> UTCTime -> (Maybe Text, Maybe Text, Maybe processWidget pid now timeRange allParams widgetBase = do let widget = widgetBase & #_projectId %~ (<|> Just pid) & #rawQuery .~ widgetBase.query + -- The prefill is best-effort: past the budget we hand back the widget with no + -- html/dataset and no `eager` flag, which is precisely the shape whose renderer + -- emits a spinner plus a self-fetch. Anomalies widgets read Postgres, not TF, and + -- have no client-side fetch path, so they keep their (unbounded) eager render. widget' <- - if widget.eager == Just True || widget.wType == Widget.WTAnomalies - then processEagerWidget pid now timeRange allParams widget - else pure widget + if widget.eager == Just True + then + withRenderBudget ("widget:" <> maybeToMonoid widget.title) (lazyWidget widget) + $ processEagerWidget pid now timeRange allParams widget + else + if widget.wType == Widget.WTAnomalies + then processEagerWidget pid now timeRange allParams widget + else pure widget -- Recursively process child widgets concurrently, inheriting the parent's dashboard id forOf (#children . _Just) widget' \kids -> pooledForConcurrently kids $ processWidget pid now timeRange allParams . (#_dashboardId %~ (<|> widget'._dashboardId)) +-- | Strip every trace of a server-side prefill so the widget renders as if it had +-- never been eager. @eager@ must go too: 'Widget.renderStatContent' treats the flag +-- itself as \"data is present\" and drops @load@ from its HTMX trigger, so a widget +-- left flagged-but-empty would render a spinner that never resolves. +lazyWidget :: Widget.Widget -> Widget.Widget +lazyWidget w = w & #eager .~ Nothing & #html .~ Nothing & #dataset .~ Nothing + + -- | Fetch widget data based on widget type (for stat and chart widgets) fetchWidgetData :: (DB es, Effectful.Reader.Static.Reader AuthContext :> es, Error ServerError :> es, Log :> es, Time.Time :> es, Tracing :> es) => Projects.ProjectId -> (Maybe Text, Maybe Text, Maybe Text) -> [(Text, Maybe Text)] -> Widget.Widget -> Eff es Widget.Widget fetchWidgetData pid (sinceStr, fromDStr, toDStr) allParams widget = case widget.wType of @@ -2148,7 +2209,10 @@ processConstantsAndExtendParams pid now timeParams allParams haystack constants ) where processOne c - | ("{{const-" <> c.key) `T.isInfixOf` haystack || ("{{" <> c.key <> "}}") `T.isInfixOf` haystack = processConstant pid now timeParams allParams c + -- A blown budget leaves the constant result-less, which renders as the same + -- empty-sentinel params an unreferenced constant produces. + | ("{{const-" <> c.key) `T.isInfixOf` haystack || ("{{" <> c.key <> "}}") `T.isInfixOf` haystack = + withRenderBudget ("constant:" <> c.key) c $ processConstant pid now timeParams allParams c | otherwise = pure c -- unreferenced: emit empty-sentinel params without running the query diff --git a/test/unit/Pkg/WidgetLazySpec.hs b/test/unit/Pkg/WidgetLazySpec.hs new file mode 100644 index 000000000..f4af0ba1c --- /dev/null +++ b/test/unit/Pkg/WidgetLazySpec.hs @@ -0,0 +1,64 @@ +-- | Regression guard for the 2026-08-02 dashboard outage. +-- +-- Dashboard page render prefilled widgets by querying TimeFusion inline, with no +-- timeout. When TF wedged, every render thread parked on it, the monoscope +-- replicas grew to their 24GB cap and Swarm OOM-killed them in a loop. +-- +-- 'lazyWidget' is the escape hatch: past the render budget we drop the prefill and +-- let the browser fetch the data. That only works if a prefill-less widget renders +-- a shell that actually re-fetches, so these tests pin that contract from both +-- sides — the stripping, and the renderer's reaction to it. +module Pkg.WidgetLazySpec (spec) where + +import Control.Lens ((&), (.~), (?~)) +import Data.Default (def) +import Data.Generics.Labels () +import Data.Text qualified as T +import Data.Text.Lazy qualified as TL +import Lucid (renderText, toHtml) +import Pages.Dashboards (lazyWidget) +import Pkg.Components.Widget qualified as Widget +import Relude +import Test.Hspec + + +-- | A minimal stat widget. Stat is the interesting case: its renderer decides +-- whether to self-fetch from the widget's own fields rather than always emitting +-- a @load@ trigger the way the table shell does. +statWidget :: Widget.Widget +statWidget = def & #wType .~ Widget.WTStat & #id ?~ "w1" & #title ?~ "Requests" + + +render :: Widget.Widget -> Text +render = toText . TL.toStrict . renderText . toHtml + + +-- | HTMX only fires a request on page load when @load@ is in the trigger list. +selfFetches :: Widget.Widget -> Bool +selfFetches = T.isInfixOf "\"load, update-query from:window\"" . render + + +spec :: Spec +spec = describe "lazyWidget (dashboard render-budget fallback)" do + it "strips every field that marks a widget as prefilled" do + let prefilled = + statWidget + & #eager ?~ True + & #html ?~ "cached" + & #dataset ?~ (def & #value ?~ 42) + stripped = lazyWidget prefilled + stripped.eager `shouldBe` Nothing + stripped.html `shouldBe` Nothing + -- WidgetDataset has no Eq instance, so assert on the constructor. + isNothing stripped.dataset `shouldBe` True + + it "produces a widget that fetches its own data" do + -- The payoff: a degraded widget must come back with a spinner that resolves. + selfFetches (lazyWidget (statWidget & #eager ?~ True)) `shouldBe` True + + -- The trap this guards. renderStatContent reads `eager` as "data is present" and + -- drops `load` from the trigger, so clearing html/dataset but LEAVING the flag + -- renders a spinner that nothing ever resolves. A blown budget would then show a + -- permanently-loading dashboard instead of a slow one. + it "would hang if the eager flag were left set" do + selfFetches (statWidget & #eager ?~ True & #html .~ Nothing & #dataset .~ Nothing) `shouldBe` False From 9e950bdbb07025ee8df840e24a9697c79b86e15b Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Sun, 2 Aug 2026 05:57:38 +0200 Subject: [PATCH 2/2] fix(dashboards): exclude anomalies widgets before the eager check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch: the budget branch tested `eager` first, so a WTAnomalies widget that also set `eager: true` got budgeted and lazyWidget'd on timeout — blanking the card, which is exactly what the exclusion existed to prevent. `eager` is a free Maybe Bool with no type-level tie to wType, so config can set both. Reorder as a MultiWayIf with anomalies first, and label the budget warning by widget id rather than title (untitled widgets all logged the same string). --- src/Pages/Dashboards.hs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Pages/Dashboards.hs b/src/Pages/Dashboards.hs index db8983f39..ddcd51c54 100644 --- a/src/Pages/Dashboards.hs +++ b/src/Pages/Dashboards.hs @@ -822,17 +822,22 @@ processWidget pid now timeRange allParams widgetBase = do -- The prefill is best-effort: past the budget we hand back the widget with no -- html/dataset and no `eager` flag, which is precisely the shape whose renderer - -- emits a spinner plus a self-fetch. Anomalies widgets read Postgres, not TF, and - -- have no client-side fetch path, so they keep their (unbounded) eager render. + -- emits a spinner plus a self-fetch. widget' <- - if widget.eager == Just True - then - withRenderBudget ("widget:" <> maybeToMonoid widget.title) (lazyWidget widget) - $ processEagerWidget pid now timeRange allParams widget - else - if widget.wType == Widget.WTAnomalies - then processEagerWidget pid now timeRange allParams widget - else pure widget + if + -- Anomalies first, ahead of the `eager` check. They read Postgres rather than + -- TF, and `widget_` renders `whenJust w.html toHtmlRaw` — nothing at all + -- without html — so there is no client-side fetch to degrade to and a blown + -- budget would blank the card. `eager` is a free `Maybe Bool` with no + -- type-level tie to `wType`, so a custom dashboard can set both; ordering the + -- guard this way makes the exclusion hold for every input, not just the + -- built-in templates. + | widget.wType == Widget.WTAnomalies -> processEagerWidget pid now timeRange allParams widget + -- Label by id, not title: untitled widgets would all log the same string. + | widget.eager == Just True -> + withRenderBudget ("widget:" <> maybeToMonoid widget.id) (lazyWidget widget) + $ processEagerWidget pid now timeRange allParams widget + | otherwise -> pure widget -- Recursively process child widgets concurrently, inheriting the parent's dashboard id forOf (#children . _Just) widget' \kids ->