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..ddcd51c54 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,38 @@ 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. widget' <- - if widget.eager == Just True || 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 -> 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 +2214,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