Skip to content

feat(gcp): Google Cloud dashboard template and docs page - #1335

Merged
JeremyFunk merged 69 commits into
feat/gcp-integration-cardfrom
feat/gcp-dashboard-template
Oct 16, 2026
Merged

JeremyFunk merged 69 commits into
feat/gcp-integration-cardfrom
feat/gcp-dashboard-template

Conversation

@JeremyFunk

@JeremyFunk JeremyFunk commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on the Google Cloud integration card PR. Merge order: bottom up.

What

PR E of the Google Cloud integration: the prebuilt dashboard template and the customer docs pages. No backend behaviour change, no database change.

  • packages/backend/src/dashboard-templates/infrastructure/gcp.ts: template gcp ("Google Cloud"), registered in dashboard-templates/index.ts.
  • packages/backend/src/dashboard-templates/infrastructure/gcp.test.ts: guard that the template charts only what the poller stores.
  • apps/landing/src/content/docs/integrations/gcp.md: the docs page (/docs/integrations/gcp).
  • apps/landing/src/content/docs/integrations/gcp-opentelemetry.md: "Google Cloud with OpenTelemetry" (/docs/integrations/gcp-opentelemetry), listed after Google Cloud under Integrations.
  • apps/web/src/lib/docs.ts, integration-catalog.tsx and gcp-integration-card.tsx: the catalog entry and the configuration dialog's Roles if you aren't an Owner, Troubleshooting and Write a filter of your own links point at the Google Cloud page. The log filter acknowledgement and the filter's lines link to the new page (docs.test.ts resolves both against the landing content).
  • template-icons.ts: the Google Cloud mark in the template picker.
  • getting-started/introduction.md and dashboards/build-dashboards.md: one mention each, where the other integrations and templates are listed.

Dashboard template

38 charts in one Services section, one tab per service, two charts per row within a tab. Filters in the toolbar: Project (cloud.account.id), Region (cloud.region), Workload (service.name), each with All. The optional project_id parameter sets the Project filter's starting value.

Tab Charts
Cloud Run Requests, 5xx responses, request latency p95, active instances, CPU utilization p95, memory utilization p95, each by service
Cloud Functions (1st gen) Executions, execution time p95, active instances, peak memory p95, each by function
GKE CPU limit utilization, memory limit utilization (non-evictable), restarts, memory in use (non-evictable), each by container; node CPU and node memory utilization by cluster
Compute Engine CPU utilization, memory in use (E2), network received, network sent, disk read, disk written, each by instance
Cloud SQL CPU, memory and disk utilization, connections, disk operations, replica lag, each by instance
Pub/Sub Unacknowledged messages, oldest unacknowledged age, sent, acknowledged, dead-lettered, each by subscription; publish requests by topic
Load balancing Requests by URL map, requests by response class, total latency p95, backend latency p95

Each widget takes its metric type, aggregation, chart preset and display unit from GCP_METRIC_GROUPS: a sum metric is charted as a delta sum, everything else as an averaged gauge, and the unit maps s, By and 1 to duration_s, bytes and percent.

A chart filters on a variable only where its series carry the attribute. Region skips Pub/Sub and load balancers (global resources). Workload applies only to charts grouped by workload. A variable on All drops its clause.

The test runs one series per curated metric through mapGcpTimeSeries and fails when a widget names a metric that is not in the table, a metric type that differs from the rows the mapping produces, an attr.* or resource.* key that is not on those rows, a quantile value the mapping does not emit, a distribution charted without a quantile filter, sum on a gauge, or an attribute group-by on a metric whose service.name varies. A second test fails when a filter lists values from a metric that is not a stored gauge carrying its attribute, or a widget filters on a variable its own series don't carry.

Metric query variables

