Feature grafana

[Olly99999]

[Olly99999]

Summary

Extracts the .grafana namespace from TorQ's grafana.q into a standalone kdb-x module: di.grafana. The module turns a kdb+ process into a Grafana SimPod JSON datasource: it installs HTTP handlers that answer Grafana's /search, /query and connection-test requests with the JSON the plugin expects, serving the process's tables as timeseries and table panels. It satisfies the di.* module contract: dependency injection via init, a clean exported API, and no hard dependencies beyond an injected logger.

Background

TorQ's grafana.q implements the Grafana JSON-datasource adaptor as a .grafana namespace that hooks .z.pp/.z.ph through TorQ's .dotz handler manager. This PR is part of the broader TorQ → kdb-x modularisation effort: make each functional area independently loadable, testable, and injectable, removing the dependency on TorQ's global process framework.

Changes

New files

File	Description
di/grafana/grafana.q	Core implementation — init, getconfig, the .z.pp/.z.ph wrappers, and the search/query request builders
di/grafana/init.q	Module entry point — loads grafana.q and declares the export list
di/grafana/test.csv	k4unit test suite (32 tests)
di/grafana/grafana.md	Module README

Differences from TorQ original

Aspect	TorQ .grafana	di.grafana
Handlers	.dotz.set / .dotz.getcommand (TorQ handler manager)	init closure-wraps .z.pp/.z.ph directly, preserving any pre-existing handler so non-Grafana traffic falls through untouched. Follows the di.timer precedent (.z.ts); no di.handlers module exists yet to inject. Verified by a fall-through test.
Logging	None	Required injected log dependency (info/warn/error, {[ctx;msg]}). Request dispatch trapped; failures logged under the grafana context
Config	@[value; .grafana.x ;default] globals	Module-local defaults + optional config dict in init (timecol, sym, timebackdate, ticks, del)
Global state / namespace	\d .grafana	Flat module namespace; mutable state on .z.m/.z.M
Dependency validation	N/A	init validates a non-null log dep and errors with a di.grafana-prefixed message if absent
Module contract	None	kdb-x use singleton, init[deps] pattern, export: list

Exported API

grafana:use`di.grafana

grafana.init[deps]      / wire dependencies + config, install handlers; required before use
grafana.getconfig[]     / return the currently active configuration dict

Everything else is driven by inbound Grafana HTTP requests via the installed handlers, so it is intentionally not exported.

deps forms:

grafana.init[enlist[`log]!enlist logdep]              / logger only, default config
grafana.init[`log`config!(logdep;`ticks!500)]         / logger + config overrides

Query target syntax

/search emits encoded metric strings that /query parses back; del (default .) separates the arguments:

Target	Meaning
t.<table>	table panel, whole table
t.<table>.<sym>	table panel filtered to one sym
g.<table>	graph, one series per numeric column
g.<table>.<col>	graph, one series per sym for a column
o.<table>...	"other" panels (stat/gauge)
f....	target is a function call rather than a table

Test coverage

Run via k4unit: k4unit.moduletest `di.grafana

Area	Tests
Export surface	only init + getconfig exported
init — dependency validation	rejects :: and a null log dep
init — wiring	wired flag set; handlers installed
Config	defaults applied; override takes; getconfig reflects it
Parsing helpers	isfunc / istab / istype / prefix
Data fetch	finddistinctsyms, memvals, catchvals
/query table	tbfunc returns the columns/rows/table schema
/query timeseries	tsfunc returns target/datapoints numeric series
Handler routing	/search POST + GET test-connection via live handlers; non-Grafana POST falls through to the pre-existing handler

Total: 32 tests, all passing.

Notes

• di.grafana has no hard module dependencies — only the injected log dict is required.
• di.log satisfies the log contract out of the box:

logdep:`info`warn`error!(log.info;log.warn;log.error)

• The handler wrapper preserves any pre-existing .z.pp/.z.ph, so adding the module to a process that already serves HTTP does not break existing endpoints.
• The /annotations endpoint returns a "not yet implemented" marker, matching the TorQ original.

[DI-Software-Engineering]

In search, the variable rsp is built by appending string table names (string tabs), then later appending prefixed targets (prefix["t";string timetabs] etc). The initial string tabs entries have no prefix, so they appear as raw table names alongside t.*/g.* targets. This is almost certainly wrong — bare table names cannot be dispatched by the prefix-based routing in tbfunc/tsfunc, and zpp → query will fall through to annotation for them. Remove the bare string tabs line or replace it with appropriate prefixed entries.

[Olly99999]

Bare table names are dispatched, not dropped.

tbfunc has an explicit else branch that resolves an unprefixed target via value, so a raw table name is served as a whole-table table panel — intended behaviour, and a faithful port of the TorQ original:

rqt:0!value $[isfunc[rqt]&istab 2_rqt; 4_rqt;   / f.t.xxx
              isfunc rqt;                2_rqt;   / f.xxx
              istab rqt;  [rqt:`$del vs rqt; ...; rqt 1];   / t.xxx
              rqt];                               / else: bare name -> value

