Skip to content

Fan the service-map rollup out over projects that actually send spans - #482

Merged
tonyalaribe merged 2 commits into
masterfrom
fix/htmx4-event-migration
Aug 8, 2026
Merged

Fan the service-map rollup out over projects that actually send spans#482
tonyalaribe merged 2 commits into
masterfrom
fix/htmx4-event-migration

Conversation

@tonyalaribe

Copy link
Copy Markdown
Contributor

What

Two commits:

  • fix(ai panel) — quote the load-chat event so the chat actually loads (htmx 4 follow-up).
  • Service-map rollup fan-out — the dispatcher enqueued one rollup per active project (1,726 in production) every 5 minutes. The rollup's span self-join costs the same scan whether or not the project has traffic, and almost none do at a given tick, so nearly all of that budget went to proving there was nothing to do: ~497k odd-jobs/day and ~5.8 TimeFusion self-joins/second, of the query shape that has OOM-killed TF before.

projectsWithSpansInRange answers for every project in one scan. It is deliberately rollupServiceEdges' own FROM clause with the per-project predicate lifted out — same store, window and kind filter — so a project is on the list exactly when the rollup would have found rows for it.

Notes

  • project_id is read as text and parsed rather than cast with ::uuid (DataFusion has no uuid type); an unparseable value is dropped rather than costing every other project its bucket.
  • Follows runHourlyJob, which already derives its project list from the span table with no projects.active intersection.
  • No behaviour change today: ENABLE_SERVICE_MAP_ROLLUP is still off, so apis.service_dependency_edges stays empty and every service map still renders "No service activity". This is prep for turning that flag on.

Testing

projectDiscovery_listsOnlyProjectsThatSentSpansInTheBucket pins both directions — the fixture's bucket returns [testPid], and a window where that same still-active project sent nothing returns []. All 8 ServiceMap specs pass.

Unverified: the discovery query is exercised against Postgres in tests (useTf = False). Its behaviour on TimeFusion — where it will actually run — has not been measured. It is a much gentler shape than the self-join (single column, no join, one 5-minute window, low-cardinality DISTINCT), but it should be timed against real TF before ENABLE_SERVICE_MAP_ROLLUP is flipped on.

🤖 Generated with Claude Code

https://claude.ai/code/session_014AteFpMusSMajKqCe2kPJa

`trigger load-chat on #ai-response-container` failed to parse: hyperscript
tokenizes the `-` as minus, so the whole feature block was rejected with
three parse errors and neither handler was ever bound. The AI panel
therefore never fetched its history — not on toggle, and not on reload
with the panel remembered open. Quoting the event name is the existing
convention here (`trigger 'update-query' on window`).

Also put hx-swap on the drawer's lazy-load element instead of leaving it
to be inherited from the wrapper. htmx 4 resolves inheritance explicitly,
so a parent's value no longer reaches the child. No behaviour change:
every drawer_ caller passes Nothing for the url, so the element is not
rendered today, and htmx 4's defaultSwap is innerHTML anyway.
The dispatcher enqueued one rollup per active project — 1726 of them in
production, every 5 minutes. The rollup's span self-join costs the same
scan whether or not a project has traffic, and almost none do at any given
tick, so ~all of that budget went to proving there was nothing to do:
~497k odd-jobs and ~5.8 TimeFusion self-joins per second, of the query
shape that has OOM-killed it before.

projectsWithSpansInRange answers for every project in one scan. It is
deliberately rollupServiceEdges' own FROM clause with the per-project
predicate lifted out — same store, window and kind filter — so a project
is on the list exactly when the rollup would have found rows for it.
project_id is read as text and parsed rather than cast with ::uuid, which
DataFusion has no type for, and an unparseable value is dropped rather
than costing every other project its bucket.

Follows runHourlyJob, which already derives its project list from the span
table with no projects.active intersection.
@tonyalaribe
tonyalaribe merged commit eec1347 into master Aug 8, 2026
8 of 9 checks passed
@tonyalaribe
tonyalaribe deleted the fix/htmx4-event-migration branch August 8, 2026 11:41
@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review

