Repository navigation
feat(dashboards): chart Google Cloud metrics live with PromQL - #1480
JeremyFunk wants to merge 23 commits into
Conversation
A widget data source kind `promql` stores a PromQL query, optional projects and a legend template. The browser and the share API both run it through GcpPromQlService as a range query with the chart's bucket as step, and draw the answer as wide chart rows. The editor gets a Google Cloud source tab with a metric search that writes a starter query.
…/gcp-promql-widget
… feat/gcp-promql-widget
Share links hold a query variable a PromQL tile reads to the free-text charset and count failed scopes instead of naming them. Variables reach PromQL under a `promql` param key, with All as a regex alternation. Browser and share path share the request and row helpers; a request the author can fix is a fix_request error. The editor marks a stale preview, shows search progress and failures, and reduces stat and gauge tiles by the rail's Aggregate.
… feat/gcp-promql-widget
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Maple review🔴 Confidence 4/10 · risky as written Adds a Findings🟠 Warning · F1 ·
|
| Issue | Service | Occurrences | File |
|---|---|---|---|
| @maple/web/errors/WarehouseInvalidInputError | maple-web | 3 | apps/web/src/lib/services/atoms/warehouse-query-atoms.ts |
After this merges, Maple checks whether they stop.
Production traffic of the changed files (last 7 days)
| File | Calls/day | Busiest operations |
|---|---|---|
packages/backend/src/services/dashboards/DashboardWidgetDataService.ts |
3 | DashboardWidgetDataService.resolve 3/day, 23.8% err |
Telemetry this change adds and removes (3)
- ➕ span name
Integrations.getGcpPromQlChart·apps/web/src/api/warehouse/gcp-promql.ts:24 - ➕ span name
Integrations.getGcpMetricSearch·apps/web/src/api/warehouse/gcp-promql.ts:44 - ➕ span name
Integrations.getGcpMetricDescriptor·apps/web/src/api/warehouse/gcp-promql.ts:58
Observability coverage: 2 of 2 changes observable
| Change | Kind | Observable | Evidence |
|---|---|---|---|
getGcpPromQlChart / metric search / describe server functions (apps/web) |
server function + outbound read | yes | runWarehouseQuery wraps each in Effect.withSpan(operation) (apps/web/src/api/warehouse/effect-utils.ts:236) |
| Share PromQL tile execution (apps/api share widgetData) | inbound endpoint + outbound Google read | yes | resolve annotates maple.widget.* (DashboardWidgetDataService.ts:249); GcpPromQlService.queryRange annotates orgId (GcpPromQlService.ts:400) |
7c45912 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
| }) | ||
| }) | ||
|
|
||
| const promQlQueries = document.widgets.flatMap((widget) => { |
There was a problem hiding this comment.
Warning
promQlQueries skips sparkline sources, so a variable can escape the guard
F1 · Warning · security
The share executes widget.display?.sparkline?.dataSource when the request asks for source: "sparkline" (DashboardWidgetDataService.ts:267, and the source is whatever the unauthenticated caller sent at share.http.ts:349), but this list walks widget.dataSource alone, so a variable referenced only by a sparkline's PromQL never enters promQlNames and promQlUnsafe stays false (share-variables.ts:150). The viewer's value then needs only to be one of the options the warehouse listed, and a telemetry value containing a quote reaches the promql branch of interpolateString unescaped — plainly, or as every option of an All joined with | — rewriting the author's matcher on a public link. Collect both data sources here.
Build the list from `widget.dataSource` and `widget.display?.sparkline?.dataSource` alike before mapping out the query strings, and add a sparkline-only reference to the `share-variables.test.ts` case.
🤖 Prompt to fix with an AI agent
In `apps/api/src/routes/v2/share.http.ts:305-313`: `promQlQueries` skips sparkline sources, so a variable can escape the guard.
The share executes `widget.display?.sparkline?.dataSource` when the request asks for `source: "sparkline"` (`DashboardWidgetDataService.ts:267`, and the source is whatever the unauthenticated caller sent at `share.http.ts:349`), but this list walks `widget.dataSource` alone, so a variable referenced only by a sparkline's PromQL never enters `promQlNames` and `promQlUnsafe` stays false (`share-variables.ts:150`). The viewer's value then needs only to be one of the options the warehouse listed, and a telemetry value containing a quote reaches the `promql` branch of `interpolateString` unescaped — plainly, or as every option of an All joined with `|` — rewriting the author's matcher on a public link. Collect both data sources here.
Suggested fix: Build the list from `widget.dataSource` and `widget.display?.sparkline?.dataSource` alike before mapping out the query strings, and add a sparkline-only reference to the `share-variables.test.ts` case.
Verify the problem exists at that location before changing it, and keep the fix to those lines.
| ...(Array.isArray(dataSource.projects) | ||
| ? { projects: dataSource.projects.filter(isString) } | ||
| : undefined), |
There was a problem hiding this comment.
Warning
dataSourcePromQl forwards an empty projects list every read rejects
F2 · Warning · correctness
projects: [] is the documented "every project" form and V2PromQlDataSource stores it, but PromQlProjects requires at least one id (packages/domain/src/http/gcp-promql.ts:31), so promQlRangeInput (packages/query-engine/src/gcp-promql.ts:28) hands the empty array to GcpPromQlRangeRequest and the decode fails: the tile shows a request error on every view, signed in and on a share, instead of reading all connected projects. The editor already omits the field when nothing is typed (widget-query-builder-page.tsx:177).
| ...(Array.isArray(dataSource.projects) | |
| ? { projects: dataSource.projects.filter(isString) } | |
| : undefined), | |
| export const dataSourcePromQl = (dataSource: unknown): PromQlDataSource | null => { | |
| if (!isRecord(dataSource) || dataSource.kind !== "promql") return null | |
| // An empty list means "every project", but `PromQlProjects` wants at least one id. | |
| const projects = Array.isArray(dataSource.projects) ? dataSource.projects.filter(isString) : [] | |
| return { | |
| query: isString(dataSource.query) ? dataSource.query : "", | |
| ...(projects.length > 0 ? { projects } : undefined), | |
| ...(isString(dataSource.legend) ? { legend: dataSource.legend } : undefined), | |
| } | |
| } |
🤖 Prompt to fix with an AI agent
In `packages/widgets/src/dashboard/access.ts:187-189`: `dataSourcePromQl` forwards an empty `projects` list every read rejects.
`projects: []` is the documented "every project" form and `V2PromQlDataSource` stores it, but `PromQlProjects` requires at least one id (`packages/domain/src/http/gcp-promql.ts:31`), so `promQlRangeInput` (`packages/query-engine/src/gcp-promql.ts:28`) hands the empty array to `GcpPromQlRangeRequest` and the decode fails: the tile shows a request error on every view, signed in and on a share, instead of reading all connected projects. The editor already omits the field when nothing is typed (`widget-query-builder-page.tsx:177`).
Replace those lines with:
export const dataSourcePromQl = (dataSource: unknown): PromQlDataSource | null => {
if (!isRecord(dataSource) || dataSource.kind !== "promql") return null
// An empty list means "every project", but `PromQlProjects` wants at least one id.
const projects = Array.isArray(dataSource.projects) ? dataSource.projects.filter(isString) : []
return {
query: isString(dataSource.query) ? dataSource.query : "",
...(projects.length > 0 ? { projects } : undefined),
...(isString(dataSource.legend) ? { legend: dataSource.legend } : undefined),
}
}
Verify the problem exists at that location before changing it, and keep the fix to those lines.
| const existingTransform = dataSourceTransform(widget.dataSource) | ||
| const transform = | ||
| widgetTypeByVisualization(state.visualization)?.isScalar === true | ||
| ? { ...existingTransform, reduceToValue: { field: "value", aggregate: state.statAggregate } } |
There was a problem hiding this comment.
Warning
Stat/gauge PromQL tiles reduce a value field the rows never carry
F3 · Warning · correctness
promQlChartRows emits wide rows { bucket, [series]: value } (packages/query-engine/src/gcp-promql.ts:68), so reduceToValue.field: "value" never matches and applyTransform falls back to the first numeric column of rows[0] (use-widget-data.ts:206): a stat over a query with several series sums, averages, mins or maxes one arbitrary series, and count returns the number of time buckets rather than the series count. Reduce the row's value columns for a PromQL source instead of naming a value field.
Emit a `value` column from `promQlChartRows` (or reshape with a `flattenSeries`-style transform) so the rail's Aggregate applies to every series rather than the first one.
🤖 Prompt to fix with an AI agent
In `apps/web/src/components/dashboard-builder/config/widget-query-builder-page.tsx:172`: Stat/gauge PromQL tiles reduce a `value` field the rows never carry.
`promQlChartRows` emits wide rows `{ bucket, [series]: value }` (`packages/query-engine/src/gcp-promql.ts:68`), so `reduceToValue.field: "value"` never matches and `applyTransform` falls back to the first numeric column of `rows[0]` (`use-widget-data.ts:206`): a stat over a query with several series sums, averages, mins or maxes one arbitrary series, and `count` returns the number of time buckets rather than the series count. Reduce the row's value columns for a PromQL source instead of naming a `value` field.
Suggested fix: Emit a `value` column from `promQlChartRows` (or reshape with a `flattenSeries`-style transform) so the rail's Aggregate applies to every series rather than the first one.
Verify the problem exists at that location before changing it, and keep the fix to those lines.
Starter queries group by project_id, and default series keys keep metrics_scope, which tells a scope's projects apart behind one answering project.
Maple review🟡 Confidence 5/10 · needs attention Dashboard tiles can chart a live PromQL query over Google Cloud metrics, with a Google Cloud source tab, a share path that runs only the stored query, and starter queries now grouped per project. The three earlier warnings (F1, F2, F3) still hold at this head; the
Before merge
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 3 of 3 changes observable
|
… feat/gcp-promql-widget
Maple review🔴 Confidence 4/10 · risky as written Adds a
Findings🟠 Warning · F4 · An All selection splices a bare
|
| Issue | Service | Occurrences | File |
|---|---|---|---|
| @maple/web/errors/WarehouseInvalidInputError | maple-web | 3 | apps/web/src/lib/services/atoms/warehouse-query-atoms.ts |
After this merges, Maple checks whether they stop.
Production traffic of the changed files (last 7 days)
| File | Calls/day | Busiest operations |
|---|---|---|
packages/backend/src/services/dashboards/DashboardWidgetDataService.ts |
3 | DashboardWidgetDataService.resolve 3/day, 23.8% err |
Telemetry this change adds and removes (3)
- ➕ span name
Integrations.getGcpPromQlChart·apps/web/src/api/warehouse/gcp-promql.ts:24 - ➕ span name
Integrations.getGcpMetricSearch·apps/web/src/api/warehouse/gcp-promql.ts:44 - ➕ span name
Integrations.getGcpMetricDescriptor·apps/web/src/api/warehouse/gcp-promql.ts:58
What was checked
- Share redaction drops
query,projectsandlegend(packages/widgets/src/dashboard/redact.ts:90, pinned by promql-source.test.ts:60) - Share variable values stay inside
TEXTBOX_ALLOWED, All included (packages/backend/src/services/dashboards/share-variables.ts:150) promQlRangeInputtrims the query and floors the step at 60 s (packages/query-engine/src/gcp-promql.ts:24)
Observability coverage: 3 of 3 changes observable
| Change | Kind | Observable | Evidence |
|---|---|---|---|
| gcp_promql range read from a dashboard tile / share | outbound call | yes | Effect.fn("GcpPromQlService.queryRange") with orgId annotation and maple.gcp.projects (packages/backend/src/services/integrations/GcpPromQlService.ts:495); resolve annotates org/widget/source (DashboardWidgetDataService.ts:249) |
| browser server function getGcpPromQlChart / getGcpMetricSearch | outbound call | yes | runWarehouseQuery("gcpPromQlQueryRange") wraps the /internal client call (apps/web/src/api/warehouse/gcp-promql.ts:34) |
| new tile failure path (Google refusal, partial answer) | error path | yes | asWidgetOutcomeFailure(request.widgetId, "promql") logs the cause and maps to a tagged failure (DashboardWidgetDataService.ts:445) |
24cd5c7 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
| return replaceRefs(input, (name) => { | ||
| const variable = values[name] | ||
| if (variable === undefined) return null | ||
| return variable.isAll ? variable.options.join("|") : variable.value |
There was a problem hiding this comment.
Warning
An All selection splices a bare a|b alternation into the matcher
F4 · Warning · correctness
variable.options.join("|") is substituted at the reference's position, so a matcher that is more than the bare variable describes the wrong set: with options ["dev","prod"], {env=~"api-$env"} becomes api-dev|prod, which (PromQL anchors =~ over the whole expression) also matches a series named prod. An option holding a regex metacharacter — a hostname or a project id with a ., a value with + — matches values it does not name, on every member view as well as the share. Group and escape each alternative the way the sql branch escapes its literals, e.g. "(" + options.map(escapePromQlRegex).join("|") + ")".
Add a small PromQL-regex escaper next to `escapeClickHouseString` usage and return `(${variable.options.map(escapePromQlRegex).join("|")})` for an All, so the alternation is grouped at the reference's position and each option is matched literally. Grafana's Prometheus datasource does the same.
🤖 Prompt to fix with an AI agent
In `packages/query-engine/src/dashboard-variables/interpolate.ts:106`: An All selection splices a bare `a|b` alternation into the matcher.
`variable.options.join("|")` is substituted at the reference's position, so a matcher that is more than the bare variable describes the wrong set: with options `["dev","prod"]`, `{env=~"api-$env"}` becomes `api-dev|prod`, which (PromQL anchors `=~` over the whole expression) also matches a series named `prod`. An option holding a regex metacharacter — a hostname or a project id with a `.`, a value with `+` — matches values it does not name, on every member view as well as the share. Group and escape each alternative the way the `sql` branch escapes its literals, e.g. `"(" + options.map(escapePromQlRegex).join("|") + ")"`.
Suggested fix: Add a small PromQL-regex escaper next to `escapeClickHouseString` usage and return `(${variable.options.map(escapePromQlRegex).join("|")})` for an All, so the alternation is grouped at the reference's position and each option is matched literally. Grafana's Prometheus datasource does the same.
Verify the problem exists at that location before changing it, and keep the fix to those lines.
Maple review🔴 Confidence 2/10 · do not merge Adds a
Before merge
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 3 of 3 changes observable
|
Maple review🔴 Confidence 2/10 · do not merge Adds a
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 2 of 2 changes observable
|
Maple review🔴 Confidence 2/10 · do not merge Adds a Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 2 of 2 changes observable
|
Maple review🔴 Confidence 3/10 · risky as written Adds a live
Before merge
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 2 of 2 changes observable
|
Maple review🔴 Confidence 2/10 · do not merge Adds a
Before merge
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 2 of 2 changes observable
|
Maple review🔴 Confidence 3/10 · risky as written Adds a Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
Observability coverage: 2 of 2 changes observable
|
Maple review🔴 Confidence 3/10 · risky as written Charts Google Cloud metrics live by adding a Before merge
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 2 of 2 changes observable
|
Maple review🔴 Confidence 3/10 · risky as written Resolves the bundle-budget merge conflicts and raises
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
|
# Conflicts: # apps/web/perf/check-bundle-budget.ts
Maple review🔴 Confidence 2/10 · do not merge Adds a
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 0 of 1 changes observable
|
# Conflicts: # apps/web/perf/check-bundle-budget.ts
Maple review🔴 Confidence 3/10 · risky as written Adds a live PromQL data source that charts Google Cloud Monitoring metrics on dashboard tiles, plus an editor tab, share-path redaction and docs. The only change since the last review is the web bundle budget; the four open warnings are unchanged, so this is not ready to merge. The open
Still open from earlier reviews
Production impactOpen errors in the changed files
After this merges, Maple checks whether they stop. Production traffic of the changed files (last 7 days)
Telemetry this change adds and removes (3)
What was checked
Observability coverage: 2 of 2 changes observable
|
This branch has not been deployed
1 participant
Stacked on #1477 (
feat/gcp-live-query-access), which sits on #1475 (feat/gcp-promql-query). #1478 (MCP tools) sits on this branch. Merge the stack bottom-up.What
Dashboard widgets can chart a PromQL query over the organization's Google Cloud Monitoring metrics. The query is read live from Google through #1475's
GcpPromQlService; nothing is ingested.promql(packages/widgets/src/dashboard/v3/data-source.ts):{ kind: "promql", query, projects?, legend?, transform? }. The public v2 dashboards API mirrors it (V2PromQlDataSource, query capped at 4,000 characters like the read).toWidgetRequestlowers the source to endpointgcp_promqlwith params{ promql, projects?, legend? }.planWidgetRequestthen fills the window and substitutes dashboard variables. A tile with no query stays atpromql_query_empty: it renders "No query yet" and sends no request. The browser's server function (apps/web/src/api/warehouse/gcp-promql.ts, loaded on first use) and the share resolver (DashboardWidgetDataService) share the same pure helpers inpackages/query-engine/src/gcp-promql.ts. They build the range request with step = the chart bucket for the window, never under 60 s, and turn the answer into chart rows.{ bucket, [series]: value }. A series is named by the legend template ({{label}}). Without a legend it is named by its labels, minus__name__, and minusproject_idwhen a single project answered. A missing point leaves its key out of that row.a|b) for a=~matcher.gcpMetricSearch. Picking a metric replaces the query with a starter query fromgcpMetricDescribe: a rate for DELTA/CUMULATIVE, avg for GAUGE,histogram_quantile(0.95, …_bucket)for distributions. It uses the UTF-8 selector form and adds amonitored_resourcematcher when the metric has several resource types.noticeon the ready state).queryvariable that a PromQL tile references must now also pass the free-text charset, including every option of an All. Its options are telemetry values, and PromQL takes them unescaped.redactForSharedropsquery,projectsandlegend.integrations/gcp.md, and the new source tab indashboards/build-dashboards.md.Decisions taken without the owner
computeBucketSecondsForRange, ~100 points), floored at 60 s. It does not follow the tile-width model, so the step does not change with tile width and Google's edge cache sees fewer distinct keys.describe_dashboard_schematakes its kinds fromWIDGET_DATA_SOURCE_KINDS, so it already listspromql. feat(mcp): query Google Cloud metrics live with PromQL #1478 has the tools.Verify
Scoped vitest only; CI runs the rest:
packages/widgets:src/dashboard/promql-source.test.ts, plus the package'stypecheckpackages/domain:src/http/v2/dashboard-widget-parity.test.tspackages/query-engine:src/gcp-promql.test.ts,src/widget-request.test.ts,src/dashboard-variables/interpolate.test.tspackages/backend:DashboardWidgetDataService.test.ts,share-variables.test.ts,widget-data-parity.test.tsapps/web:data-source-registry.test.ts,share-support.test.ts,use-widget-data.test.ts,dashboard-summary.test.tsbun run ios:openapi:check: up to dateNot verified without a live system; this needs a visual pass at the top of the stack:
Need help on this PR? Tag
@codesmith-botwith what you need.