Also, zpp routes on the request URL (query/search), not on the target:

handler:$["query"~r 0;query;"search"~r 0;search;annotation];

so annotation is the fallback for an unknown endpoint (e.g. /annotations), never for an unknown target. A metric request arrives as query …, so it always reaches query → tbfunc/tsfunc; a target can't fall through to annotation.

Verified locally — a bare "trade" target returns a normal {columns, rows, type: "table"} response:

q).m.di.0grafana.tbfunc enlist[`targets]!enlist `target`type!("trade";"table")
"[{\"columns\":[{\"text\":\"time\",\"type\":\"string\"},...],\"rows\":[...],\"type\":\"table\"}]"

Keeping string tabs as-is. The one true edge — a bare name dropped into a graph panel — returns `$"Wrong input", which is benign (not an annotation fallthrough, not a crash) and matches the TorQ original.

[DI-Software-Engineering]

finddistinctsyms uses a functional select that references the module-level names timecol, sym, and timebackdate by value (not via .z.m.*). After init overrides these via .z.m.timecol etc., the local namespace variables are never updated, so finddistinctsyms (and other functions referencing timecol, sym, ticks, timebackdate, del by bare name) will always use the initial default values instead of the configured ones. The init function assigns deps values to .z.m.timecol, .z.m.sym, etc. but the rest of the code reads the unqualified names, which resolve to the module-level constants set at load time.

[Olly99999]

The configured values were in fact being applied (no stale defaults), but the comment correctly spotted that the bare-name reads made that non-obvious, so I've made them explicit.

Why it was already correct: there's no \d namespace switch here. The module loads into .m.di.0grafana, and .z.m is an alias for that same namespace. So the load-time default, init's .z.m.timecol: write, and a bare timecol read inside a function all resolve to the same slot (.m.di.0grafana.timecol) — init's writes were visible to the bare reads. (Same pattern di.timer uses for enabled/nextruntime.)

What changed: every config read — timecol, sym, timebackdate, ticks, del, and the wired flag — now goes through .z.m. explicitly, matching the "read mutable state via .z.m" convention and removing the ambiguity. The load-time bare defaults remain as the initial values of those .z.m slots. For example:

finddistinctsyms:{[x]
  :?[x;enlist(>;.z.m.timecol;(-;.z.p;.z.m.timebackdate));1b;{x!x}enlist .z.m.sym] .z.m.sym;
  };

Verified — config overrides flow through end-to-end (40/40 k4unit tests pass), e.g.:

q)grafana.init[`log`ticks!(logdep;5)]
q)grafana.getconfig[][`ticks]
5      / getconfig reads .z.m.ticks

[DI-Software-Engineering]

memvals uses ?[x;enlist(within;i;count[x]-ticks,0);0b;()]. The withincheck`i within count[x]-ticks,0is evaluated as`i within (count[x]-ticks),0— meaning the lower bound iscount[x]-ticksand the upper bound is0, which is always an empty range for any positive count. The intended expression is likely (count[x]-ticks),count[x]-1or?[x;();0b;()]followed by a tail take. This meansmemvals` silently returns an empty table for every in-memory table.

[Olly99999]

This parses the other way round — q is strictly right-to-left, so ,0 binds to ticks first, not to (count[x]-ticks).

count[x]-.z.m.ticks,0 evaluates as:

1. .z.m.ticks,0 → (ticks;0) (e.g. 1000 0)
2. count[x] - (ticks;0) → (count[x]-ticks; count[x])

So the range is (count-ticks; count) — lower bound count-ticks, upper bound count[x] (not 0). i within (count-ticks; count) therefore selects the last ticks rows (or all rows when ticks ≥ count). This is the unchanged TorQ original.

Worth noting your suggested (count[x]-ticks; count[x]-1) yields the same rows — the code just uses count[x] as a harmless (non-existent) upper index.

Verified — the test suite asserts 10 = count memvals on a 10-row table, which passes (it would be 1 if the upper bound were 0):

q)10 - 1000,0          / shows ,0 binds first -> upper bound is count, not 0
-990 10
q)trade:([]time:.z.p-til 10;sym:10#`a;price:10?1f)
q)count .m.di.0grafana.memvals trade
10

So memvals returns the rows, not an empty table.

[DI-Software-Engineering]

In tsfunc, args 1 is used as a table reference (passed to value) when the target is a plain g.table.col string. When isfunc is false, args is `$del vs targ — a symbol list. args 1 is thus a symbol like `trade, and value on a symbol executes it, which is fine for a global. But when isfunc is true, args is a 2-element list from (0;1+targ?del)cut targ:2_targ — a pair of strings, not symbols. args 1 would then be a string, and 0!value args 1 would attempt value on a string, which evaluates it as q code rather than looking up a table. This is an injection risk: a crafted Grafana target of the form f.<q expression> will be executed by the server.

[Olly99999]