Solid, well-documented change — the Haddock comments carry real justification (window/kind-filter parity with rollupServiceEdges, the runHourlyJob precedent for not intersecting projects.active), and the new test (projectDiscovery_listsOnlyProjectsThatSentSpansInTheBucket) pins exactly the property that matters (in-bucket vs. idle-bucket). The two htmx/hyperscript fixes are minimal and each has a comment explaining the underlying htmx4/hyperscript behavior, which is exactly the kind of thing that isn't derivable from the diff.

Reuse: this is now the 3rd near-duplicate "distinct project_id from spans in a window" query

  • runHourlyJob (src/BackgroundJobs.hs:1300-1305): SELECT DISTINCT project_id::uuid FROM otel_logs_and_spans WHERE timestamp >= ... AND timestamp <= ... (plain Hasql, Postgres only).
  • Projects.recentlyActiveProjectIds (src/Models/Projects/Projects.hs:439-442): joins projects.projects + otel_logs_and_spans, hardcoded 24h window, plain Hasql only. The docs/service-map-spec.md:499 design doc actually called for the rollup dispatcher to fan out over this function ("the way the daily seeder already does").
  • ServiceGraph.projectsWithSpansInRange (this PR, src/Models/Telemetry/ServiceGraph.hs:393-402): new function, parametrized window + kind filter + PG/TimeFusion switch via useTf.

The divergence from the spec doc is defensible — recentlyActiveProjectIds is hardcoded to 24h and joins projects.active, neither of which fits a 5-minute rollup bucket that deliberately wants no active-project intersection (per the runHourlyJob precedent cited in the PR body) — but it's worth calling out explicitly since it contradicts the written design doc, and because three slightly-different copies of the same query shape is exactly the kind of drift risk that bites later (e.g. if the kind list or the span table's schema changes, someone has to remember to update three places, not one). A shared helper — e.g. projectsWithSpansInRange taking an optional kind filter and living somewhere both BackgroundJobs and ServiceGraph can reach — would collapse this to one implementation. Not a blocker, but worth a follow-up issue if not addressed now.

Correctness

  • Window and kind filter in projectsWithSpansInRange are verified consistent with rollupServiceEdges's own FROM/WHERE (timestamp >= lo AND timestamp < hi, same four kinds) — good, this is the property the whole optimization depends on.
  • mapMaybe (idFromText @"project") silently drops unparseable project_id text — intentional and documented, reasonable tradeoff (one bad row shouldn't sink the whole tick).
  • Dropping the projects.active/deleted_at intersection is intentional and consistent with existing runHourlyJob behavior, but it's worth double-checking: can a soft-deleted or deactivated project still have rows land in otel_logs_and_spans within the lookback window (e.g., a race between deactivation and in-flight ingestion, or retained historical rows)? If so this will enqueue a rollup for a project that arguably shouldn't get one. Given runHourlyJob already has this exact behavior, it's presumably an accepted risk rather than a new one — just flagging for confirmation.

Performance / Test coverage / Security

  • The DISTINCT project_id scan against TimeFusion is unverified per the PR description itself ("timed against real TF before ENABLE_SERVICE_MAP_ROLLUP is flipped on") — agreed this is the one thing that must happen before enabling the flag, since the whole point of the PR is avoiding TF load and an unverified new query shape against TF is exactly where that could go wrong again.
  • Tests only exercise the Postgres path (useTf = False); TimeFusion behavior (including the text/uuid parsing path) is untested by definition of the test harness. Same caveat as above — acceptable since self-disclosed, but worth a tracking item.
  • SQL uses Hasql.interp's typed interpolation throughout (no string concatenation) — no injection concern.
  • No behavior change today since ENABLE_SERVICE_MAP_ROLLUP is off — good staged-rollout discipline.

Minor

  • Log.logInfo "Service-map rollup fan-out" $ AE.object ["bucket" AE..= bucket, "projects" AE..= length projects] — the "projects" field holds a count, not the list; "project_count" (matching the naming already used a few lines up in runHourlyJob's Log.logTrace ... ("count", ...) and NotificationSweepJob's "count") would avoid the field name implying an array.

Nice find on the hyperscript minus-sign parse failure — that's a nasty silent breakage (three parse errors, no exception surfaced to the user) and the fix is exactly right (matches the existing 'update-query' quoting convention).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant