Mem/proxy datasources - #1785
Conversation
Every metric and log query in the app is assembled in the browser and sent straight to the Prometheus or Loki datasource, so the app has to know PromQL and LogQL, the shape of the metrics, and which of the two backing datasources holds what. Add a Go backend to the Synthetic Monitoring datasource -- the first Go code in this repository -- that lets a caller ask for a query *by name* instead. `checks_uptime` with a job, an instance and a frequency resolves to a PromQL expression against the metrics datasource; `check_error_logs` resolves to LogQL against the logs datasource. The registry in namedqueries.go is the whole point: adding a query the app can use means adding an entry there and nothing in the frontend. Queries reach their backing datasource by going back through Grafana's own /api/ds/query, which is what makes the datasource's existing authentication and any label policy attached to its token apply unchanged. The response decodes straight into backend.QueryDataResponse, so frames pass through untouched. The backend authenticates to Grafana with its own service account, issued from the new `iam` block. Note what that does *not* do: the query runs as the plugin, not as the person who asked, so a user could reach data through this that they could not query directly. A later commit closes that. Nothing in the app calls this yet. `backend: true` and the existing `routes` are independent mechanisms, so the datasource proxy that serves every fetchAPI call keeps working exactly as before. The SDK's mage targets already understand an app with a nested datasource: with no `executable` in src/plugin.json they fall back to src/datasource/plugin.json and write the binary to dist/datasource/. Build with `mage -v build:linux`, and note that `yarn build` clears dist/, so the backend has to be rebuilt after it.
Until now the backend queried Prometheus and Loki with its own service
account, so anyone who could reach the app could read data through it that
they had no permission to query directly. That is a privilege escalation,
not merely a missing feature.
Before fetching anything, ask Grafana whether the *calling* identity may
query the target datasource:
GET /api/access-control/users/permissions/search
?namespacedId=<identity>&action=datasources:query
The decision is delegated rather than reimplemented -- Grafana stays the
source of truth. Label-based access control needs no special handling: the
label policy travels with the target datasource's own token and is applied
when Grafana runs the query, so verifying "this identity may query
datasource D" and then querying D returns exactly what they would have got
querying D themselves.
The caller comes from PluginContext.User, which Grafana sets rather than
the client. It carries a login but no id, and the permissions API wants a
typed identifier, so the login is resolved to either `user:<id>` or
`service-account:<id>`. That needs two lookups: /api/org/users/lookup does
not return service accounts, so resolving only users would refuse a service
account token for the wrong reason -- "I cannot tell who you are" rather
than "you may not query this". Both lookups are substring matches returning
lists, so the login is matched exactly rather than taking the first result.
The three additions to `iam` are the permissions these endpoints need and
the plugin's service account did not have.
It fails closed. No caller on the request, no service account credential,
or a failed lookup all deny the query; falling back to the plugin's own
identity is the thing being avoided.
Verified end to end both ways. A user granted datasources:query on the
Synthetic Monitoring datasource but not on Prometheus is refused with 403
and no frames, and granting the Prometheus permission -- same user, same
query -- returns data. A service account with the permission is likewise
allowed, and the permission check is made against its service-account
identity.
One caveat worth knowing: the permissions search is cached and eventually
consistent, and unlike /api/access-control/user/permissions it takes no
reloadcache parameter. Grants take a few seconds to appear; more
importantly, so do revocations. Checking permissions out of band cannot be
as immediate as having Grafana enforce them on the query path.
Port every builder in src/queries/ into the registry, taking it from two
named queries to nineteen. Each entry names the frontend builder it came
from, and a test pins the expression it produces against that original so
the two cannot drift apart unnoticed.
Two things made this possible. Grafana's macros are interpolated by the
Prometheus datasource itself, so expressions can keep $__rate_interval and
$__range and have them resolved from the interval and time range the backend
sends -- verified against a running Prometheus, which reported executing
[4m0s] rather than the macro. Subquery forms such as [$__range:] work
unchanged too, which the three avg_request_* breakdowns rely on.
Porting these as written would have introduced PromQL injection. Several of
the builders splice caller-supplied strings in as bare identifiers --
${metric}, ${label}, ${labelName} -- and others as unescaped label values.
In the browser that was harmless because the caller was the app itself; here
the parameters arrive in a request body, so a metric of `up} or secret{`
would have executed PromQL of the caller's choosing and made serving queries
by name decorative. promql.go therefore validates identifiers against
Prometheus's grammar, keeps avg_quantile_web_vital restricted to the same six
metrics the frontend's union type allowed, and escapes label values. The
escaping also fixes a latent bug: a job name containing a double quote
produces a broken query in the frontend today.
All nineteen were run against a real Prometheus and Loki and returned
frames. The tests cover every expression, the parameter validation, and the
injection attempts, plus a guard that fails if a query is registered without
an expression test.
The frontend still calls only checks_uptime and check_error_logs; the other
seventeen are reachable but unused, and src/queries/ is untouched, so those
queries are currently defined in both places. Wiring the call sites over is
separate work, and it is where the risk is, since each one changes a
rendered panel.
Not ported: the inline expressions in the scenes panels. Eleven of them use
$Filters, an ad-hoc filters variable that Scenes resolves into label matchers
in the browser, so those need the resolved matcher passing in as a parameter
-- a design decision rather than a port.
Move two queries off the frontend and onto the named queries the backend now serves, so the app stops carrying the expressions and stops needing to know which datasource holds the data. The two are deliberately different shapes, because they exercise different paths: useCheckUptimeSuccessRate posts to /api/ds/query directly. That path does not involve the datasource class at all -- Grafana routes on the datasource uid -- so it would work without any of the changes to DataSource.ts. The error logs panel is the one that needs them. Scenes resolves the datasource and calls query() on it, so SMDataSource becomes a DataSourceWithBackend and query() dispatches: named types go to the backend, and probes, checks and traceroute keep being answered in the browser as before. Without that dispatch a named query would match no branch and return an empty result with no error, which is a blank panel rather than a visible failure -- so this commit and that panel belong together. applyTemplateVariables is required rather than cosmetic. DataSourceWithBackend interpolates each target through it, and the panel passes $job, $instance and $probe; without it the backend would receive the literal variable names. It reuses the multi-value probe handling already in interpolateVariablesInQueries. Changing query() to return an Observable is safe: nothing in the app calls SMDataSource.query(), only Grafana and Scenes do. The PromQL for uptime now exists in two places -- Go for the named query, and queries/uptime.ts for the UptimeStatViz panel, which still builds it in the browser. The backend test pins the ported expression against the frontend original so the two cannot drift silently, but moving that panel over is left for later.
The panel menu builds its Explore link from the queries in the request and
the datasource they were sent to. That works only while every panel sends
PromQL to Prometheus directly.
A panel asking the Synthetic Monitoring datasource for a named query sends
parameters instead of an expression, and the SM datasource's query editor
only understands probes, checks and traceroute -- so Explore would open
against the wrong datasource with nothing in it, and "Copy JSON" would be
equally useless. Nothing would report an error; the menu item would simply
stop being useful.
Take the expression from the frame metadata instead. Prometheus and Loki
both report what they actually ran in meta.executedQueryString, so it is
available for any named query without the backend having to echo it back:
Prometheus: Expr: max by () (max_over_time(probe_success{...}[60s]))
Step: 1m0s
Loki: Expr: {probe=~".*", job="test"} | logfmt
Panels that still send `expr` keep interpolating it themselves, so this is
behaviour-preserving on its own -- no caller passes the new
exploreDatasourceUid yet, and every existing panel takes the same path it
did before. That is deliberate: it can be verified before any panel moves.
The parsing is unit-tested against strings captured from a running Grafana,
including Loki's single-line form and Prometheus's trailing Step, because a
silent failure here degrades a feature rather than breaking a build.
Move the three hooks that fetch metrics outside a Scenes panel onto named queries: probe execution and failure rates, the unique check configs, and the max probe duration. The expressions they used are already in the backend registry, so this removes the second copy for those four queries. The TimepointExplorer hooks already consumed data frames, so they are a straight swap of queryMimir for queryNamedQuery. useProbeExecutionStats needed more than a swap. It read `d.metric.probe` and `d.value[1]` from the Prometheus HTTP API's instant response; a named query returns frames, one per series, with the probe label on the value field. So the lookup is now by field label, reading field 1. Its two exported functions keep their signatures, so components/ProbeCheckExecutionStats.tsx is unchanged. That reshaping is not covered by the test suite -- the component test mocks the hook outright -- so it was checked against the datasource directly. The same probe and the same value, 0.008414719251306898, come back through the old Prometheus proxy path and the new named query. Also fills in QueryType with the rest of the registry's names and the parameters they take. Four names are deliberately absent: avg_request_latency, avg_request_success_rate, avg_request_expected_response and scripted_http_requests_error_rate. The panels using them read fields called `name`, `method` and `Value #<refId>` -- Grafana's joined-table shape, which the Prometheus *frontend* datasource produces. Routing those through the backend yields `Time` and a metric-named field instead, so `findValueByName` would match nothing and the tables would render empty without erroring. They keep querying Prometheus directly.
Switch the two check-dashboard stat panels onto named queries. They send the name and the check's parameters; the range flag, the interval and the legend now come from the backend along with the expression, so the panels no longer set them either. Both panels keep useMetricsDS, but only to tell the panel menu where "Explore" should go: the data comes from the Synthetic Monitoring datasource and Explore has to open against Prometheus, which is what the preceding commit made possible. Adds NamedQueryRequest for the shape a panel hands to the query runner -- the name plus its parameters, with no panel concerns in it. One behaviour change worth noting: the uptime panel no longer requests exemplars. It is a stat panel with graphMode None, so there was nothing to draw them on, but the request did previously ask for them.
The four panels shared between the browser and scripted dashboards took a built query object as a prop, so each dashboard assembled PromQL and passed it down. They now take a NamedQueryRequest -- a name and its parameters -- and the dashboards choose the name. That is what the shared panels were really parameterised over: the browser dashboard wants browser_data_sent where the scripted one wants scripted_data_sent, and the two duration and target panels differ only by which metric they aggregate. The metric is still the caller's choice, but it is now validated as a metric name in the backend rather than pasted into an expression here. The range and instant flags and the legend format come from the backend with the expression, so the panels no longer derive them from the query object. Each keeps useMetricsDS solely to point "Explore" at Prometheus.
The last of the panels whose queries the backend already serves. Both spread a built query into their targets, which also meant spreading a queryType of 'range'; the name now determines that, along with the expression. The metric was previously constrained by a union type in TypeScript, which said nothing once the value reached a query string. The backend keeps the same six web vital metrics as an allow-list and rejects anything else, so the constraint now holds where the expression is built.
Twelve builders have no callers left, so the expressions they held exist only in the backend registry. Removing them is the point of the migration: while both copies existed, either could drift and the Go tests would only have pinned the port, not the behaviour. getCheckProbeAvgDuration goes too. It had no frontend caller before this work started; it is in the backend registry, so the capability is not lost. Also drops the MSW branch that stubbed the probe execution and failure rate queries on the Prometheus proxy. Those hooks now go through /api/ds/query, so the branch could never match, and it was the last thing importing the builders. Four builders stay, with queries.types.ts for them: avgRequestLatency, avgRequestSuccessRate, avgRequestExpectedResponse and scriptedHTTPRequestsErrorRate. The two Scripted table panels read fields called `name`, `method` and `Value #<refId>`, which is Grafana's joined-table shape produced by the Prometheus frontend datasource. Asking the backend for those queries returns `Time` and a metric-named field instead, so findValueByName would match nothing and the tables would render empty without erroring. Migrating them needs that shape reproduced deliberately, which is separate work.
Script size changes
Totals
|
There was a problem hiding this comment.
Works well — tested locally (dem-dev) and on a deployed Cloud stack. Haven't tried it with different RBAC roles yet, that needs a look.
What I care about is how gcx and other third-party callers consume this long term. Right now gcx rebuilds these queries itself (status.go), so this PR kills real duplication — but if gcx just calls the named queries it ends up hardcoding the 19 names and the params fields instead.
schemabuilder from the plugin-sdk would avoid that: one test generates schema/v0alpha1/query.types.json from the Go types and the file ships with the plugin (grafana/grafana#122557). No feature toggles needed to read it — /public/plugins/grafana-testdata-datasource/schema/v0alpha1/query.types.json returns a QueryTypeDefinitionList on my stack today. gcx could read that and hardcode nothing.
The catch is params. With all 19 queries sharing one struct the generated schema is 19 copies of "12 optional fields", which tells a caller nothing — checks_uptime needing job/instance/frequency is only expressed in Go. There are 8 real shapes in there (base check(), +frequency, +metric, +label, +labelName/value/method, +unsuccessfulOnly, the web vital one, none) so splitting isn't much work.
My other issue is that we are 100% not going to get the API right from the start and I haven't worked out a clear route that supports v1/named_query and v2/named_query simultaneously. Grafana's documentation seems to just imply 'get it right at the start and make everything backwards compatible...' 😅😅😅
Forgot to mention all the comments below are just reminders for myself that our agentic friends flagged as I was working through the PR.
| target: targetMetrics, | ||
| build: func(params) (built, error) { | ||
| return built{ | ||
| expr: fmt.Sprintf(`sum(rate(probe_all_success_count[%s])) by (probe)`, defaultQueryFromTime), |
There was a problem hiding this comment.
These two hardcode defaultQueryFromTime (3h) instead of using the request's time range, so a caller asking for the last hour gets a 3-hour rate regardless. Mirroring the frontend constant makes sense for the panels that use it, but as a backend API this is now a query endpoint that accepts a time range and silently ignores it for two of its 19 queries — not something a caller can work out from the outside.
Take the window from the request range, or put it in the name (probe_execution_rate_3h) so it's explicit?
| IntervalMs: q.Interval.Milliseconds(), | ||
| MaxDataPoints: maxDataPoints(b, q), | ||
| }) | ||
| ranges[ds] = q.TimeRange |
There was a problem hiding this comment.
This is assigned per query inside the loop, so when several queries group onto the same target the last one's range wins silently. The comment above acknowledges the SDK-versus-/api/ds/query mismatch, but last-wins means two queries with different ranges on the same target return one answer computed over the wrong window, with nothing signalling it.
/api/ds/query genuinely does impose one range per request, so the options are probably to split the group per distinct range or reject mixed ranges outright. Either beats silent.
|
|
||
| // POC: Grafana's own address. Inside the dev container this is Grafana itself. | ||
| // GrafanaConfig.AppURL() is preferred and tried first; this is the fallback. | ||
| const devAppURL = "http://localhost:3000" |
There was a problem hiding this comment.
Security — localhost fallback
This should hard-fail outside dev rather than silently falling back to localhost.
| "iam": { | ||
| "permissions": [ | ||
| { "action": "datasources:read", "scope": "datasources:*" }, | ||
| { "action": "datasources:query", "scope": "datasources:*" }, |
There was a problem hiding this comment.
Security — iam blast radius
datasources:query on datasources:* means the plugin SA can query any datasource in the org; authorize() is the only guard. Worth calling out in the PR description.
Can we scope iam to the linked metrics/logs UIDs, or is * required by the platform? Either way, authz is load-bearing — suggest non-admin test coverage before prod rollout.
| } | ||
| } | ||
|
|
||
| return devAppURL |
There was a problem hiding this comment.
Security — localhost fallback (see also devAppURL const ~line 19)
Should hard-fail outside dev rather than silently falling back to localhost. Cloud should always provide AppURL() via plugin context.
| * so the expression runs to the end of the string or to the next `Detail:` line. | ||
| * | ||
| * Exported for testing. | ||
| */ |
There was a problem hiding this comment.
Explore link fragility
Smoke-tested: works — opens Prometheus Explore with the recovered expression. Parsing executedQueryString via regex is fragile if Prom/Loki metadata format changes. Accepted trade-off for now; worth a code comment noting the dependency.
| // Fails closed: any inability to establish permission -- no user on the request, | ||
| // no plugin credential, a lookup failure -- denies the query. Falling back to the | ||
| // plugin's own identity is what we are trying to avoid. | ||
| func (a *authorizer) authorize(ctx context.Context, appURL, token string, user *backend.User, ds linkedDatasource) error { |
There was a problem hiding this comment.
Authz not exercised locally
Anonymous Admin in dev makes authz look like a no-op; anonymous sessions without a user identity caused stat panels to show N/A ("no user on the request").
Before Cloud prod: test with a non-admin user who can use SM but cannot query Prometheus directly.
|
|
||
| return built{ | ||
| expr: fmt.Sprintf( | ||
| `group by(frequency, config_version) (max_over_time(sm_check_info{job="%s", instance="%s", probe=~"%s"}[$__range]))`, |
There was a problem hiding this comment.
Macro dependency
Registry entries emit $__range / $__rate_interval — fine when Grafana executes the query, but expressions aren't self-contained if a consumer receives the string (gcx, CallResource). Flag for external-client work.
| `plugin.json`. That requires two settings: | ||
|
|
||
| ```ini | ||
| [auth] |
There was a problem hiding this comment.
Local dev gap
Documents service-account settings here, but dev/custom.ini doesn't set managed_service_accounts_enabled or externalServiceAccounts. Consider adding to dev Docker env so yarn server works for named queries out of the box.
| // | ||
| // Queries are grouped by backing datasource so that a request mixing metrics and | ||
| // logs costs one round trip per datasource rather than one per query. | ||
| func (d *Datasource) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { |
There was a problem hiding this comment.
Performance note
Named queries: browser → backend → authz (up to 3 API calls) → /api/ds/query → Prom/Loki. Worth measuring on Cloud with TimepointExplorer. Authz caching per user/datasource is an obvious follow-up if latency shows up.
registry entries shared one 12-field params struct regardless of what each query actually took, so nothing outside the build closures could tell checks_uptime needed a frequency or avg_quantile_web_vital needed a metric and quantile. Replace it with 8 typed shapes (queryshapes.go) covering all 19 entries, and generate a schemabuilder-based query-type schema (schema_test.go) from them, served at /public/plugins/synthetic-monitoring-datasource/schema/v0alpha1/query.types.json. TestSchemaCoversRegistry pins registry and the schema mapping to the same set of names so they can't silently drift apart. No behavior changes: resolve()'s signature is unchanged, and the existing namedqueries_test.go suite (which only exercises resolve() through JSON strings) passes without modification.
Check out the latest push I made, which I codified this recommendation. I've attached a sample response. I think that helps define the shape of the query, but an agent likely needs one a set of examples as well. The one drag I see with this is that(at least as far as I can tell), we'll need to commit the generated json blob as code(🤮). This isn't a huge deal breaker, and maybe even something we could generate on the fly before building the bundle. I've been working through using claude + gcx to gather the examples + query type definitions and use it to build queries. I'll try and post a video tomorrow demo'ing it. |
3 participants
Draft PR to see the diff and as a place for discussion of the pros and cons of turning into a backend plugin.