This was a real RCE surface (inherited from the TorQ original, where f. is an "evaluate this expression" target and bare targets were valued too). Fixed, secure by default:

• Table targets (t./g./o./bare) are now resolved by name, not evaluated. They're looked up as symbols against tables[] (resolvetab); an unknown name is rejected. No target string reaches value through these paths.
• f. function targets are gated behind a new allowfunctions flag (default 0b). With it off they're rejected (and not evaluated); with it on, trusted deployments keep the eval feature. A Security section in the README documents this and the port trust boundary.

Added k4unit coverage: f. rejected-and-not-evaluated by default, unknown-table rejected, and f. evaluating only after opting in.

q)grafana.init[enlist[`log]!enlist logdep]               / allowfunctions off
q).m.di.0grafana.tbfunc enlist[`targets]!enlist `target`type!("f.1+1";"table")
'di.grafana: function targets are disabled (set allowfunctions to enable)

[DI-Software-Engineering]

In tsfunc, when isfunc is false the branch is `$del vs targ giving a symbol list; args 0 is then a symbol like `g. The subsequent check 10h=abs type args 0 tests for a char/string — a symbol has type -11h, so tyargs will be assigned via `$1# applied to a symbol, yielding `g (symbol), not `g (symbol from char). The downstream $[(2<numargs)and\g~tyargs;...]comparisons use ``g (symbol) for `tyargs` and g `` (symbol) for the literal, so they coincidentally match — but when isfuncis trueargs 0is a string (type 10h),tyargsbecomes ``$1#"g" = g ``, so both branches produce the same result. The logic is confusing and fragile but the isfuncpath hides the mismatch. Flag for clarity: the10h=abs type args 0branch should be theisfunc`-false (symbol) branch, not the string branch; the condition is inverted relative to the comment.

[Olly99999]

The condition isn't inverted, and the two branches aren't coincidental. args 0 has a different shape in the two paths, and the test normalises both to the same symbol:

• isfunc false → args is `$del vs targ → symbols, so args 0 is `g (type -11h). abs -11h = 11h, so 10h=11h is false → identity branch → tyargs = `g. (The `$1# branch is not taken for a symbol.)
• isfunc true → args is a cut of a string → char strings, so args 0 is "g…" (type 10h) → true → `$1# → `$"g" = `g.

So 10h=abs type args 0 reads as "is this the string (f.) form? then take its first char as a symbol; otherwise it's already a symbol, leave it" — correct as written, no hidden mismatch.

Agreed the point-free $[…;$1#;]args 0` is hard to read, so I've made it explicit (behaviour identical):

/ panel-type indicator (g/t/o) as a symbol - args 0 is a char string on the f.
/ path (take its first char) and already a symbol otherwise
tyargs:$[10h=abs type args 0;`$1#args 0;args 0];

[DI-Software-Engineering]

sethandlers stores the previous .z.pp/.z.ph into .z.m.prevpp/.z.m.prevph and then defines the new .z.pp/.z.ph as plain lambda closures that reference .z.m.prevpp etc. by name. However, .z.m.wired is set at the end of sethandlers, but init checks the unqualified wired variable — which is always 0b (it was set at module load and never updated via .z.m.wired). So sethandlers is called on every init invocation, re-wrapping the handlers each time and creating a chain of growing closures. The guard if[not wired;sethandlers[]] should read if[not .z.m.wired;sethandlers[]].

[Olly99999]

The guard already reads .z.m.wired on the current branch (in a follow-up commit), so the suggested change is in.

On the premise though: wired and .z.m.wired were never separate variables. .z.m is the loader's alias for the module's own namespace (.m.di.0grafana), so the load-time wired:0b and sethandlers's .z.m.wired:1b write the same slot — init's bare wired read saw the update, and sethandlers ran only once (no growing closure chain). Verifiable:

q).m.di.0grafana.wired                         / 0b  (slot from load-time `wired:0b`)
q)grafana.init[enlist[`log]!enlist logdep]
q).m.di.0grafana.wired                         / 1b  (sethandlers' .z.m.wired:1b wrote it)
q)p1:.m.di.0grafana.prevpp; grafana.init[enlist[`log]!enlist logdep]; .m.di.0grafana.prevpp~p1
1b                                             / re-init does not re-wrap

Switched it (and the other config reads) to explicit .z.m. regardless — same slot, clearer intent.

[DI-Software-Engineering]

In zpp, r:(0;n?" ")cut n:first x splits the raw HTTP request line at the first space. For a POST /query HTTP/1.1 request, r 0 will be the method (e.g. "POST"), not the path (e.g. "query"). The handler dispatch $["query"~r 0;...] then compares the method string to the path, and will never match, so every request falls through to annotation. The split should extract the path component (between the first and second spaces), not the method.

[Olly99999]

Verified against a real HTTP POST — kdb does not pass .z.pp the raw request line. It passes a 2-item list (requeststring; headers), and requeststring is the endpoint followed by the body (method and leading / already stripped), so r 0 is the path, not the method:

q)\p 8903
q).z.pp:{`CAP set x; "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\nok"}
q)/ shell: curl -s -X POST -d '{"targets":[{"target":"t.trade","type":"table"}]}' http://localhost:8903/query
q)count CAP
2
q)first CAP
"query {\"targets\":[{\"target\":\"t.trade\",\"type\":\"table\"}]}"