Query variables could only list values from traces and logs. A metrics-only integration never writes those.

  • New variable source { kind: "metric", metricType, metricNames, scope: "metric" | "resource", attributeKey }: a data-point or resource attribute's values on the named metrics.
  • Engine: attributeValues takes metricNames. Metric-scoped discovery reads ResourceAttributes for the resource scope. It reads the raw metric table, like the query-builder autocomplete.
  • Wired through the option query (options.ts, used by the board and the share path), the web provider, getMetricAttributeValues, the variables dialog ("Metric attribute…" / "Metric resource attribute…") and the v2 API schema.
  • Template previews show the root canvas, then each section's first tab: a widget's layout.y is relative to its tab. The layout test checks rows per tab.
  • /lab/gcp-dashboard: the built template on the read-only canvas, with fixture series that follow the filters, and the variables dialog. gcp-dashboard-fixture.json is gcpTemplate.build({}) (web can't import @maple/backend); the regenerate command is in the lab file.

Docs page outline

Intro (what a connection collects), Prerequisites (roles per scope, where to find the IDs), Connect (the steps of the Connect dialog and its apply stage, the terminal output of a first run, how long first data takes, what the wrapper lines of the pasted text do, the secret), Change what a connection collects (Configure, a table of when each change takes effect in Maple and in Google Cloud, the confirmation, the band that waits for the script), Log filter (the recommended filter, including GKE container logs after a confirmation, keeping a sink's filter, and a custom filter), Verify (the numbers at the top of the page, one table of statuses per capability, the bands and their buttons, the connection labels), Permissions (every API and role the script uses, and the predefined roles that cover the script without Owner), Collected data (logs: the recommended filter as a table of what it excludes and why, GKE container logs among them, with the filter text below it, the reserved maple-setup log, service.name rule, attributes, severity; metrics: all 46 names per service, value conventions, attributes; resources), Google Cloud costs, Limits, Troubleshooting (In Cloud Shell, Log forwarding, Metrics and resources, each keyed on the sentence the script or the card shows; Domain restricted sharing, the anchor the script links to), Disconnect (cleanup first), Next steps.

Google Cloud with OpenTelemetry

For customers whose workloads also send telemetry over OTLP. The Google Cloud page points to it near its top.

  1. Intro and one table: per signal, what comes from the workloads over OpenTelemetry and what comes from the Google Cloud connection.
  2. Recommended setup, as numbered steps per runtime: GKE (SDK or the Maple Kubernetes collector, connection on the recommended filter), Cloud Run (SDK, connection, and leaving the container output out of the filter when the service exports logs over OpenTelemetry), Compute Engine.
  3. GKE container logs: why the recommended filter leaves them out (duplicates, no trace link on Google's copy, volume) and when to include them.
  4. Change the filter: the three options, editing LOG_FILTER by hand, and the clause that also leaves out Cloud Run container output.
  5. Instrument a Cloud Run service: service name, sampling, flushing and log emission, each with its reason, a Node.js setup, the start command, the two environment variables, a log record, the stdout alternative, and two limits.
  6. Instrumentation guides.

The Cloud Run section comes from a Node.js service that was instrumented and measured against a connected Google Cloud project: the server span of every measured request had the trace ID of Google's request log for it. The general Node.js guide is not changed here.

The terminal transcript in Connect is the output of the script as this stack renders it, run against a stubbed gcloud. Status labels and failure sentences are quoted from the card and the services.

The Infrastructure page is not described; its PR adds that section.

After the final live run the page was corrected against what the stack ships: the script's four closing lines, a wrong ID fixed by removing the connection and connecting the right one, the 10-minute rule for reads that fail, both sign-in messages, messages by scope, Setup running, where to read a kept log filter, and one to two minutes for a run that removes the sink.

The reworked integration page

The page no longer has switches or a Show setup script button (see the integration card PR below). Both docs pages follow it:

  • Connect: Connect Google Cloud or Add connection opens the dialog; Create and continue leads into Apply in Google Cloud with its three steps.
  • Change what a connection collects: Configure, then Save and continue. A table says when each change takes effect: off acts in Maple at once and in Google Cloud when the script runs; on starts once the script has run; back on resumes at once when the resources still exist.
  • The log filter is chosen in the first stage of the dialog; Continue to the script is how the script is reached again.
  • Verify: the status tables use the card's labels (Not set up for a capability, Setup pending for a connection); a table lists the three bands and their buttons.
  • Disconnect is in the card's menu; the cleanup panel is dismissed with a confirmation.
  • Quoted script output and failure texts are unchanged.

Decisions

  • Charts by workload use service.name. A chart grouped by an attribute keeps one service's value per group in the query engine, so attribute group-bys are used only where service.name is the constant gcp/<resource type> (Cloud SQL, Pub/Sub, load balancers, GKE nodes). Cloud Run errors are therefore "5xx responses by service" with a class filter, and Cloud Functions executions are by function, not by status. This can be relaxed once the query engine adds services up under an attribute group-by (tracked as a separate follow-up).
  • Gauges use avg: sum on a per-minute gauge adds up every minute of a bucket. A workload running in several regions or projects under one name reads as the average of them.
  • Percentile charts filter one quantile (0.95) and group by workload.
  • Label values used in filters were checked against Google's metric reference: state = active (Cloud Run instance count), memory_type = non-evictable (GKE), response_code_class = 5xx (Cloud Run SLI examples use 1xx, 2xx, 4xx).
  • The docs name the predefined roles that replace Owner (Service Usage Admin, Project IAM Admin, Pub/Sub Admin, Service Account Admin on the host project; Logs Configuration Writer and the IAM admin role on the scope). They are derived from the permissions the script's own check asks for; a run with exactly these roles against Google Cloud has not been done.
  • Domain restricted sharing is described without a value to allow: the remedy is the one the script prints (lift the policy for the host project while the script runs).
  • Docs use "connection" (the card's word) throughout, and name the two things a connection collects instead of calling them switches.
  • GKE container logs are left out by the recommended filter because a workload that sends its logs over OpenTelemetry would have each line stored twice. The page names the filter the way the card does ("recommended filter"); the API value stays default.
  • The clause that leaves out Cloud Run container output uses log_id("run.googleapis.com/stdout") and log_id("run.googleapis.com/stderr"), the log IDs Google's documentation lists for Cloud Run next to run.googleapis.com/requests. It was not run against a sink.
  • The new page's sidebar label is Google Cloud + OpenTelemetry: the full title is cut off in the sidebar.
  • The costs section carries no prices or estimates, only who is billed for what and links to Google's pricing pages. "Cloud Asset Inventory searches are free of charge" is from Google's Cloud Asset Inventory pricing page.

Not verified

  • The docs page was rendered from the landing dev server: at 1440px and 390px, every in-page link has a target, and the anchors the app and the script link to exist (#google-cloud-costs, #troubleshooting, #domain-restricted-sharing, and #gke-container-logs on the new page). Both pages were rendered again for the log filter round. The template picker was rendered with the new mark.
  • The template charts have not run against real Google Cloud data. The three filters' option queries were run against local Tinybird holding the 2026-10-08 e2e data and returned the project, us-central1/europe-west1 and the Cloud Run, GKE and VM workloads. The lab was checked with Playwright: tabs, Region narrowing GKE but not Pub/Sub, the dialog's metric source.
  • That Cloud Monitoring reads are billed to the host project follows from the poller sending them with the host project as quota project; the poller PR lists it as unverified without a live project.

Review notes

  • stored?.kind in gcp.ts: a misspelled metric name still builds, so the test reports it by name instead of a TypeError from build.
  • "p95 by service" is the average over the bucket of each series' p95, as in the Cloudflare template's percentile charts. The titles stay short.
  • The shared per-tick request and time budgets are not listed under Limits: a customer sees the same note and the same remedy as for the per-read caps.
  • Integrations → Google Cloud keeps the arrow the other integration pages use for menu paths.

Summary by CodeRabbit

  • New Features
    • Added Google Cloud infrastructure dashboards for Cloud Run, GKE, Compute Engine, Cloud SQL, Pub/Sub, and other services.
    • Added guides for connecting Google Cloud, collecting metrics and logs, and configuring OpenTelemetry for GKE, Cloud Run, and Compute Engine.
    • Added Google Cloud to the infrastructure dashboard templates and integration catalog, with links to setup and troubleshooting guidance.
  • Updates
    • Dashboard templates now show a consistent “no metrics” status when required telemetry is unavailable.

Service map: Google Cloud as a platform

The gcpDetector setup in gcp-opentelemetry.md sets faas.name on Cloud Run and Cloud Functions, so the service map drew those services as AWS Lambda (and GKE as plain Kubernetes).

  • cloud.provider = gcp classifies as a new gcp platform, checked before the Lambda and Kubernetes rules (query-engine.http.ts and the local-ui copy).
  • Node shows the Google Cloud mark with a "Google Cloud" tooltip; colour-by-platform has a Google Cloud colour and legend entry (legend uses labels instead of capitalize).
  • GoogleCloudIcon moved from apps/web into @maple/ui icons, re-exported from @/components/icons.
  • Rendered on /lab/bench/service-map (colour by Platform).

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

This change adds Google Cloud integration and OpenTelemetry guides, links them from integration and documentation surfaces, and adds a Google Cloud infrastructure dashboard template. It also updates dashboard template readiness descriptions and display behavior to use metric availability rather than connection-specific missing messages.

Changes: Google Cloud integration and dashboard

Layer / File(s) Summary
Integration setup and documentation
apps/landing/src/content/docs/integrations/gcp.md, apps/web/src/components/integrations/gcp-configure.tsx, apps/web/src/components/integrations/integration-catalog.tsx, apps/web/src/lib/docs.ts, apps/landing/src/content/docs/getting-started/introduction.md
Documents connection setup, collection, permissions, troubleshooting, and cleanup. The integration catalog, configuration UI, and introductory guide link to the documentation.
OpenTelemetry guidance
apps/landing/src/content/docs/integrations/gcp-opentelemetry.md, apps/web/src/components/integrations/gcp-configure.tsx
Documents signal ownership and setup guidance for GKE, Cloud Run, and Compute Engine, including a Node.js example. The configuration UI links to related guidance.
Dashboard template and discovery
packages/backend/src/dashboard-templates/infrastructure/gcp.ts, packages/backend/src/dashboard-templates/index.ts, packages/backend/src/dashboard-templates/infrastructure/gcp.test.ts, apps/web/src/components/dashboard-builder/templates/template-icons.ts, apps/landing/src/content/docs/dashboards/build-dashboards.md
Adds charts for Google Cloud workloads and resources, with optional project filtering and metric-based aggregation and display settings. Registers the template, assigns its icon, tests its queries, and lists it in dashboard documentation.

Dashboard template readiness

Layer / File(s) Summary
Requirement readiness contract and display
packages/backend/src/dashboard-templates/types.ts, packages/domain/src/http/dashboards.ts, packages/domain/src/http/v2/dashboards.ts, packages/backend/src/dashboard-templates/infrastructure/cloudflare.ts, packages/backend/src/dashboard-templates/infrastructure/planetscale.ts, packages/backend/src/dashboard-templates/index.test.ts, apps/api/src/routes/v2/dashboards.http.test.ts, apps/web/src/components/dashboard-builder/templates/template-detail-panel.tsx, apps/web/src/components/dashboard-builder/templates/use-template-readiness.ts, apps/web/src/components/dashboard-builder/templates/use-template-readiness.test.ts
Documents metric-prefix readiness for integration requirements and removes the “not connected” missing label from Cloudflare and PlanetScale templates. Unready requirements display “no matching metrics,” and related tests check derived or explicit metric missing labels.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Merge Risk: 🟡 Moderate · up to 4fb72

The Google Cloud dashboard may blend metrics from different projects or clusters that share a resource name, and the Cloud Run sample may lose telemetry under default billing. Fix the chart grouping and the sample before merging; the other open documentation and display notes are smaller.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 15 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main changes: adding a Google Cloud dashboard template and documentation page.
Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 15 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@maple-review-bot

maple-review-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 10/10 · safe to merge
quality 100/100 · no findings · tests covered · risk low

Adds the prebuilt Google Cloud dashboard template and its docs page, with a test that checks every chart against what the Cloud Monitoring poller stores. No backend behaviour change; safe to merge.

  • gcpTemplate adds 24 GCP widgets, metric type/unit taken from GCP_METRIC_GROUPS
  • Optional project_id parameter scopes every widget with resource.cloud.account.id
  • gcp.test.ts replays each curated metric through mapGcpTimeSeries and checks every chart against those rows
  • Docs catalog entries point /docs/integrations/gcp at the new landing page
What was checked
  • Every metric name, kind and unit in gcp.ts matches GCP_METRIC_GROUPS (packages/domain/src/gcp-metrics.ts:100-314), including quantile and "1" utilization metrics
  • Attribute/resource group-bys (attr.response_code_class, resource.gcp.resource.labels.database_id, resource.k8s.cluster.name) exist on the rows the poller mapping produces (mapping.ts:190-200)
  • Layout and query-draft shape are covered by the existing index.test.ts / query-specs.test.ts guards, and docs.test.ts resolves the new docs slug

e56b40a · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 10/10 · safe to merge
Docs-only edit reviewed at the head; every checkable claim it makes (/v2/integrations/gcp, 46 metric queries) matches the code and tests.
quality 100/100 · no findings · tests not needed · risk low

The only change since the last review is copy in the Google Cloud docs page (Google price figures dropped from the costs section). The page's factual claims still line up with the code, so it is safe to merge.

What was checked
  • /v2/integrations/gcp exists as a registered v2 route (packages/domain/src/http/v2/integrations-gcp.ts:360, docs/api-v2.md:175)
  • The "46 metric queries" claims match GCP_METRIC_GROUPS and the service tests (GcpMetricsService.test.ts:21, GcpConnectorService.test.ts:227)
  • Docs links resolve to landing content: /docs/integrations/gcp in apps/web/src/lib/docs.ts:35 and introduction.md:28

d176c85 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

…exts, where the secret shows

From an end-to-end run against a real project: metrics need billing on the host
project, a project's first sink can drop entries for about ten minutes, the push
endpoint is readable in the subscription and the audit log, and the dashboard
template's project field sits under Parameters.
@maple-review-bot

maple-review-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
The template's metric, attribute and unit names rest on GCP_METRIC_GROUPS and the poller mapping, which I could not read: the sandbox read/grep tools returned 500, so the guard test is the only chec…
quality 100/100 · no findings · tests covered · risk low

Adds the gcp dashboard template (24 charts, optional project_id scope), its guard test, the Google Cloud docs page and the catalog/docs links. Contained, test-pinned, no backend or schema change; safe to merge.

  • gcpTemplate charts 24 Cloud Monitoring metrics, type and unit taken from GCP_METRIC_GROUPS
  • gcp.test.ts fails on any metric, attribute, quantile or aggregation the poller does not store
  • docsUrl("gcp") links /docs/integrations/gcp from the catalog entry
What was checked
  • Every added widget names a metric in the curated table, pinned by gcp.test.ts:48
  • Attribute and resource group-bys are asserted against mapped rows (gcp.test.ts:59)
  • escapeMetricStringLiteral is applied to the project_id parameter before it reaches the where clause (gcp.ts:216)

8bdbb82 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
Every widget metric, type, unit, attribute key and layout is pinned by the new test and the existing template guards; no runtime path changes.
quality 100/100 · no findings · tests covered · risk low

Adds the gcp dashboard template (24 widgets, optional project_id scoping), registers it, and links the new Google Cloud docs page from the integration catalog. Data-only plus a docs link; safe to merge.

  • gcpTemplate charts 24 Cloud Monitoring metrics, taking each type, aggregation and unit from GCP_METRIC_GROUPS
  • Attribute group-bys are limited to metrics whose service.name is the constant gcp/<resource type>
  • project_id scopes every widget through resource.cloud.account.id, escaped by escapeMetricStringLiteral
  • Catalog entry for gcp gains docsUrl("gcp"), resolving to the new /docs/integrations/gcp page
What was checked
  • All 24 metric names, kinds and units checked against GCP_METRIC_GROUPS (packages/domain/src/gcp-metrics.ts:100) and the poller's mapGcpTimeSeries; every name, type and attr.*/resource.* key …
  • Layout satisfies the row guard: two w=6 h=6 widgets per row, consecutive y (dashboard-templates/index.test.ts:27)
  • isMonotonic never reaches the query SQL (query-engine/src/query-builder/model.ts), so leaving it unset on sum metrics cannot misread the delta sums

6ebeb6a · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
Template semantics were pinned by the new test against the poller's own mapping; the stacked PR's GcpMetricsService/mapping.ts files are not in this diff and were read only at the base.
quality 100/100 · no findings · tests covered · risk low

Adds a Google Cloud dashboard template (24 metric widgets over the Cloud Monitoring poller's curated metrics), registers it, and links the new /docs/integrations/gcp page from the web integration catalog. No backend behavior change; safe to merge.

  • gcpTemplate builds 24 Cloud Run/Functions/GKE/Compute/SQL/Pub-Sub/LB widgets, one per GCP_METRIC_GROUPS metric
  • Widget type, aggregation and unit derive from the metric table (sum → delta sum, else averaged gauge)
  • Optional project_id scopes every widget with resource.cloud.account.id
  • docsUrl("gcp") added to the web docs table and the GCP catalog entry
What was checked
  • Every widget metric name matches a GCP_METRIC_GROUPS entry (packages/domain/src/gcp-metrics.ts:100-314)
  • Where-clauses lower warning-free: exactly one attr. filter and up to five resource. filters (packages/query-engine/src/query-builder/model.ts:983-1018)
  • project_id is escaped with escapeMetricStringLiteral before interpolation (gcp.ts:216); attr.quantile spells the 0.95 the mapping emits (mapping.ts:208)

8f85997 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
Every charted metric, label, unit and quantile is pinned against GCP_METRIC_GROUPS and mapGcpTimeSeries by the new guard test, and the engine's one-attr-filter limit is respected per widget.
quality 100/100 · no findings · tests covered · risk low

Adds the gcp dashboard template (24 widgets driven by the poller's GCP_METRIC_GROUPS), its guard test, the template-picker mark and the docs path, with the Google Cloud docs page. Purely declarative: no data path, schema or endpoint changes, and it is safe to merge.

  • gcpTemplate charts 24 GKE/Cloud Run/SQL/PubSub/LB metrics, deriving metric type, aggregation and unit from GCP_METRIC_GROUPS
  • Guard test gcp.test.ts runs every curated metric through mapGcpTimeSeries and fails on an uncurated metric, attribute, quantile or aggregation
  • project_id parameter scopes every widget with resource.cloud.account.id
  • gcp registered in DASHBOARD_TEMPLATES, TEMPLATE_ICONS and DOCS
What was checked
  • Metric names, attr.*/resource.* keys and quantile values checked against GCP_METRIC_GROUPS and gcpResourceAttributes (packages/domain/src/gcp-metrics.ts, mapping.ts:32)
  • Each widget carries at most one attr.* filter plus one resource.* filter, inside the engine's single-attr-filter limit (query-builder/model.ts:990)
  • Attribute group-bys only on metrics whose service.name is the constant gcp/<resource type>, so the engine's per-group service value cannot under-count

8728f18 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
Template behaviour is pinned by gcp.test.ts against the poller's own mapping; the project-id parameter reaches the where clause only through escapeMetricStringLiteral.
quality 100/100 · no findings · tests covered · risk low

Adds the gcp dashboard template (24 metric widgets), its docs page, and the catalog/docs link so the Google Cloud card points at /docs/integrations/gcp. No backend behaviour or schema change; safe to merge.

  • gcpTemplate registers a gcp template of 24 widgets built from GCP_METRIC_GROUPS
  • Widget metric type, aggregation and display unit follow the stored metric's kind and unit
  • gcp-integration-card and the catalog entry now use docsUrl("gcp") instead of a hardcoded URL
What was checked
  • The optional project_id value is escaped by escapeMetricStringLiteral (helpers.ts:150) before it enters the where clause
  • docsUrl("gcp") resolves to the added apps/landing/src/content/docs/integrations/gcp.md path covered by docs.test.ts
  • Charted metrics, attribute and resource keys and quantile values were verified against the poller mapping by the new test

4a4cbd6 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
gcp.test.ts plus query-specs.test.ts lower every new widget through the real mapping and the query engine, and each GCP metric name, unit and group-by is in gcp-metrics.ts.
quality 100/100 · no findings · tests covered · risk low

Adds the prebuilt "Google Cloud" dashboard template, its /docs/integrations/gcp page and the catalog/card links to it. The 24 widgets match the metrics the poller stores, and the docs anchors they link to exist. Safe to merge.

  • gcpTemplate charts 24 GCP metrics, type, aggregation and unit read from GCP_METRIC_GROUPS
  • gcp.test.ts fails a widget whose metric, type, attribute, quantile or group-by the mapping does not produce
  • The gcp integration card and catalog entries now link into /docs/integrations/gcp
What was checked
  • Every one of the 24 metric names resolves in GCP_METRIC_GROUPS, and its sum/gauge/quantiles kind matches the aggregation the widget sets
  • resource.cloud.account.id and resource.gcp.resource.labels.<key> group-bys are keys the GCP mapping stores (gcp/mapping.ts); the query-spec guard rejects any unsupported key as a warning
  • The three files changed since 4a4cbd6 (gcp-integration-card.tsx, setup-scripts.ts and its test) are byte-identical to the base branch except the six card lines this PR adds; the APIS_OFF check r…

29fb1c2 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 10/10 · safe to merge
quality 100/100 · no findings · tests covered · risk low

Adds the Google Cloud dashboard template, its docs page and the catalog/card links to it. The two links added to the integration card point at anchors that exist in the new docs page; safe to merge.

  • gcpTemplate charts 24 widgets from GCP_METRIC_GROUPS, with an optional project_id filter
  • docsUrl("gcp") links the catalog entry and two card sections to the new page
  • gcp.test.ts pins every widget's metric, type, group-by attribute and quantile to the poller's output
What was checked
  • Both new anchors resolve: the docs page has ## Google Cloud costs (gcp.md:296) and ## Troubleshooting (gcp.md:317)
  • docsUrl("gcp") is a pure string builder (docs.ts:47) and the gcp key exists (docs.ts:35)
  • ExternalLink takes an arbitrary href and renders target="_blank" rel="noreferrer" (gcp-integration-card.tsx:153)

d1af10b · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
Since the last review only the docs page changed; the code hunks are pinned by the metric-table guard test, but the GCP metric table itself was unread (sandbox checkout never finished).
quality 100/100 · no findings · tests covered · risk low

Adds the gcp dashboard template (24 widgets driven by GCP_METRIC_GROUPS), a guard test over the poller's mapping, and the Google Cloud docs page plus its catalog, icon and docs-link entries. Safe to merge.

  • gcpTemplate builds 24 widgets with type, aggregation and unit taken from GCP_METRIC_GROUPS
  • gcp.test.ts runs each curated metric through mapGcpTimeSeries and fails on unknown metric, type, attribute or quantile
  • DOCS.gcp wires the card and catalog entry to the new docs page
What was checked
  • Card links #google-cloud-costs and #troubleshooting both resolve to headings in docs/integrations/gcp.md:301,322
  • project_id reaches the where clause through escapeMetricStringLiteral (gcp.ts:216)
  • The guard test asserts every widget's query is checked (expect(checked).toBe(widgets.length)), so a metric or attribute that drifts fails the test

d1ff238 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
Every chart is pinned by a test that runs the poller's own mapping; the rest is catalogue, icon and docs links.
quality 100/100 · no findings · tests covered · risk low

Adds the "Google Cloud" dashboard template (24 charts over the GCP metrics the poller stores), registers it, and links the new /docs/integrations/gcp page from the catalogue, the template picker and the integration card. Safe to merge.

  • gcpTemplate charts 24 GCP metrics, taking metric type, aggregation and unit from GCP_METRIC_GROUPS
  • gcp.test.ts runs each curated metric through mapGcpTimeSeries and fails on a drifted metric, type, attribute or quantile
  • Card, catalogue and template picker link the new docs page and its Costs/Roles/Troubleshooting anchors
What was checked
  • All 24 chart metrics, kinds, units and labels resolved against packages/domain/src/gcp-metrics.ts
  • Layout: two 6-column charts per 6-high row, so index.test.ts's full-width, gapless row check holds
  • Only utilization metrics carry unit 1, so the 1 → percent mapping (gcp.ts:32) is right for every chart that uses it

3b73b98 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
The hunks new at this head are a behaviour-identical helper extract and lab null-safety; the metric options branch (F1) still has no test.
quality 98/100 · 1 note · tests partial · risk medium · 1/1 new units observable

The head reworks only two reviewed surfaces: metricScopedAttributeValuesQuery now reads its attribute through one helper for both scopes, and the GCP dashboard lab tightens optional handling. No new defect found; the earlier tests gap on the metric variable branch is unfixed.

  • metricScopedAttributeValuesQuery reads ResourceAttributes/Attributes through one attributeValue helper
  • The GCP dashboard lab guards missing groupBy/whereClause and casts widget.display

Still open from earlier reviews

What was checked
  • attributeValue keeps the pre-refactor result for both scopes (packages/query-engine/src/ch/queries/attribute-keys.ts:156)
  • An empty metricNames cannot reach CH.inList: the schema requires minLength 1 (packages/domain/src/query-engine.ts:485, packages/widgets/src/dashboard/shared/variables.ts:50)
  • The lab fixture stays lazy; src/lab/ is banned from startup (apps/web/perf/check-bundle-budget.ts:114)
Observability coverage: 1 of 1 changes observable
Change Kind Observable Evidence
Metric-scoped attribute values query (metrics table scan by attributes) client yes runs through executeCHQuery under the "attributeValues:metric-scoped" operation (packages/query-engine/src/runtime/query-engine.ts:2354)

854446e · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
The only diff since the last review is a conflict-marker cleanup in a CI budget script, read end to end; the open info finding is unaffected.
quality 98/100 · 1 note · tests not needed · risk low

The change since the last review only clears stale merge-conflict markers out of the web bundle-budget script and reflows its comment; the 699.5 KB budget constant is unchanged. Safe to merge, with the earlier information-level finding — the metric variable branch is still untested — left open.

  • Removes <<<<<<< HEAD/>>>>>>> conflict markers from check-bundle-budget.ts, which were a syntax error
  • maxGzipBytes stays at 699.5 * 1024, so the CI gate is unchanged

Still open from earlier reviews

What was checked
  • Head file parses and the budget constant is still 699.5 * 1024 (apps/web/perf/check-bundle-budget.ts:106)
  • options.ts:90 metric branch re-read against options.test.ts:55-96: no test covers it, so F1 stands

d5cdd6a · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 7/10 · likely safe to merge
The metric-source option branch (F1) is still untested, and the 13 files listed as changed since the last review are unchanged against the PR base, so I could not anchor findings there.
quality 98/100 · 1 note · tests partial · risk medium

Adds the Google Cloud dashboard template, its docs pages, the Google Cloud service-map platform and metric-backed dashboard variables. Nothing new to fix in the hunks this pull request authors, but F1 — the untested metric options branch — still stands.

Still open from earlier reviews

What was checked
  • metric variable source maps to kind: "attributeValues" with scope, metricNames and metricType; the engine passes scope through (runtime/query-engine.ts:2341) and attribute-keys.ts:167 …
  • metricScoped still requires metricType, and metricName is only sent when metricNames is absent, so the legacy single-metric path is unchanged
  • GKE rows classifying as gcp is intended: use-local-service-map.test.ts:91 asserts exactly that for cloudPlatform: "gcp_kubernetes_engine"

c3e20a1 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 7/10 · likely safe to merge
The metric-source variable branch (F1) is still untested and nothing exercises the multi-metric resource scan end to end; every other hunk I traced is consistent.
quality 98/100 · 1 note · tests partial · risk medium · 1/1 new units observable · 1 file not read

Adds the Google Cloud dashboard template (38 charts, seven service tabs), a metric-scoped dashboard variable source, gcp service-map classification and the GCP docs pages. Mergeable; the new metric branch in dashboardVariableOptionsQuery still has no test (open F1). The 2.4k-line lab fixture was checked only structurally, as lab data no production path reads.

  • gcp template: 38 charts across seven service tabs
  • Dashboard variables gain a metric source (metricNames, metricType, scope)
  • Metrics attributeValues query takes a metric list and resource scope
  • Service map classifies Google Cloud traces as gcp

Still open from earlier reviews

What was checked
  • Metric variable values interpolate as widget filters, never as SQL (dashboard-variables-context.tsx:150)
  • resolveDashboardVariableValue prefers defaultValue over All, so project_id sets the Project start (options.ts:58)
  • The metric attributeValues query keeps its OrgId filter and queryStats event from the shared builder (attribute-keys.ts:126)
Observability coverage: 1 of 1 changes observable
Change Kind Observable Evidence
Metric-scoped dashboard variable options (attributeValues over metrics) warehouse query yes run through WarehouseQueryService.compiledQuery with the existing profile/context wiring, same path as the traces/logs option queries (query-engine.ts:2321)
Files not reviewed (1)

The review ended before it read these diffs, so nothing above vouches for them.

  • apps/web/src/lab/gcp-dashboard-fixture.json

2e89d84 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟡 Confidence 6/10 · needs attention
The template is pinned by a strong test, but the platform classifier change is not, and the lab/web-builder hunks went unread by their timed-out group.
quality 88/100 · 1 warning · 1 note · tests partial · risk medium

Adds the Google Cloud dashboard template (38 charts, project/region/workload filters), a metrics-only source for query variables, the Google Cloud mark in the picker and service map, and the GCP docs. Safe to merge once the new gcp platform branch stops swallowing GKE workloads.

  • gcpTemplate charts 38 GCP metrics in one Services section, one tab per service
  • Dashboard query variables gain a metric source reading attribute values from metric tables by name
  • ServicePlatform gains gcp, with a legend, colour and Google Cloud mark
  • buildTemplatePreview now stacks only each section's first tab

Findings

🟠 Warning · F2 · cloudProvider === "gcp" is checked before the Kubernetes rule, so GKE workloads report gcp

correctness · apps/api/src/routes/internal/query-engine.http.ts:261-262

Google Cloud's resource detector sets cloud.provider = gcp on GKE as well as on Cloud Run (cloud.platform = gcp_kubernetes_engine), and the comment only justifies beating the Lambda rule. Every GKE service that reported kubernetes now reports gcp, while an EKS service still reports kubernetes — the map's Kubernetes badge and colour disappear for GCP clusters. Gate the new branch on the FaaS signal the comment describes (faasName !== "" or cloudPlatform === "gcp_cloud_run" | "gcp_cloud_functions"), or move it below isKubernetes; apps/local-ui/src/hooks/use-local-service-map.ts:112 repeats the same order and must change with it.

Reorder so `isKubernetes` is tested before the `cloudProvider === "gcp"` branch, or narrow the branch to the Cloud Run/Cloud Functions platform values, and apply the same order in `classifyPlatform` in `apps/local-ui/src/hooks/use-local-service-map.ts`.
🤖 Prompt to fix this finding with an AI agent
Findings from an automated review of commit d08dda3db2f2841e9a9952b801a15fcd4f807434. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F2 · Warning · correctness · apps/api/src/routes/internal/query-engine.http.ts:261-262
`cloudProvider === "gcp"` is checked before the Kubernetes rule, so GKE workloads report `gcp`
Google Cloud's resource detector sets `cloud.provider = gcp` on GKE as well as on Cloud Run (`cloud.platform = gcp_kubernetes_engine`), and the comment only justifies beating the Lambda rule. Every GKE service that reported `kubernetes` now reports `gcp`, while an EKS service still reports `kubernetes` — the map's Kubernetes badge and colour disappear for GCP clusters. Gate the new branch on the FaaS signal the comment describes (`faasName !== ""` or `cloudPlatform === "gcp_cloud_run" | "gcp_cloud_functions"`), or move it below `isKubernetes`; `apps/local-ui/src/hooks/use-local-service-map.ts:112` repeats the same order and must change with it.
Suggested fix: Reorder so `isKubernetes` is tested before the `cloudProvider === "gcp"` branch, or narrow the branch to the Cloud Run/Cloud Functions platform values, and apply the same order in `classifyPlatform` in `apps/local-ui/src/hooks/use-local-service-map.ts`.

Still open from earlier reviews

What was checked
  • Template test maps every curated metric through mapGcpTimeSeries and fails on unknown metric, type, attribute or quantile
  • metricNames is passed to CH.inList, matching the existing convention in errors.ts and infra.ts
  • metricNames is capped at 20 in both the widgets and domain schemas (variables.ts:50, query-engine.ts:486)

d08dda3 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot maple-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 inline note from Maple's review. The score and summary are in the review comment above.

: mapleSdkType === "client"
? "web"
: "unknown"
: cloudProvider === "gcp"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

cloudProvider === "gcp" is checked before the Kubernetes rule, so GKE workloads report gcp

F2 · Warning · correctness

Google Cloud's resource detector sets cloud.provider = gcp on GKE as well as on Cloud Run (cloud.platform = gcp_kubernetes_engine), and the comment only justifies beating the Lambda rule. Every GKE service that reported kubernetes now reports gcp, while an EKS service still reports kubernetes — the map's Kubernetes badge and colour disappear for GCP clusters. Gate the new branch on the FaaS signal the comment describes (faasName !== "" or cloudPlatform === "gcp_cloud_run" | "gcp_cloud_functions"), or move it below isKubernetes; apps/local-ui/src/hooks/use-local-service-map.ts:112 repeats the same order and must change with it.

Reorder so `isKubernetes` is tested before the `cloudProvider === "gcp"` branch, or narrow the branch to the Cloud Run/Cloud Functions platform values, and apply the same order in `classifyPlatform` in `apps/local-ui/src/hooks/use-local-service-map.ts`.
🤖 Prompt to fix with an AI agent
In `apps/api/src/routes/internal/query-engine.http.ts:261-262`: `cloudProvider === "gcp"` is checked before the Kubernetes rule, so GKE workloads report `gcp`.

Google Cloud's resource detector sets `cloud.provider = gcp` on GKE as well as on Cloud Run (`cloud.platform = gcp_kubernetes_engine`), and the comment only justifies beating the Lambda rule. Every GKE service that reported `kubernetes` now reports `gcp`, while an EKS service still reports `kubernetes` — the map's Kubernetes badge and colour disappear for GCP clusters. Gate the new branch on the FaaS signal the comment describes (`faasName !== ""` or `cloudPlatform === "gcp_cloud_run" | "gcp_cloud_functions"`), or move it below `isKubernetes`; `apps/local-ui/src/hooks/use-local-service-map.ts:112` repeats the same order and must change with it.

Suggested fix: Reorder so `isKubernetes` is tested before the `cloudProvider === "gcp"` branch, or narrow the branch to the Cloud Run/Cloud Functions platform values, and apply the same order in `classifyPlatform` in `apps/local-ui/src/hooks/use-local-service-map.ts`.

Verify the problem exists at that location before changing it, and keep the fix to those lines.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing: classifying GKE as gcp is intentional and pinned by use-local-service-map.test.ts; the service map's Infrastructure tab still lists the Kubernetes workloads.

🤖 Addressed by Claude Code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't fix — classifying GKE as gcp is intentional and pinned by use-local-service-map.test.ts.

🤖 Addressed by Claude Code

@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🔴 Confidence 4/10 · risky as written
Read the template, its guard test and the query paths myself; the lab/dev files and the backend group went partly unread, and the dialog can build a variable no layer will accept.
quality 78/100 · 2 warnings · 1 note · tests partial · risk medium · 1/1 new units observable

Adds a 38-chart Google Cloud dashboard template with project/region/workload variables, a metrics-only query-variable source, the GCP platform in the service map, and the integration docs. Safe to merge once the dialog stops allowing more metric names than the variable schema and the query accept.

  • gcpTemplate adds 38 charts across 7 service tabs with project, region, workload variables
  • New { kind: "metric" } dashboard variable source lists values from named metrics
  • metricScopedAttributeValuesQuery gains metricNames and a resource scope
  • Service map gains the gcp platform, colour, legend and icon

Before merge

  • Manual · Merge the stack bottom-up: this sits on the Google Cloud integration card PR

Findings

🟠 Warning · F3 · Dialog accepts more than 20 metric names, which every reader of the field caps at 20

correctness · apps/web/src/components/dashboard-builder/config/variables-manager-dialog.tsx:112-118

draftError only rejects an empty entry, so pasting 21+ names saves a variable whose metricNames violates the isMaxLength(20) check in packages/widgets/src/dashboard/shared/variables.ts:50, packages/domain/src/http/v2/dashboards.ts:545 and packages/domain/src/query-engine.ts:486. The saved variable then fails the document decode and its options query is rejected, so its dropdown comes up empty with nothing naming the cause.

	if (draft.type === "query" && draft.source.kind === "metric") {
		if (draft.source.metricNames.some((name) => name === "")) {
			return "Name at least one metric to list values from."
		}
		if (draft.source.metricNames.length > 20) {
			return "List at most 20 metrics."
		}
	}
🤖 Prompt to fix this finding with an AI agent
Findings from an automated review of commit 9e6527eebdb42b1da67dc5efa524ca0e3c894cfc. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F3 · Warning · correctness · apps/web/src/components/dashboard-builder/config/variables-manager-dialog.tsx:112-118
Dialog accepts more than 20 metric names, which every reader of the field caps at 20
`draftError` only rejects an empty entry, so pasting 21+ names saves a variable whose `metricNames` violates the `isMaxLength(20)` check in `packages/widgets/src/dashboard/shared/variables.ts:50`, `packages/domain/src/http/v2/dashboards.ts:545` and `packages/domain/src/query-engine.ts:486`. The saved variable then fails the document decode and its options query is rejected, so its dropdown comes up empty with nothing naming the cause.
Replace those lines with:
	if (draft.type === "query" && draft.source.kind === "metric") {
		if (draft.source.metricNames.some((name) => name === "")) {
			return "Name at least one metric to list values from."
		}
		if (draft.source.metricNames.length > 20) {
			return "List at most 20 metrics."
		}
	}

Still open from earlier reviews

What was checked
  • Template guard replays the poller's mapper over every curated GCP metric (gcp.test.ts:10-34)
  • Widget units are real tokens: percent expects a 0-1 fraction, duration_s/bytes exist (units.ts:45,61,64)
  • The widened metric query still filters OrgId (attribute-keys.ts:166)
Observability coverage: 1 of 1 changes observable
Change Kind Observable Evidence
Metric-scoped attribute values query (metricNames + resource scope) warehouse query yes runs through WarehouseQueryService.compiledQuery with context "attributeValues:metric-scoped" (packages/query-engine/src/runtime/query-engine.ts:2354)

9e6527e · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot maple-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 inline note from Maple's review. The score and summary are in the review comment above.

Comment thread apps/web/src/components/dashboard-builder/config/variables-manager-dialog.tsx Outdated
@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 7/10 · likely safe to merge
Reviewed the only file changed since 9e6527e (the 20-metric cap); the open toServicePlatformRow and metric-options findings are unchanged at this head.
quality 88/100 · 1 warning · 1 note · tests partial · risk low

The delta since the last review is the metric-variable name cap in the variables dialog, which now matches the schema's 20-name limit and blocks Save. Nothing new is wrong in it; the Google Cloud template and docs were reviewed earlier.

  • draftError refuses a metric variable listing more than 20 metric names

Still open from earlier reviews

Fixed since the last review

  • ✅ F3 · Dialog accepts more than 20 metric names, which every reader of the field caps at 20
What was checked
  • Cap matches Schema.isMaxLength(20) on metricNames (packages/widgets/src/dashboard/shared/variables.ts:50)
  • submitDisabled={hasErrors} blocks saving the over-long list (variables-manager-dialog.tsx:228)
  • options.test.ts:55-97 still has no metric source case, so the open options finding stands

6dc96c5 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 7/10 · likely safe to merge
The only change since the last review is a test that pins the metric option branch; F2's GKE-as-gcp classification is still in the shipped code and stays open.
quality 90/100 · 1 warning · tests partial · risk medium

Since the last review this PR only adds a test for the metric-variable option query, which closes the "metric branch is untested" finding. The open warning about GKE services classifying as gcp is unchanged at this head.

  • options.test.ts pins dashboardVariableOptionsQuery's metric arm to a metrics attribute-values scan

Still open from earlier reviews

Fixed since the last review

  • ✅ F1 · dashboardVariableOptionsQuery's new metric branch is untested
What was checked
  • New test fails without the metric branch: the fixture has no facet, so it falls into the traces branch
  • Metric arm in options.ts:90-99 returns exactly scope, attributeKey, metricNames, metricType the test asserts
  • GKE workloads still appear in the service map Infrastructure tab, which reads serviceWorkloads, not platform

6696942 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

…rom it

Instantiating a template (v2 API and the MCP create_dashboard tool) copied
only the name, tags, time range and widgets of the built document, so the
Google Cloud template came out as one flat page with no filters and widgets
still referencing $project. Both paths now keep the whole build.

The picker's live preview starts each variable on its default, else All, so a
filtered widget no longer queries a literal $project and draws nothing.
@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
Instantiate's tabs and variables are pinned by the new HTTP test; the MCP tool path and the preview's variable seeding carry none.
quality 100/100 · no findings · tests partial · risk medium

Fixes template instantiation so a template's sections and variables reach the created dashboard (HTTP route and MCP tool), and seeds the picker preview with each variable's default or All. Contained, correct and safe to merge.

  • instantiateTemplate spreads the built document, keeping tabs and variables
  • create_dashboard (MCP) spreads the build the same way
  • TemplateLivePreview wraps its grid in ResolvedDashboardVariablesProvider
  • previewVariableValues seeds each variable from its default, else All
What was checked
  • Spreading a decoded document is safe: an absent optionalKey field is not an own property (DashboardPersistenceService.ts:155)
  • Every template variable carries includeAll, so no variable is left unseeded (gcp.ts:67)
  • Variable refs are all whereClause clauses, where All drops the clause (interpolate.ts:73)

0020074 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@JeremyFunk
JeremyFunk merged commit 3992faf into main Oct 16, 2026
11 checks passed
@JeremyFunk
JeremyFunk deleted the feat/gcp-dashboard-template branch October 16, 2026 17:17
@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
Three fan-out groups ran out of budget, but I read every changed hunk; the metric-variable ClickHouse path is capped and test-pinned.
quality 100/100 · no findings · tests covered · risk medium · 1/1 new units observable

Adds the Google Cloud dashboard template, a metric-sourced dashboard variable, the GCP docs pages and the service-map GCP platform, plus a lab page. Contained and safe to merge; it does not touch the open McpQueryError whose frame sits in lab/registry.ts.

  • gcpTemplate charts 38 GCP metrics in one Services section, one tab per service
  • New metric dashboard variable source lists an attribute from named metrics of one type
  • buildTemplatePreview and instantiate now carry a template's sections and variables
  • Service map classifies cloud.provider = gcp as its own platform

Production impact

Open errors in the changed files
Issue Service Occurrences File
@maple/mcp/errors/McpQueryError maple-chat 15 apps/web/src/lab/registry.ts

After this merges, Maple checks whether they stop.

What was checked
  • metricNames reaches ClickHouse only as a capped IN list (packages/domain/src/query-engine.ts:485), and the metricName param is dropped in that mode (`packages/query-engine/src/runtime/query-en…
  • GCP count metrics carry {instance}/{connection}/{message} units, so DISPLAY_UNITS maps only s, By, 1 (gcp.ts:30)
  • The .github/*, vcs/* and pr-review/* files listed as changed since the last review are not in this PR's diff against feat/gcp-integration-card
Observability coverage: 1 of 1 changes observable
Change Kind Observable Evidence
metric-scoped attribute-values warehouse query (multi-metric IN list) db query yes runs through compiledQuery with context attributeValues:metric-scoped (packages/query-engine/src/runtime/query-engine.ts:2354)

92b6dc4 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

Copy link
Copy Markdown

Maple: after this shipped

✅ Shipped clean. Nothing this change touches got worse in the hour after it deployed.

Deployed to maple-chat (production) at 2026-10-16 19:25 UTC as d4ea21c, the first version after the merge (the merge commit itself never reported). Compared the 60 minutes before and after.

Errors in the changed files:

  • ➖ @maple/mcp/errors/McpQueryError: 0/h → 0/h

Maple looks once, an hour after the first deploy that carries the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant