agoda-com / agoda-com/devfeedback-js
Ingestion API: data model changes required by the full dev-cycle events (#46)
- Vorherrschende Sprache
- TypeScript
- Sterne
- 7
- Forks
- 8
- PR-Merge-Kennzahlen
- Keine gemergten PRs in 30 T.
Beschreibung
Companion to #46, which extends the JS client from single compilations to the whole local dev cycle. Everything below is what the **ingestion API** has to change to accept and store what the client now sends. The client work is done; without these changes the new events are either rejected or land with their most useful columns dropped.
**Scope:** ingestion API + storage schema + dashboards. No client changes here.
---
## 1. A new event type and endpoint: `type: "command"`
The three existing endpoints (`/webpack`, `/vite`, `/rspack`) each accept one compilation-shaped payload. Lifecycle spans do not fit that shape, so they go to a new endpoint:
```
POST /command (client env var override: COMMAND_ENDPOINT)
```
Default URL is `http://compilation-metrics/command`, matching the existing convention.
The payload is `CommonMetadata` plus the fields below. Sample payloads for every phase are in [`examples/command.json`](https://github.com/agoda-com/devfeedback-js/blob/master/examples/command.json).
| Field | Type | Required | Notes |
| --- | --- | --- | --- |
| `type` | `"command"` | yes | discriminator |
| `phase` | enum | yes | `install` \| `codegen` \| `typecheck` \| `lint` \| `test` \| `build` \| `devserver` \| `clientready`. Only `install`, `devserver` and `clientready` are emitted today; the rest are reserved so adding them later is not another schema change. **Please store as a string, not a DB enum** — a new phase should not need a migration. |
| `command` | string | yes | e.g. `vite dev`, `rsbuild dev`, `npm install`, or the raw package manager user agent |
| `exitCode` | int | yes | `0` success, `130` SIGINT |
| `success` | bool | yes | |
| `signal` | string | no | `SIGINT` when the developer gave up waiting. **This is the field the "abandonment" analysis depends on.** |
| `errorCount` | int | no | reserved, not emitted yet |
| `timeTaken` | number | yes | inherited from `CommonMetadata`, milliseconds, the duration of the span |
### Install-specific
| Field | Type | Notes |
| --- | --- | --- |
| `packageManager` | `npm` \| `yarn` \| `pnpm` | all three are in use across our repos, so all three must be accepted |
| `packageManagerVersion` | string | |
| `coldInstall` | bool | `node_modules` absent beforehand |
| `lockfileChanged` | bool | lockfile hash before vs after |
| `measurementSource` | `preinstall` \| `postinstall` \| `npm-timing` | **how the number was obtained, and it matters for comparability.** `preinstall` is an exact span. `postinstall` is inferred from the package manager's process start time and ends when our package is linked, so it systematically undercounts. `npm-timing` is npm's own total. Do not aggregate across these without splitting by this field. |
| `npmTimers` | `map` | npm's per-phase and per-package timers, milliseconds. Unbounded key set (`build:run:install:node_modules/playwright`, `reify`, `idealTree`, …), client-capped at 500 entries. See §3. |
### Dev server / client specific
| Field | Type | Notes |
| --- | --- | --- |
| `prebundled` | bool | Vite only, **tri-state** — `true`, `false`, or absent when unknown. Absent must not be coerced to `false`; under Rolldown-based Vite the metadata file may not exist at all. |
| `domContentLoadedMs` | number | `clientready` phase |
| `firstContentfulPaintMs` | number | `clientready` phase |
---
## 2. `sessionId` on **every** event type
`CommonMetadata` gains one field, so this lands on `webpack`, `vite`, `vitehmr`, `rspack`, `rsbuild` **and** `command` payloads alike:
```
sessionId: string // uuid
```
This is the whole point of the exercise: it is what lets `install` → `devserver` → first `vitehmr` be stitched into one timeline and answer "how long from `git pull` to a working app?".
Ingestion changes needed:
- Add `sessionId` to every existing event table/schema.
- Backward compatible in both directions: old clients send no `sessionId` (accept and store null), new clients send one to old endpoints (must not be rejected as an unknown field). **Please confirm the current validation is additive-tolerant** — if unknown fields are rejected today, that is a blocker for rollout, not a nice-to-have.
- Index it. Every interesting query groups by it.
---
## 3. `npmTimers`: decision needed on storage shape
`npmTimers` is a wide, sparse, unbounded-key map — one key per npm phase plus one per package that runs an install script. Three options, roughly in order of my preference:
1. **JSON column on the install event.** Cheapest to land, query with JSON functions. Fine if these are analysed occasionally rather than charted continuously.
2. **Child table** `install_timer(event_id, timer_name, duration_ms)`. Best for "which package costs the most install time across the org, week over week", which is the question that justified collecting this at all.
3. Store only a known subset (`reify`, `idealTree`, `audit`, `build:*` aggregate).
I do **not** want option 3 — per the design conversation, prefer keeping the raw data and deciding what to aggregate later over precomputing and discovering we threw away the interesting bit. Option 2 if this is going on a dashboard, option 1 if not.
---
## 4. `spooledAt`: keep it as a first-class field
```
spooledAt?: number // epoch ms
```
Install events and SIGINT events are **spooled locally and delivered by the next dev server or build start**, because neither an install nor a dying process can afford to wait on the network. Delivery can therefore be minutes, hours, or (after a weekend offline) days after the event.
`spooledAt` is when the event was written to the local spool. Decision: **store it as a real column** rather than inferring lateness from ingest time minus `timestamp`.
- Inferring it conflates "delivered late" with "the build genuinely took that long" and with clock skew on the developer's machine.
- Dashboards should be able to filter to `spooledAt IS NULL` for real-time events without a heuristic.
- It costs one nullable bigint.
Note that for `measurementSource: "npm-timing"` events the client sets `spooledAt` to the mtime of npm's log file — i.e. when the install actually happened — since those are scraped after the fact by definition.
---
## 5. Rspack/Rsbuild endpoint correction
`rsbuild` and `rspack` events were being posted to the **webpack** endpoint, contradicting the README, which has always documented `/rspack`. The client now posts them to `/rspack`.
Ingestion side:
- Confirm `/rspack` accepts `type: "rsbuild"` as well as `type: "rspack"`.
- Anything downstream that reads Rspack data off the webpack endpoint needs repointing, and there will be a period where old client versions still send to webpack — **both paths need to work during the transition.**
- Worth a one-off check for how much existing "webpack" volume is actually rsbuild, since that skews any historical webpack numbers.
---
## Acceptance criteria
- [ ] `POST /command` accepts every phase in `examples/command.json` and rejects nothing valid.
- [ ] `sessionId` is stored on all six event types and is queryable/indexed.
- [ ] An old client (no `sessionId`, no `type: "command"`) still ingests successfully.
- [ ] A new client posting to an un-upgraded endpoint is not rejected for unknown fields.
- [ ] `measurementSource` is preserved and exposed, so install-duration aggregates can be split by it.
- [ ] `prebundled` distinguishes absent from `false`.
- [ ] `spooledAt` is stored as its own nullable column.
- [ ] `/rspack` accepts both `rspack` and `rsbuild`; the webpack path keeps working for older clients.
- [ ] `npmTimers` storage shape decided and implemented (see §3).
## Open questions for the ingestion side
1. Is current payload validation strict (rejects unknown fields) or additive-tolerant? This gates the rollout order — if strict, ingestion must ship before the client.
2. `npmTimers`: JSON column or child table (§3)?
3. Any retention or cardinality limit I should know about before we start writing one `command` event per dev server start per developer per day?
Beitragsleitfaden
Bewertung
Dieses Issue wurde noch nicht bewertet.