So (0;n?" ")cut first x gives r 0 ~ "query", $["query"~r 0;…] matches, and the request goes to query — never annotation. I've added a clarifying comment on the split, and a regression test that posts a real-shape "query {body}" (body built with .j.j) through the installed .z.pp and asserts a table response.

[DI-Software-Engineering]

In query, rqtype:raze rqt[\targets]`typeretrieves thetypefield from all targets and razes them. If multiple targets are present with different types (mixed timeserie/table), the raze produces a concatenated string and the comparisonrqtype~"timeserie"will be false, silently sending a mixed request totbfunc`. The dispatch logic does not handle multi-target requests correctly.

[Olly99999]

Multi-target requests were mishandled. The old query did raze rqt[targets]type / raze ...targetacross *all* targets, so two queries got concatenated into one nonsense target.query` now dispatches each target by its own type and merges the per-target JSON arrays.

Before — a two-query panel:

q)t:`target`type!("t.trade";"table")
q).m.di.0grafana.query enlist[`targets]!enlist (t;t)
'di.grafana: unknown table tradet        / "t.trade" + "t.trade" razed into one

After:

q).m.di.0grafana.query enlist[`targets]!enlist (t;t)
"[ {...table...}, {...table...} ]"       / each target built, then merged

Implementation notes:

• The change is confined to query; tsfunc/tbfunc and the leaf builders are untouched, and single-target output is unchanged.
• It iterates targets by index and normalises the targets field (Grafana sends an array; a lone target can arrive as an object), so list/table/dict shapes are all handled.
• Per-target arrays are merged as strings, not via a .j.k/.j.j round-trip — re-parsing would float-ify the epoch-ms longs and risk precision loss on timestamps.
• Added a k4unit test that runs a two-target request through query and asserts both tables are present in the response.

[DI-Software-Engineering]

In tbfunc, rqt:raze rqt[\targets]`targetretrieves all targets and razes them. Iftargetscontains multiple entries (Grafana can send multiple targets per query), the strings are concatenated byraze, producing a garbled target name. All of tsfuncandtbfunc` assume a single target; multi-target requests are silently corrupted rather than rejected or iterated.

[Olly99999]

Same root cause as the multi-target comment above, and fixed by the same change. query now iterates the targets and calls tbfunc/tsfunc once per target with a single-target request, so neither builder ever sees more than one target — the raze rqt[targets]target here flattens exactly that one target (intentional), it no longer concatenates across targets.

q)t:`target`type!("t.trade";"table")
q).m.di.0grafana.query enlist[`targets]!enlist (t;t)   / two targets
"[ {...table...}, {...table...} ]"                      / iterated + merged, not corrupted

tsfunc/tbfunc are intentionally single-target; the per-target dispatch and merge live in query. Covered by the new multi-target k4unit test.

[DI-Software-Engineering]

In query, rqtype:raze rqt[targets]type extracts the type field from the targets list. When multiple targets are present, raze concatenates all their type strings together (e.g. "timeserietimeserie" or "timetimeserie"), so the subsequent ~"timeserie" test will silently fail for more than one target. Each target should be dispatched individually, or the type should be taken from a single/first target rather than razing all.

[Olly99999]

This is the same multi-target issue, and the fix does exactly what you suggest: each target is dispatched individually. The rqtype:raze rqt[targets]type line is gone; query now iterates the targets and dispatches each on its own type (tsfunc for "timeserie", tbfunc otherwise), then merges the per-target results. So mixed-type and multi-target requests are handled correctly, not silently dropped.

Covered by the new multi-target k4unit test.

[DI-Software-Engineering]

query dispatches the entire rqt (with all targets) to either tsfunc or tbfunc, but those functions extract raze rqt[targets]target which collapses all targets into one string. Grafana /query requests regularly contain multiple targets; only the first (or a garbled concatenation) will be processed, silently dropping the others. The response should loop over targets.

[Olly99999]

Fixed - query now loops over the targets.

[DI-Software-Engineering]

memvals uses `i as a virtual column in a functional select: ?[x;enlist(within;i;count[x]-ticks,0);0b;()]. The lower bound count[x]-tickscan be negative when the table has fewer rows thanticks, producing an invalid range (e.g. -500to0). This will return zero rows instead of the whole table. The lower bound should be clamped to 0, e.g. 0|count[x]-ticks`.

[Olly99999]

This is the same parse as the earlier memvals note. q is right-to-left, so ,0 binds to ticks, giving the range (count-ticks; count), not (count-ticks; 0):

q)10 - 1000,0      / ,0 binds first -> (count-ticks; count)
-990 10

