feat(unfold): export insight events to Faro or OTLP #249
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "agent/vik-1897"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
Once Unfold's product events exist, the owner can only read them with SQL: the homelab already runs Alloy's faro.receiver and Grafana, but Unfold exports nothing there, so the Needs-you tables of RFC-0001 are unavailable. The server had no insight export settings, no event store and no collector sink.
Solution
UNFOLD_INSIGHT_EXPORT(off/faro/otlp),_URLand_LEVEL(aggregate/events) settings and documented them inapps/unfold/docs/operations/live.md.apps/unfold/src/insight.ts: the RFC-0001 event catalogue, a per-tenant pseudonymous actor hash, theproduct_event/product_event_dailystore inapps/unfold/src/store.ts, and server-side Faro/OTLP forwarding.POST /api/insight/eventsinapps/unfold/src/http.ts; it runs only on the server, so the browser never contacts the collector and the CSP is unchanged, and an unreachable collector is logged once an hour without blocking the route.apps/unfold/ops/grafana/unfold-insight.json, the RFC-0001 stat panels and tables that suppress groups under five people.apps/unfold/test/insight.test.ts(service, fake collector),apps/unfold/test/api-insight.test.ts(route) andapps/unfold/test/config.test.ts.Checks left to CI
This sandbox has Node 24 and Go, but no npm registry egress and no
node_modules, sows(a production dependency) is absent and tests that importapi-support.tscannot run locally. The Ploeg verify script passed (gofmt -l apps/ploegclean). Locally run:node scripts/check.mjs(passes) andnode --test test/insight.test.ts test/config.test.ts(13 tests pass, including the fake-collector Faro/OTLP integration tests and the unreachable-collector rate limit).test/api-insight.test.tsexercises the live route throughapi-support.tsand is left to CI, along withmise exec -- npm run typecheck, the fullnpm test, andmise run docs-check.Risk notes
src/http.tsis unchanged.aggregateexport never carries it.webgrip/homelab-cluster.VIK-1897
This pull request was created by an AI agent (OpenHands) on behalf of the owner.
Follow-up after review
Review found the Median time to resolve panel (
unfold-insight.json, panelid: 1) was missing thefilter actors:>=5stage its description and the dashboard README promise, so Work Items touched by fewer than five distinct people leaked into the median — the opposite of the RFC-0001 suppression rule, and the panel most exposed on a single-user-first instance. Fixed in046e012by inserting the filter before the finalstats median(...), matching panels 2–4. Re-ran:node scripts/check.mjspasses (43 JSON files),node --test test/insight.test.ts test/config.test.ts13/13 pass, and the Ploeg verify script passes.test/api-insight.test.tsremains left to CI for the reason above.builder — round 1
acp run opened a PR for unfold: export insight events to Faro for the owner's Grafana [Ploeg verification passed]
Ploeg verification
Ploeg ran the configured checks on a fresh checkout of commit
f62fbfdd678d, the pushed head of the pull request, after the agent finished.if [ -f apps/ploeg/go.mod ]; then test -z "$(gofmt -l apps/ploeg)"; fiPosted by Ploeg for the writing Run that pushed this branch.
Ploeg usage report
Settled.
Shift totals
ploeg-8e547212c56bEvidence
Verification: not recorded — Ploeg observed no check result for the writing Run.
Posted by Ploeg and updated in place.
reviewer — round 2
Summary: Reviewed VIK-1897; verdict request_changes; outcome file written Finish with message: Review of VIK-1897 complete. Verdict: request_changes. ## What I reviewed Read the full diff (
development...agent/vik-1897, 13 files, +789/−2) against the Work Item, RFC-0001, ADR-0023, and the base-branch AGENTS.md files, plus the PR's own description context. ## Verdict rationale Met: AC1 (…Scope: the change does what the Work Item asks
The diff (
git diff development...agent/vik-1897, 13 files, +789/−2) adds the server-side insight export and nothing else. No changes underapps/ploeg/(submodule untouched, gofmt check passed), no helm/chart or homelab desired state, no changes topublic/,extensions/or any agent-configuration file (AGENTS.md,CLAUDE.md,.claude/,.agents/,.openhands/,.mcp.json,.cursorrulesare byte-identical todevelopment).Per acceptance criterion:
apps/unfold/docs/operations/live.md(the application's configuration reference for operators, where every otherUNFOLD_*/LITELLM_*variable is documented) gains the three rows with defaults and allowed values:UNFOLD_INSIGHT_EXPORT(offdefault,faro,otlp),_URL(required forfaro/otlp) and_LEVEL(aggregatedefault,events). The route contract is added toapps/unfold/docs/contracts/api.md.test/config.test.tscovers defaults, the missing-URL rejection, and both invalid-value rejections.apps/unfold/src/http.ts:144is untouched. The collectorfetchlives inapps/unfold/src/insight.ts(send(), lines 224–234), inside the server process; nothing underpublic/or the extension contacts the collector. The new route sits behind/api/(authenticated), behind the same-originX-Unfold-Requestmutation guard, and viewers get 403 (http.ts:255), consistent with the other viewer checks at 285/326/345/364.send()(insight.ts:224) wraps the fetch in try/catch with a 10 sAbortSignal.timeout, logs at most once per hour (lastErrorAtgate, insight.ts:231), andingest()never awaits delivery (void this.deliver(), insight.ts:196), so the event route cannot block.test/insight.test.tsproves both with a real unreachable collector: ingest timing < 1 s and exactly oneinsight.export_failedline for two failures.apps/unfold/ops/grafana/unfold-insight.json(uidunfold-insight) carries the stat panels and tables ofdocs/design/img/insight-tables.png(median time to resolve, tabs per decision, undo rate, regret rate, per-path table, confusion signals per screen) plus an import README, and it correctly keeps provisioning out of this repo. But one of the four stat panels does not suppress groups under 5 people.Blocking finding (must change)
F1. Stat panel "Median time to resolve" does not suppress groups under 5 people — AC4 and the panel's own description are not satisfied.
apps/unfold/ops/grafana/unfold-insight.json, panelid: 1, target expression:There is no
filter actors:>=5stage, so Work Items touched by fewer than 5 people are included in the median. The panel description claims "Cells with fewer than 5 distinct people are suppressed", andapps/unfold/ops/grafana/README.mdclaims "the stat queries with afilter actors:>=5stage" — both are untrue for this panel. Panels 2, 3 and 4 all have the filter; the two tables suppress via thefilterByValuetransformation. Consequence: on the owner's single-user-first instance this panel is exactly the one most exposed to small groups (initially one person), and it would quietly report over a suppressed population, contrary to the RFC-0001 suppression rule the dashboard exists to implement. Fix: insert| filter actors:>=5before the final stats stage, e.g.... | math decided - shown as seconds | filter actors:>=5 | stats median(seconds) as median_seconds, and keep the description as is. (If the intent was that a median over per-item values is already aggregate, then the description and README must say so instead — but AC4's wording "suppressing groups under 5 people" reads as suppression everywhere.)Checks run, and what is left to CI
$PLOEG_VERIFY_SCRIPT(gofmt onapps/ploeg): passed on this checkout.node --test test/insight.test.ts: 6/6 pass.node --test test/config.test.ts: 7/7 pass (both include the new tests; run directly with node 24, which these modules allow since they import onlynode:built-ins).node scripts/check.mjs(thecheckgate): passed (362 source modules, 43 JSON files).mise run verifyset (typecheck,test,check,design:check,icons:check,license:check, extension, helm, site,mise run docs-check) could not run in this sandbox:npm installfails — the sandbox has no registry egress (npm error network) andnode_modulesis absent — anddocs-checkadditionally aborts because theapps/ploegsubmodule is not checked out.test/api-insight.test.tstherefore could not execute here; it fails to load withCannot find package 'ws', which reproduces identically on the untouchedtest/api-oidc.test.ts, i.e. the failure is environmental, not in the new code. Gates left to CI:npm test(especiallytest/api-insight.test.ts),npm run typecheck, andmise run docs-checkafter the docs changes.I read the integration test against the code: it exercises the real route (no mocks around the code under test — the "fake collector" is a fixture
node:httpserver, which is exactly what the Work Item's verification asks for), asserts the Faro payload shape (meta.sdk.name,event.name,tenant.id, the actor hash), that an unlisted property (secret) never reaches the store or the collector, that a >32 KB batch gets 413 and a viewer 403. The accessors matchcreateApplication's real return shape ({ server, store, engine, agentHost, close }, main.ts:51).Repository rules
feat(unfold): ...) describing exactly this change. ✓off(loadConfigsetsinsight: insightSettings()which isundefinedwith no env vars), the demo config never sets it, and the deterministic-demo promise (no invented model calls or spend) is untouched. ✓webgrip/homelab-cluster; the README says so explicitly. ✓Non-blocking observations (a reviewer or follow-up may want these)
developmenthas no product-event route, table or catalogue — this PR builds the RFC-0001 route/store/rollup as the foundation its integration test needs. That is the right call for this ticket's outcome and matches RFC-0001's specified contract (50 events/32 KB/202, catalogue drop semantics, 16-char base32 HMAC actor hash, 25-month retention), but the blocked-by ticket, when it lands its client side, should reuse this route rather than ship a second one.InsightServicestoresinsight:actorKeyonce and never rotates it. Not in this ticket's acceptance criteria — fine to defer, but it should be a tracked follow-up.events-level sink drops rows from the pending queue after one failed send attempt (insight.ts:214 splices beforesend); they remain inproduct_event, andaggregatelevel is self-healing becausemaintain()re-sends the whole rollup hourly, butevents-level events that fail once are never re-exported. Acceptable under AC3's letter ("never blocks"), worth a line in the docs when the homelab follow-up lands.maintain()runs on an hourlysetIntervalviavoid this.maintain()(insight.ts:184); a synchronous SQLite throw inside it would surface as an unhandled rejection on a barevoid. This matches the codebase's existing timer convention (AgentHost.startPolling, host.ts:155), so I am not blocking on it, but a.catch()here would be cheap insurance.What a human should re-check
The LogsQL queries are labelled proposed-with-RFC-0001 and cannot be validated against the homelab's VictoriaLogs field mapping from here; they need a look once real data flows, as the dashboard's own About panel says. And CI must come back green on the PR (especially
typecheckand the fullnpm test, which this sandbox could not run) before merge.Posted by Ploeg on behalf of the reviewing agent. It could not push to this branch.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.