Commit 1b1a122
authored
feat: metric-view runtime (#474)
* chore(appkit): metric-view route skeleton + measures-only SQL (PR2 phase 1)
POST /api/analytics/metric/:key over the standard SSE envelope, SP lane only.
Synchronous config-parse registration from config/queries/metric-views.json
against the landed metricSourceSchema (single metricViews map; lane derived
from executor). Measures-only buildMetricSql (SELECT MEASURE(m) AS m FROM
<fqn> [LIMIT n]) gated by MEASURE_NAME_PATTERN + assertSafeFqn — grammar gate
only, no name allowlist. 503 METRIC_REGISTRY_LOAD_FAILED on malformed config,
404 on unknown key (generic public bodies; detail to telemetry).
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore(appkit): metric dimensions + structured filter engine (PR2 phase 2a)
GROUP BY ALL for dimensions; recursive 12-operator filter engine
(equals/notEquals/in/notIn/gt/gte/lt/lte/contains/notContains/set/notSet)
with every value bound as a :f_<idx> named parameter — never interpolated.
member/dimension gated by DIMENSION_NAME_PATTERN; AND/OR composition; depth
capped at 8, enforced twice (iterative pre-Zod preCheckFilterDepth + renderer
re-check). Static (registration-free) request schema: operator enum,
per-operator value cardinality, group/values/limit caps. No name allowlist.
timeGrain application deliberately held (parsed + grammar-gated, not yet
applied) pending the grain-target decision — lands in phase 2b.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore(appkit): apply timeGrain via explicit timeDimension (PR2 phase 2b)
Resolves the grain-target gap: PR2 drops the registry that #341 used to infer
which dimension was temporal, so the grain's target is named explicitly. New
optional `timeDimension` request field; when `timeGrain` is set, that column
renders as date_trunc('<grain>', <col>) AS <col> (grain single-quoted literal
gated by TIME_GRAIN_PATTERN; column gated by DIMENSION_NAME_PATTERN — neither
can be a bind param). Other dimensions render bare. Two 400 rules in the
request schema superRefine: timeGrain without timeDimension; timeDimension not
in dimensions. Warehouse remains the authority on column temporality.
Deviates from the PRD's "date_trunc on time-typed dimensions" wording (which
presupposed the deleted registry) — the explicit-field shape is the runtime
query shape PR2 settles; PR5's hook contract inherits the timeDimension arg.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore(appkit): metric OBO dispatch + cache-key isolation (PR2 phase 3)
Lane dispatch from the registration (metric-views.json executor), not a URL
segment: OBO-lane metrics run on-behalf-of the requesting user via asUser(req)
with a per-user cache scope; SP-lane metrics run as the app service principal
with a shared scope. Executor + key computed inside a try so a missing/
whitespace OBO identity (from asUser/resolveUserId/deriveMetricExecutorKey)
lands on the canonical 401 envelope, not an uncaught 500.
composeMetricCacheKey builds metric:{key}:{argsHash}:{executorKey} over
canonicalized args (sorted measures/dimensions, order-independent filter
fingerprint via canonicalizeFilter, grain, timeDimension, limit).
deriveMetricExecutorKey returns "sp" for SP and a sha256 of the trimmed user
identity for OBO — the raw email/principal never enters the cache layer.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore(appkit): harden metric route lookup, arg uniqueness, sort key, format
Four defensive fixes to the PR2 metric-view runtime surfaced by adversarial
review:
- Registry lookup (B): build the registry with a null prototype and gate the
route read with Object.hasOwn, so a metric key colliding with an inherited
Object.prototype member (__proto__, constructor, toString, …) can no longer
resolve to a truthy non-registration and bypass the unknown-key 404.
- Measure/dimension uniqueness (C): reject a name that repeats within measures,
within dimensions, or across both. Measures and dimensions alias to their own
name in the SELECT list (MEASURE(x) AS x, x), so a duplicate collapses to one
row-object key and silently drops a value during row materialization.
- Filter sort key (D): delimit the (member, operator) sort key with "/" instead
of a bare concatenation, so two distinct pairs cannot map to the same key and
fork the cache on semantically equal filters. Matches the delimiter
canonicalizeFilter already uses for its leaf fingerprint.
- Format (F): reject any format other than JSON_ARRAY (legacy aliases
normalized first). The metric route delivers JSON rows only at v1; an Arrow
request previously returned JSON silently instead of failing loud.
Adds 10 tests covering all four.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore(appkit): unify metric-view FQN grammar + quote at SQL interpolation
The runtime, the shared schema, and the type-generator disagreed on what a
metric-view `source` FQN may contain. The shared schema and typegen accept the
full UC quoted-identifier grammar (hyphens, non-ASCII) and typegen backtick-
quotes before interpolation; the runtime used a narrower ASCII allowlist
(assertSafeFqn) AND interpolated the FQN UNQUOTED. So a documented-legal UC
name like `prod-data.analytics.revenue` passed config + type generation but
threw (or, if the pattern were widened, emitted invalid SQL) at request time.
Converge on one grammar + one escaper:
- Move `isValidFqn` (three-part UC predicate) and `quoteFqnForSql` (backtick
escaper) into the shared zod-free leaf `metric-fqn.ts`, beside
`UC_FQN_PATTERN`. Grammar and quoting now have a single home both the
type-generator and the analytics runtime import. `describe.ts` and
`config.ts` import them back; `mv-registry.test.ts` imports the escaper from
the leaf.
- Runtime `buildMetricSql` replaces the narrow `assertSafeFqn` regex with
`quoteSafeFqn`: validate via `isValidFqn`, then interpolate the
`quoteFqnForSql`-escaped FQN. Quoting is the injection boundary, so the
runtime accepts exactly what UC (and the schema, and typegen) accept.
- `UC_THREE_PART_FQN_PATTERN` stays in `metric-source.ts` so zod still emits a
JSON-schema `pattern`; it derives from the same per-segment charset as
`isValidFqn`, so the two shapes cannot diverge.
Also folds in cache-key hardening (in-flight): salt the metric cache key with
`source` so repointing a metric key to a different FQN cannot stale-serve, and
only salt `timeDimension` when `timeGrain` is set (it has no SQL effect
otherwise). `renderDimensionClause` re-gates the grain against
TIME_GRAIN_PATTERN at its interpolation point.
Tests: requote the ~13 emitted-FROM assertions; add regression tests that a
hyphenated FQN is accepted+quoted and that a backtick-bearing segment is
neutralized by doubling.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore(appkit): load metric registry lazily with an mtime-validated cache
The metric registry memoized both success AND failure on the first
`/metric/:key` request and never re-read: editing `metric-views.json` needed a
server restart, and a transient first-request error latched a 503 forever —
unlike the sibling `.sql` query path, which re-reads `config/queries/` per
request. It was also a synchronous `readFileSync`, blocking the event loop for
every request (metric or not) under load.
Match the `.sql` path's behavior, then beat its per-request cost:
- `loadMetricRegistry` is now async (`fs.promises`) and stays a pure, stateless
parse. Metric views are already heavier than a plain query on the warehouse
side, so the SDK layer must not add a blocking read.
- New `getMetricRegistry(dir)` wraps it with a module-level cache keyed by the
queries DIR (not the plugin instance): the registry is a pure function of the
config file, warehouse-independent, so two plugins at one dir share one parse.
Each request does a single async `stat`; the read + JSON.parse + zod
validation are skipped when the file's (mtimeMs, size) signature is unchanged
— steady-state cost is below the `.sql` path (a stat, not a full read).
- Failures are NOT cached (cache populated only on a successful parse), so a
fixed config self-heals on the next request; an edit bumps mtime so a working
config hot-reloads; an absent file stays dormant (ENOENT → empty registry).
- Deletes the `metricRegistry` / `metricRegistryLoadError` fields and the
`_getMetricRegistry` memo; the route calls `getMetricRegistry` in a try/catch
→ 503 on throw.
Registry loading is now exercised through real files: `AnalyticsPlugin` takes a
test-only `queriesDir` config (documented `@internal`), so tests point the
loader at a temp dir and drive the real stat→read→cache path instead of poking
private state. Removes the `setRegistry` backdoor. Adds hot-reload, self-heal,
and mtime-cache tests.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore(appkit): quote measure/dimension identifiers + close review-round-4 gaps
Third adversarial review round (two reviewers, deduped). Highest-priority
finding: the FQN grammar drift fixed earlier was still present for
measures/dimensions/filter-members — the type-generator emits DESCRIBE column
names verbatim into the generated measureKeys/dimensionKeys unions, but the
runtime gated them on the narrow /^[a-zA-Z_][a-zA-Z0-9_]*$/ and interpolated
bare, so a UC column like `net-revenue` / `café_sales` typechecked yet 500'd at
runtime (generate-but-500, same class as the FQN bug).
A+C — quote column identifiers + validate early:
- Add `isValidColumnName` + `quoteIdentifier` to the shared zod-free leaf.
`quoteIdentifier` is the single-identifier escaper (does NOT split on `.`, so
a column literally named `net.revenue` becomes one delimited identifier);
`quoteFqnForSql` now maps it over dot-split segments. The column grammar is
the full delimited-identifier set — reject only control/newline (what cannot
be safely quoted), matching exactly what typegen can emit.
- Runtime backtick-quotes measures (MEASURE(`x`) AS `x`), dimensions, the
date_trunc column, and filter members. Quoting — not a narrow allowlist — is
the injection boundary, so an injection-shaped name is neutralized (inert
quoted column), not rejected. Row-key preservation holds: the warehouse
reports the aliased column under the unquoted name, so `{ "net-revenue": … }`.
- Move identifier validation into `validateMetricRequest` (via a refine on
measures/dimensions/timeDimension/filter.member) so a malformed identifier
returns the canonical 400 instead of failing inside the SSE execute path as a
retried 500 (finding C). The builder keeps its checks as defense-in-depth.
- Delete the now-unused MEASURE_NAME_PATTERN / DIMENSION_NAME_PATTERN.
- Fixed a regression this introduces: the filter sort key and canonicalize
fingerprint used a "/" delimiter that was safe only while members couldn't
contain "/"; now that members accept the full grammar, both JSON-encode the
tuple so distinct (member, operator[, values]) pairs can't collide and
fork/merge cache entries.
B — registry cache signature adds ctimeMs (from the same stat): a same-size,
same-mtime edit (equal-length source/executor swap on a coarse-mtime FS) now
invalidates, closing a stale-source/stale-lane serve. + same-size regression
test.
D — runtime config caps now match the type-generator: MAX_METRIC_VIEWS (200)
and per-segment length (255) enforced via the shared schema's superRefine, plus
a declarative maxLength (767) on `source` (the only cap `z.toJSONSchema` can
serialize — regenerated metric-source.schema.json). Refinements are invisible
to JSON-schema generation, so runtime + typegen stay the authoritative gates.
E — export test-only `__resetMetricRegistryCache` so cache isolation between
tests is intentional, not an accident of unique temp dirs. F — comment that a
non-ENOENT stat error is deliberately fatal-per-request (self-heals next call).
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* refactor(appkit): split metric runtime modules
Signed-off-by: Atila Fassina <atila@fassina.eu>
* refactor(appkit): derive metric operator sets from shared bases
Remove the duplicated operator lists in mv/constants.ts: SINGLE_VALUE_OPERATORS
spreads STRING_OPERATORS, and METRIC_FILTER_OPERATORS derives from the union of
SINGLE_VALUE / LIST_VALUE / NULL sets rather than re-listing all twelve names.
One source of truth per operator category; the twelve-operator tuple can no
longer drift from the per-category sets.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore: adjust comments
* docs: metric-view runtime endpoint (#484)
* docs(appkit): document the metric-view runtime endpoint
Signed-off-by: Atila Fassina <atila@fassina.eu>
* fix(analytics): reject empty and-group in metric filter validation
An empty `and` group contributes no constraint and renders to no WHERE
clause — identical SQL to omitting `filter` entirely — but it canonicalizes
to a distinct cache key (`and()` vs `_`), needlessly splitting the cache
across semantically identical requests. Reject empty groups of either kind
so request shape maps one-to-one to cache key.
Addresses GitHub Copilot review finding on PR #474.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* refactor(analytics): route metric-view config reads through AppManager
The metric-view runtime read config/queries/metric-views.json via a
bespoke fs loader that ran parallel to AppManager, the gateway the
analytics plugin already uses for its sibling .sql query files. Centralize
the read through AppManager so both file types share one dev-aware read
path and one source of truth for the queries directory.
- AppManager gains readConfigFile(fileName, req?, devFileReader?): a
narrow, domain-agnostic single-file read that applies the existing
traversal guard and switches dev-tunnel vs direct-fs like getAppQuery.
Returns null only for a genuine not-found and throws on any other error,
so callers keep the dormant (404) vs unreadable/malformed (503,
self-heals) distinction. Adds isDevRequest(req) and a read-only
queriesDir getter; the dir is now overridable only via a test-only
constructor arg.
- The metric registry reads through AppManager but keeps ownership of
freshness (fs.stat) and parse+cache. Production behavior is unchanged
(stat-signature cache preserved). A ?dev request bypasses stat+cache and
re-reads every request, so a developer's local metric-views.json edits
now hot-reload over the dev-remote tunnel the same way .sql edits do —
the first dev-remote support for metric views.
- Delete the misleading test-only IAnalyticsConfig.queriesDir field, the
plugin's _queriesDir, and the duplicated mv/constants.ts QUERIES_DIR;
the route now reads through the plugin's shared this.app. Trim an
orphaned FQN-check comment in the typegen config reader.
Resolves the PR #474 review: the AppManager-centralization thread (five
comments) and the orphaned-comment nit; the queriesDir-JSDoc finding is
resolved by deleting the field.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* fix(analytics): json-encode measures/dimensions in metric cache key
`composeMetricCacheKey` joined the sorted measures/dimensions lists with a
raw `.join(",")`. A comma is a legal identifier character —
`isValidColumnName` rejects only control characters and newlines — so
`["a,b"]` and `["a","b"]` produced the same key element while generating
different SQL, which could serve wrong cached rows. Encode each list with
`JSON.stringify` instead, matching the collision-safe encoding
`canonicalizeFilter` already uses for predicate members, so the cache key
stays one-to-one with the generated SQL.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* fix(analytics): render empty AND filter group as vacuous-true
`renderFilter` emitted `1 = 0` (vacuous-false) for an empty OR but returned
`null` for an empty AND, which the parent group drops. Dropping is only
correct at the top level: nested in an OR, an empty AND is identity-true, so
`TRUE OR P` must match all rows — but the drop collapsed it to `P` and
under-returned. Emit `1 = 1` for empty AND, parallel to the empty-OR
sentinel, so the group renders its Boolean identity element and is correct
in any position.
Unreachable through the route today (the validator rejects empty groups
with 400 before the renderer runs), but `renderFilter`'s empty-group
handling exists as the defense-in-depth fallback for a bypass, so it must
be semantically correct too.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore: adjust comments
* refactor(analytics): read metric-views.json per request, drop mtime cache
Remove the module-level mtime-validated registry memo (getMetricRegistry +
metricRegistryCache Map + __resetMetricRegistryCache) and have the metric
route call loadMetricRegistry directly, reading + parsing the config once per
request — matching the sibling `.sql` query path, which already re-reads config
every request with no cache. Per-request re-read still delivers hot-reload /
self-heal / dormant semantics; the parse cost on a <=200-entry config is
negligible next to the warehouse round-trip.
Deleting the cache removes its duplicate fs.stat ENOENT check, so absent-file
classification now flows through a single owner (readConfigFile ->
isNotFoundError) inside AppManager. isDevRequest, whose only external caller was
the deleted cache, is demoted to private; isNotFoundError (incl. its dev
message-match branch) is kept intact so dev and prod agree an absent config is
dormant.
Drops the now-unused RegistryCacheSignature type and the cache-mechanism tests
(mtime/ctime revalidation, dev-remote uncached suite); keeps the self-heal /
hot-reload route tests as the regression guarantee for per-request re-read.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* chore(appkit): relocate metric-view config to config/metric-views/definitions.json (#487)
Move the metric-view declaration file out of the queries folder into its own
config surface: config/queries/metric-views.json -> config/metric-views/definitions.json.
Metric views are independent of .sql queries, so both the type generator and
the runtime now gate on the metric-views folder rather than the queries folder:
- AppManager gains a metricViewsDir (sibling of queriesDir) and a
readMetricViewsConfig() reader; the traversal guard is generalized to any
base dir.
- type-generator/vite-plugin accept an explicit metricViewsFolder, activate on
either config surface, and watch config/metric-views/definitions.json by
directory (not bare basename).
- generate-types CLI generates when either config/queries or
config/metric-views exists.
- Runtime registry reads definitions.json via readMetricViewsConfig.
- Update dev-playground config, docs, and the JSON schema description.
- Template ships config/metric-views/definitions.json guarded by
{{if .plugins.analytics}} (same pattern as the config/queries/*.sql files),
so it only scaffolds into apps that enable the analytics feature.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
* refactor(analytics): hoist definitions.json filename to shared METRIC_CONFIG_FILE
Dedupe the four value-uses of the "definitions.json" basename onto a single
`METRIC_CONFIG_FILE` in the zod-free shared `metric-fqn` module: the analytics
runtime (mv/constants), the type-generator (config + vite-plugin), and the
generate-types CLI. Keeps one source of truth without pulling zod into the
type-generator's locked dependency graph. Addresses PR review feedback.
Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
---------
Signed-off-by: Atila Fassina <atila@fassina.eu>1 parent 40914cd commit 1b1a122
35 files changed
Lines changed: 4532 additions & 263 deletions
File tree
- apps/dev-playground
- config/metric-views
- docs
- docs
- development
- plugins
- static/schemas
- packages
- appkit/src
- app
- tests
- plugins/analytics
- mv
- tests
- type-generator
- mv-registry
- tests
- template/config/metric-views
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
| 2 | + | |
2 | 3 | | |
3 | 4 | | |
4 | 5 | | |
| |||
Lines changed: 19 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
| 34 | + | |
33 | 35 | | |
34 | 36 | | |
35 | 37 | | |
| 38 | + | |
36 | 39 | | |
37 | 40 | | |
38 | 41 | | |
| 42 | + | |
| 43 | + | |
39 | 44 | | |
40 | 45 | | |
41 | 46 | | |
42 | 47 | | |
43 | 48 | | |
| 49 | + | |
44 | 50 | | |
45 | 51 | | |
46 | 52 | | |
| 53 | + | |
47 | 54 | | |
48 | 55 | | |
49 | 56 | | |
| 57 | + | |
50 | 58 | | |
51 | 59 | | |
52 | 60 | | |
| |||
80 | 88 | | |
81 | 89 | | |
82 | 90 | | |
| 91 | + | |
| 92 | + | |
83 | 93 | | |
84 | 94 | | |
85 | 95 | | |
| 96 | + | |
| 97 | + | |
86 | 98 | | |
87 | 99 | | |
88 | 100 | | |
89 | 101 | | |
| 102 | + | |
| 103 | + | |
90 | 104 | | |
91 | 105 | | |
92 | 106 | | |
| 107 | + | |
| 108 | + | |
93 | 109 | | |
94 | 110 | | |
95 | 111 | | |
96 | 112 | | |
97 | 113 | | |
| 114 | + | |
98 | 115 | | |
99 | 116 | | |
100 | 117 | | |
| 118 | + | |
101 | 119 | | |
102 | 120 | | |
103 | 121 | | |
| 122 | + | |
104 | 123 | | |
105 | 124 | | |
106 | 125 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
89 | | - | |
| 89 | + | |
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
93 | | - | |
| 93 | + | |
94 | 94 | | |
95 | | - | |
| 95 | + | |
96 | 96 | | |
97 | 97 | | |
98 | 98 | | |
| |||
0 commit comments