The upper bound is count, not 0. When count < ticks the lower bound is negative, but i within (negative; count) still selects every row (all i in 0..count-1), i.e. the whole table — the intended "fewer than ticks rows ⇒ return all" behaviour, not zero rows.

[DI-Software-Engineering]

In tsfunc, rqt:0!value args 1 evaluates args 1 as a kdb+ value. When isfunc is false the target is split by del and args is a symbol list (`$del vs targ), so args 1 is a plain symbol and value on it looks up a variable — that is acceptable. But when isfunc is true the target is a string (the function-call string is kept as a character list), so args 1 is a character-list substring that value will execute as code. A Grafana user (or attacker) controlling the target field can inject arbitrary q code. The function path should use a safe lookup rather than value on a user-supplied string.

[Olly99999]

Fixed same hardening as the f. injection in tbfunc. tsfunc's resolution is now:

coln:cols rqt:$[isfunc targ;0!evalfunc args 1;resolvetab args 1];

• Non-f. targets: args 1 is a symbol, resolved via resolvetab, which looks the name up against tables[] — never value on a string.
• f. targets: go through evalfunc, gated behind the allowfunctions flag (default 0b). With it off they're rejected; only an explicit opt-in enables expression evaluation.

So a crafted f.<q> target can't execute by default. Same mechanism as the table-builder fix; k4unit covers f. rejected-by-default, unknown-table rejected, and f. evaluating only when enabled.

[DI-Software-Engineering]

tsfunc converts the time column with rqt:@[rqt;timecol;+;.z.D] when the column type is not "p". .z.D is a date, not a timespan/timestamp. Adding a date to most time types (e.g. time which is a timespan) uses kdb+ temporal arithmetic, but the result type and correctness depend on the original column type. If the column is already a datetime or timestamp-like type other than "p", this coercion may produce unexpected results or a type error.

[Olly99999]

Verified — for the time-column types Grafana data realistically uses, this is correct and doesn't error:

q)v:09:00:00.000          / a `time` value (type -19h)
q)v + .z.D
2026.06.29D09:00:00.000000000      / correct timestamp

time + date and timespan + date both yield the right timestamp, and a timestamp column skips the branch ("p"<>...). So p/t/n are all handled correctly.

The one type where +.z.D would double-count is datetime ("z") — but that's deprecated and banned by the repo style guide, so it's out of scope. Leaving as-is.

[DI-Software-Engineering]

range:"P"$-1_'x[range]fromtodrops the last character of the Grafana ISO-8601 datetime strings (theZ) before casting to timestamp. This fragile manual trim assumes the strings always end in exactly one character to drop. A malformed or differently formatted timestamp from Grafana (e.g. with milliseconds "2024-01-01T00:00:00.000Z") will still have its last character dropped but may then fail to parse or parse incorrectly. Use a robust parse or strip the trailing Zexplicitly withssr`.

[Olly99999]

The -1_ works for the format Grafana actually sends.

Confirmed empirically: "P"$ rejects the trailing Z ("P"$"…000Z" → 0Np) but parses the ISO string fine once it's gone ("P"$"…000" → a timestamp). So stripping the Z is necessary and correct — the risk is only that blindly dropping the last char corrupts any string that doesn't end in Z. Now it strips conditionally:

range:"P"${$["Z"=last x;-1_x;x]}each x[`range]`from`to;

So a Z-terminated ISO string (what Grafana sends) parses as before, and anything else is passed to "P"$ intact rather than truncated. Also switched the timeseries tests to a real ISO-8601 range ("2000-01-01T00:00:00.000Z"…) so they exercise the actual "P"$-on-ISO path, which the previous kdb-format test strings didn't.

[DI-Software-Engineering]

buildnosym uses value each ?[x;();0b;z!z] to extract column values, where z is a two-element list (colname;msec). Selecting two columns returns a table with two columns; value eachon a table returns a list of two column vectors (one per row ofvalue). The datapoints structure Grafana expects is a list of (value;milliseconds)pairs.flip value flip(as used inothersym/graphsym) would be correct here; value each` on the table produces per-column vectors, not per-row pairs.

[Olly99999]

value each here is correct — each over a table iterates rows, not columns, so value each ?[…] applies value to each row dict and yields that row's (value; ms) pair. It's equivalent to flip value flip:

q)t:([]price:1 2 3f;msec:10 20 30)
q)(value each t) ~ flip value flip t
1b

Confirmed in the actual response — datapoints comes out as [[value, ms], …] pairs:

[{"target":"price","datapoints":[[86.71843,1782306589550],[79.02208,1782306588550],...]}]

So no change needed; nosymresponse/buildnosym already emit the per-row pair structure Grafana expects.

[DI-Software-Engineering]

In tbfunc, when the target is a plain table name (not prefixed with f. or t.), the code falls into the istab branch anyway because istab rqt is tested last and returns false, so rqt is passed through unchanged as a string. The final else branch rqt would be the raw target string. Then 0!value rqt attempts to evaluate the raw string. A target like "trade" (no prefix) would not match isfunc or istab, so rqt remains the original string and value executes it — which is another code-injection vector for non-prefixed targets. The else/fallthrough case should reject unknown prefixes rather than calling value on the raw input.

