From 013990e213e86847d45bb983acc7600c637fb8a3 Mon Sep 17 00:00:00 2001 From: Joana Carvalho Date: Sat, 3 Oct 2026 14:32:15 +0000 Subject: [PATCH] fix(grafana): name grafana_slo's required destination_datasource in the pattern (#165) grafana/grafana requires at least one destination_datasource block on grafana_slo; without it terraform validate fails with "Insufficient destination_datasource blocks". The golden snapshot has always carried it, so CI's validate was green, but neither terraform-patterns nor terraform-gen mentioned it, so whether a generated module validated depended on the model going beyond its instructions (the reporter saw it add the block in one run and omit it in another). - terraform-patterns: a "SLO resource" pattern with the three required blocks (query, objectives, at least one destination_datasource) and the shared prometheus_datasource_uid variable the dashboard and alert rules already use, plus the real error text. - terraform-gen: names the blocks in its Grafana instructions. - tests/hooks/grafana-slo-pattern.test.sh derives the block names from the validated golden and asserts the skill's SLO pattern contains each, so the guidance cannot fall behind the snapshot again. Wired into lint-hooks; 6 of its 7 checks fail against the previous text. Bumps to 0.8.19. Not changed: self-hosted Grafana, where grafana_slo is not available, is #167. --- .claude-plugin/plugin.json | 2 +- .github/workflows/ci.yml | 2 ++ ROADMAP.md | 2 +- agents/terraform-gen.md | 2 +- skills/terraform-patterns/SKILL.md | 40 ++++++++++++++++++++++++- tests/hooks/grafana-slo-pattern.test.sh | 37 +++++++++++++++++++++++ 6 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 tests/hooks/grafana-slo-pattern.test.sh diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index ec59381..226a113 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "otel-as-code", - "version": "0.8.18", + "version": "0.8.19", "description": "OpenTelemetry instrumentation and observability-as-code for any stack", "author": { "name": "radra.gwent", diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b9354ca..62c50c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,6 +57,8 @@ jobs: run: bash tests/hooks/go-instrument-wiring.test.sh - name: Run PHP refusal test (detected, in scope, refused with a reason) run: bash tests/hooks/php-refusal.test.sh + - name: Run Grafana SLO pattern contract (skill text names every block the validated golden carries; #165) + run: bash tests/hooks/grafana-slo-pattern.test.sh - name: Run zero-services guard contract (every command answers an empty scan the same way; #164) run: bash tests/hooks/zero-services-guard.test.sh - name: Run collector exporter-auth contract (--exporter-header; --force keeps exporter headers; #171) diff --git a/ROADMAP.md b/ROADMAP.md index 1fe95c1..af4ed1d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -8,7 +8,7 @@ > of the same shape, and every concrete line item that isn't done yet has a linked tracking issue — > comment there rather than only in this file if you want to weigh in. -Current release: **0.8.18**. Node.js, Python, Java, .NET, Go, Ruby, and Rust (axum) instrumentation, Terraform +Current release: **0.8.19**. Node.js, Python, Java, .NET, Go, Ruby, and Rust (axum) instrumentation, Terraform for Grafana, Datadog, New Relic, and Dash0, semconv pinned at 1.44.0. Since 0.2.0 the command set has grown past generation: `/otel-uninstrument` rolls instrumentation back behind an ownership marker, `/otel-remediate` proposes a deployment-config diff for services that have no application diff --git a/agents/terraform-gen.md b/agents/terraform-gen.md index 4adf5cb..0ea8fc0 100644 --- a/agents/terraform-gen.md +++ b/agents/terraform-gen.md @@ -278,7 +278,7 @@ Rules: ## Grafana main.tf Generate using patterns from the `terraform-patterns` skill (grafana section). -Include: provider block, grafana_folder, grafana_dashboard (request rate + error rate + p99 latency panels **plus one panel per confirmed `businessAttrs` entry** — see "Business-attribute panels" above), grafana_rule_group (error rate > 5% for 5m, p99 > 500ms for 5m), grafana_slo (availability SLO at 99.9% over 30d). +Include: provider block, grafana_folder, grafana_dashboard (request rate + error rate + p99 latency panels **plus one panel per confirmed `businessAttrs` entry** — see "Business-attribute panels" above), grafana_rule_group (error rate > 5% for 5m, p99 > 500ms for 5m), grafana_slo (availability SLO at 99.9% over 30d, with its three required blocks: `query`, `objectives` and at least one `destination_datasource { uid = var.prometheus_datasource_uid }`; leaving the last out fails `terraform validate`, #165). ## Datadog main.tf diff --git a/skills/terraform-patterns/SKILL.md b/skills/terraform-patterns/SKILL.md index 2aae88c..04daf5c 100644 --- a/skills/terraform-patterns/SKILL.md +++ b/skills/terraform-patterns/SKILL.md @@ -136,7 +136,45 @@ variable "grafana_service_account_token" { 1. `grafana_folder` — create a folder to scope all generated resources 2. `grafana_dashboard` — uses `config_json` with a JSON-encoded dashboard model 3. `grafana_rule_group` — unified alerting (NOT the deprecated `grafana_alert_notification`) -4. `grafana_slo` — Grafana Cloud SLOs (requires `grafana_slo` resource, available on Cloud plans) +4. `grafana_slo` — Grafana Cloud SLOs (requires `grafana_slo` resource, available on Cloud plans). + **It has required blocks, and omitting one fails `terraform validate`** (#165): at least one + `destination_datasource` (without it: `Insufficient destination_datasource blocks … At least 1 + "destination_datasource" blocks are required`), plus `query` and `objectives`. Follow the shape + in "SLO resource" below. + +### SLO resource +Shares the `prometheus_datasource_uid` variable with the dashboard panels and alert rules, so there +is one place to point all of them at the Prometheus/Mimir datasource: +```hcl +variable "prometheus_datasource_uid" { + description = "UID of the Prometheus/Mimir datasource backing these queries" + type = string + default = "grafanacloud-prom" +} + +resource "grafana_slo" "otel" { + name = "${var.service_name} availability" + description = "99.9% of requests to ${var.service_name} are non-5xx over 30d." + + query { # required + type = "freeform" + freeform { + query = "" + } + } + + objectives { # required + value = 0.999 + window = "30d" + } + + destination_datasource { # required: at least one + uid = var.prometheus_datasource_uid + } +} +``` +`tests/snapshots/grafana/main.tf.snap` is this shape and is `terraform validate`d in CI; keep the +two in step. ### Key gotchas - `grafana_folder.uid` is auto-generated on create; reference it via `grafana_folder..uid` (e.g. `grafana_folder.otel_folder.uid`) diff --git a/tests/hooks/grafana-slo-pattern.test.sh b/tests/hooks/grafana-slo-pattern.test.sh new file mode 100644 index 0000000..dd98c06 --- /dev/null +++ b/tests/hooks/grafana-slo-pattern.test.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# tests/hooks/grafana-slo-pattern.test.sh +# Locks the #165 contract: the Grafana guidance the generator reads must name every block the +# validated golden grafana_slo carries. The golden has always had destination_datasource, so CI's +# terraform validate was green, while the skill and generator text never mentioned it: whether a +# generated module validated depended on the model going beyond its instructions. +set -uo pipefail +cd "$(dirname "$0")/../.." +pass=0; fail=0 +check() { if eval "$2"; then echo "PASS: $1"; pass=$((pass+1)); else echo "FAIL: $1"; fail=$((fail+1)); fi; } + +SKILL="skills/terraform-patterns/SKILL.md" +GEN="agents/terraform-gen.md" +GOLD="tests/snapshots/grafana/main.tf.snap" + +# Nested block names directly inside the golden's grafana_slo resource. +BLOCKS=$(awk '/^resource "grafana_slo"/{f=1;next} f&&/^}/{f=0} f&&/^ [a-z_]+ *\{/{gsub(/[ {]/,""); print}' "$GOLD") +echo "golden grafana_slo blocks: $(echo $BLOCKS)" + +check "the golden grafana_slo has the three blocks this contract is about" \ + 'grep -qx query <<<"$BLOCKS" && grep -qx objectives <<<"$BLOCKS" && grep -qx destination_datasource <<<"$BLOCKS"' + +SLO_SECTION=$(sed -n '/^### SLO resource/,/^### Key gotchas/p' "$SKILL") +for b in $BLOCKS; do + check "terraform-patterns SLO pattern includes the golden's \`$b\` block" \ + 'grep -qE "^ $b *\{" <<<"$SLO_SECTION"' +done +check "terraform-patterns says omitting a required block fails validate, with the real error text" \ + 'grep -qF "Insufficient destination_datasource blocks" "$SKILL"' +check "terraform-patterns defines the shared prometheus_datasource_uid variable the SLO uses" \ + 'grep -qF "variable \"prometheus_datasource_uid\"" <<<"$SLO_SECTION" && grep -qF "uid = var.prometheus_datasource_uid" <<<"$SLO_SECTION"' +check "terraform-gen names destination_datasource in its Grafana instructions" \ + 'grep -qF "destination_datasource { uid = var.prometheus_datasource_uid }" "$GEN"' + +echo "" +echo "Results: $pass passed, $fail failed" +[ "$fail" -eq 0 ]