[Olly99999]

Already addressed by the same hardening as the f. injection. The bare/else branch no longer calls value on the raw target — it resolves by name:

rqt:$[isfunc rqt;0!evalfunc …;
      istab rqt;[…;resolvetab parts 1];
      resolvetab(`$rqt)];          / bare: `$rqt -> symbol -> lookup in tables[]

resolvetab turns the bare string into a symbol and rejects anything not in tables[], so "trade" resolves but "1+1" / "system …" are rejected, never executed. Added a test that proves a bare q-expression target isn't evaluated:

q).test.pwned:0b
q)@[.m.di.0grafana.tbfunc;enlist[`targets]!enlist (enlist[`target]!enlist ".test.pwned:1b");{x}]
q).test.pwned          / still 0b — the string was rejected, not run
0b

[DI-Software-Engineering]

init guards against re-installing handlers with if[not wired;sethandlers[]], reading the module-level wired variable. But sethandlers writes .z.m.wired (the namespaced copy) while the guard reads the bare wired (the top-level copy). After the first init call .z.m.wired is 1b but wired remains 0b, so sethandlers will be called on every subsequent init, re-wrapping .z.pp/.z.ph each time and creating a growing chain of nested closures.

[Olly99999]

Switched the guard to explicit .z.m.wired regardless

[DI-Software-Engineering]

In sethandlers, the .z.pp handler detects Grafana requests by checking ($"X-Grafana-Org-Id") in key last x. The kdb+ HTTP POST handler receives a two-element list: the body string and the header dictionary. If the request arrives with no headers (or xis not a two-element list),last xis the body string andkey on a string returns indices (0 1 2 ...), so the symbol will never be found — this is benign. However, if xis a plain string (e.g. non-HTTP invocation),last xis a character andkeyerrors. More critically, a plainGETfrom.z.phwhere headers are absent will cause.z.ppto error rather than fall through. The handler should guardtype last xbefore callingkey`.

[Olly99999]

Added a typed guard and factored the duplicated header check into one predicate:

isgrafana:{[x]$[99h=type h:last x;(`$"X-Grafana-Org-Id")in key h;0b]};
.z.pp:{[x]$[.z.m.isgrafana x;.z.m.zpp x;.z.m.prevpp x]};
.z.ph:{[x]$[.z.m.isgrafana x;"HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n";.z.m.prevph x]};

Now any non-dict last x (malformed or non-HTTP invocation) falls through to the pre-existing handler instead of erroring on key. Behaviour is unchanged for real requests (dict header → same check). Added a test for the malformed-arg fall-through.

[DI-Software-Engineering]

DIReview Summary

2 critical | 8 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

[DI-Software-Engineering]

All top-level variable assignments (timecol, sym, timebackdate, ticks, del, allowfunctions, types, epoch, wired) and all function definitions are in the root (.) namespace rather than inside \d .di.grafana / \d . guards. TorQ convention requires every file to open and close a named namespace. These symbols will pollute the global namespace.

[DI-Software-Engineering]

All top-level variables (timecol, sym, timebackdate, ticks, del, allowfunctions, types, epoch, wired) are declared outside any namespace. TorQ convention requires every file to open with \d .ns and close with \d .. Without this, these names pollute the root namespace and can collide with other modules.

[DI-Software-Engineering]

Single-slash / is used throughout as an inline comment character, but in q a single / at the start of a line opens a multiline comment block. All inline comments should use // to avoid silently commenting out subsequent code if a / happens to be the first non-whitespace token on a line.

[DI-Software-Engineering]

memvals uses ?[x;enlist(within;`i;count[x]-.z.m.ticks,0);0b;()]. The right-hand side of within must be a 2-element list (lo;hi), so count[x]-.z.m.ticks,0 produces a 2-element list only when ticks < count. When count x <= ticks (table has fewer rows than ticks), count[x]-ticks is zero or negative, giving a range like (-5;0) — i will never satisfy within and the result is an empty table even though all rows should be returned.

[DI-Software-Engineering]

In tsfunc, the expression if["p"<>meta[rqt][.z.m.timecol;\t];rqt:@[rqt;.z.m.timecol;+;.z.D]]adds.z.D (a date atom) to the time column to coerce it to a timestamp, but adding a date to a time gives a datetime, not a timestamp (p). This will not produce a timestamp column, and the subsequent withincomparison on the timestamp range will type-error. Use ``timestamp$rqt .z.m.timecol `` or an explicit cast instead.

[DI-Software-Engineering]

tbfunc strips the prefix from an f. target with $[istab 2_rqt;4_rqt;2_rqt]: it drops the first 2 chars (f.), checks if the remainder starts with t., then drops 4 chars from the original rqt or 2. But rqt at this point is still the full original string (e.g. "f.t.trade"). Dropping 4 chars from "f.t.trade" gives "trade", while evalfunc receives "trade" — that is actually correct for "f.t.trade". However for "f.g.trade" (not istab) it drops only 2 chars giving "g.trade", which evalfunc then calls value on, evaluating the symbol `g applied to trade rather than evaluating a function expression. The intent of f. targets is to eval arbitrary q; stripping the inner type prefix before evaluation is fragile and undocumented.

[DI-Software-Engineering]

In tsfunc, args is built as `$.z.m.del vs targ for non-f. targets, giving a symbol list. Then args 1 is passed to resolvetab which already expects a symbol — fine. But for the f. path, args is (0;1+targ?.z.m.del)cut targ — a list of strings — and args 1 (a string) is passed to evalfunc. evalfunc calls value on a string, which evaluates it as q code. If the expression in args 1 itself contains the delimiter character the cut at targ?.z.m.del (first occurrence only) will silently truncate the expression. This is a correctness issue for any f. target whose q expression contains a ..

[DI-Software-Engineering]

range is a two-element list of ISO-8601 strings that is passed directly as the within right argument. within expects a two-element list of values of the same type as the column (timestamps), not strings. The ISO-8601 strings are never converted to timestamps, so the within filter will always type-error or silently fail. Strings must be cast: e.g. "P"$each range.

[DI-Software-Engineering]

if["p"<>meta[rqt][.z.m.timecol;\t];rqt:@[rqt;.z.m.timecol;+;.z.D]]attempts to coerce non-timestamp time columns by adding.z.D (today as a date). Adding a date to an integer or other non-temporal type is a type error. The intent is presumably to promote a time (t`) column to a timestamp, but the operation is wrong for most non-timestamp types and will signal an error rather than coercing cleanly.

[DI-Software-Engineering]

The range variable shadows "P"$ in the next line: range:"P"${...}each x[\range]`from`to— the"P"$prefix IS present and produces a two-element timestamp list, so range filtering is actually typed correctly. However the variable namerangealso shadows the outer q built-inrangeif used, which is a latent hazard. (Withdraw if not a real issue.) — Actually reading again:range:"P"${$["Z"=last x;-1_x;x]}each x[`range]`from`to` does cast to timestamp. Retract the previous finding about strings — the cast is present. This finding is withdrawn.

[DI-Software-Engineering]

In graphnosym, the time column is removed with -1_coln where \number=types(0!meta rqt)`t. This removes the *last* column of the filtered list, not necessarily the time column. If the numeric columns do not include the time column at the end, a data column is silently dropped. If the time column (type p) is not numberin the types dict (it is typed asstringper thetypestable), it is not incolnat all, and-1_removes a legitimate data column. Fix: explicitly exclude the time column:coln except enlist .z.m.timecol`.

[DI-Software-Engineering]

range is built as "P"$ applied to each ISO-8601 string after stripping a trailing Z. "P"$ parses a kdb+ timestamp literal (format yyyy.mm.ddTHH:MM:SS.nnnnnnnnn), but Grafana sends ISO-8601 UTC strings like "2000-01-01T00:00:00.000Z" (dashes, not dots). "P"$ will fail to parse this format and return a null timestamp, making the within filter on line 117 silently pass or reject all rows, producing wrong query results.

[DI-Software-Engineering]

graphnosym does coln:-1_coln where \number=types(0!meta rqt)`t— it drops the last column of the numeric columns. The intent is presumably to excludemsec(which was just appended), butmsecis not incols rqtat the pointcolnis computed (it is added torqtafterwards intsfunc`). If the last numeric column happens to be a genuine data column it will be silently dropped from the graph.

[DI-Software-Engineering]

In graphsym, syms is computed by `$string ?[rqt;();1b;{x!x}enlist .z.m.sym] .z.m.sym. The functional select with 1b (distinct) returns a table; indexing it with .z.m.sym returns a column list, and `$string casts it. However the distinct-select already returns symbols, so `$string round-trips correctly only for simple symbols; symbols containing special characters will be mangled. More critically, if rqt has already been time-filtered and contains zero rows, this produces an empty list and the scan over syms silently returns (), yielding an empty JSON array with no error.

[DI-Software-Engineering]

sethandlers assigns directly to .z.pp and .z.ph (.z.* handler assignment) rather than using a TorQ-approved mechanism such as .dotz.set. Per TorQ conventions, direct .z.* assignment should be avoided in favour of .dotz.set.

[DI-Software-Engineering]

In normlog, the detection test is any \getlvl`sinks`fmts in key logdict. A plain infowarnerrordict that happens to also contain agetlvl, sinks, or fmtskey would be misidentified as a kx.log instance and its functions would be wrapped into a different calling convention, silently breaking the logger. The detection should require *all three* keys (useall`) or check a more unique key combination.

[DI-Software-Engineering]

normlog detects a kx.log instance by checking for getlvl/sinks/fmts keys, then reads logdict\infoandlogdict`warn` etc. If the passed dict does not have those keys (e.g. a partial kx.log object) the extraction will return a null/missing value without error, and the resulting wrapped function will fail at call time with a confusing error rather than a clear validation message.

[DI-Software-Engineering]

In sethandlers, .z.m.prevpp is set to {[x]} (a no-op returning the generic null) as the fallback when .z.pp is not defined. A no-op POST handler returns null/nothing to Grafana's fallthrough clients, silently dropping requests instead of returning an HTTP error response. If no pre-existing .z.pp exists, a 501 or empty response should be returned explicitly.

[DI-Software-Engineering]

DIReview Summary

4 critical | 11 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

[DI-Software-Engineering]

buildnosym calls value each ?[x;();0b;z!z] — value on a table row (a dict) returns its values as a list, not a (value;msec) datapoints pair. For Grafana's timeseries datapoints schema each entry must be (value; epoch_ms). Calling value each on each row-dict gives a list of all column values per row, not the expected (numericValue; msecValue) pair, producing malformed datapoints JSON.

[DI-Software-Engineering]

In tbfunc, when the target is an f. path, the code is 0!evalfunc $[istab 2_rqt;4_rqt;2_rqt]. Here rqt is the target string (renamed from the outer rqt), not the request dict. istab 2_rqt checks if the expression after stripping f. starts with t., which is intended to detect f.t.<expr>. But 4_rqt drops the first 4 chars of the original string, not of the already-stripped 2_rqt, so for f.t.expr the evaluated expression is t.expr (still has the t. prefix) rather than expr. The strip should be 4_rqt vs 2_rqt from the already-2_-stripped string: use s:2_rqt; evalfunc $[istab s;2_s;s].

[DI-Software-Engineering]

All top-level variables (timecol, sym, timebackdate, ticks, del, allowfunctions, types, epoch, wired) and functions are defined outside any namespace (\d is never set). Per TorQ convention every file should open with \d .ns and close with \d ., otherwise all symbols pollute the root namespace and risk clashing with user data.

[DI-Software-Engineering]

All inline comments use a single / (e.g. / grafana json datasource adaptor). In q, a / at the start of a line (or after whitespace at the start) opens a multiline comment block until the next \. Single-line comments should use //. This means the file is likely interpreted with large sections as multi-line comments depending on indentation, silently skipping code.

[DI-Software-Engineering]

DIReview Summary

4 critical | 11 warning(s) | 0 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

[DI-Software-Engineering]

All changed files were excluded by the skip blocklist — nothing to review.

[DI-Software-Engineering]

The inners filtering step (inners where 0<count each inners) drops empty per-target results silently. A target that returns an empty timeseries or table is legitimately empty (no data in the time range), and Grafana expects an entry in the array even for empty results (otherwise panel series disappear unexpectedly for targets that have no data). The empty-result case should be preserved rather than filtered out.

[DI-Software-Engineering]

Each per-target result from tsfunc/tbfunc is already a JSON string (produced by .h.hy[\json]). Stripping the first and last character with 1_-1_xassumes the string always starts with[and ends with]. If either builder ever returns an object {}` or an error string, this silently corrupts or truncates the response rather than erroring. No guard is applied before the slice.

[DI-Software-Engineering]

rqt[\targets]after.j.kon a Grafana JSON body is always a list of dicts (JSON array), so99h=type tgtswill be0band theenlistbranch is never taken. However, the real problem is thattgts iwheretgtsis a list of dicts returns the dict at indexi, but buildthen calls@[rqt;`targets;:;t]replacingtargetswith a bare dict.tsfunc/tbfuncdownstream accessrqt[`targets]as a list and then index into it — replacing it with a bare dict breaks that indexing. The per-target dispatch should pass the single target directly (not re-inserted astargets) or wrap it in enlist`.

[DI-Software-Engineering]

isgrafana uses last x to get the headers. For a normal .z.pp argument x is (requeststring;headerdict), so last x is the header dict — correct. But for .z.ph, Grafana's GET health-check, the argument shape may differ across kdb versions (it can be a single string or a 2-element list). The .z.ph handler now delegates to isgrafana which calls last x; if x is a plain string last x returns the last character (a char), 99h=type is 0b, and the guard correctly falls through — but this is a fragile coincidence rather than an intentional check. The original code also had this latent issue, but widening isgrafana to both handlers without documenting the different arg shapes increases the risk.

[DI-Software-Engineering]

DIReview Summary

1 critical | 3 warning(s) | 1 suggestion(s)

⚠️ Spec check skipped — tracker lookup failed (NO_REF_FOUND). Standards axis only.

Suggestions

• di/grafana/grafana.q:192 — The range fix conditionally strips a trailing Z with $["Z"=last x;-1_x;x]. "P"$ in kdb+ accepts ISO-8601 timestamps with a trailing Z directly (it treats Z as UTC); the strip is harmless but unnecessary and introduces a branch. If future Grafana versions use a +00:00 suffix instead of Z, the cast will fail silently and produce a null timestamp, causing an empty result rather than an error.