feat(ansible): opt-in Grafana Cloud observability via new grafana_alloy role - #57
Conversation
…oy role (#55) Add a loopback-only Grafana Alloy agent that ships the node's /metrics and OTLP span exports to Grafana Cloud, gated by one mirrored inventory flag (decdn_grafana_cloud_enabled) defined identically in both roles: - roles/grafana_alloy: pinned v1.19.2 .deb install (sha256-verified) or manual stub mode; fail-loud preflight (secret file must be root:root 0600, four GC_* keys grepped on-host, https-only cloud URLs, ratio/duration/ label/port validators); hardened systemd unit with --config.expand-env so credentials never appear as literals in config.alloy; Alloy's own admin server pinned to loopback. - decdn_node: when the flag flips on, inject otlp_endpoint = "http://127.0.0.1:4317" unless explicitly set — the port is guarded by an assert + molecule tests on both sides. - site.yml runs grafana_alloy between baseline and decdn_node; the Galaxy collection stages the third role. - Tests: new grafana-cloud scenario (positive E2E vs a stub binary), disabled-path asserts in default, five negative cases in validation; suite is now seven scenarios (docs + CI comments synced). - Docs: ansible/README.md deploy subsection, group_vars commented opt-in, roles/grafana_alloy/README.md (rotation, cost/cardinality guardrails). Helm chart untouched.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate review findings remain and must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds opt-in Grafana Cloud observability for Ansible-deployed nodes through a hardened, loopback-only Grafana Alloy role.
Changes:
- Adds Alloy installation, validation, telemetry pipelines, and systemd hardening.
- Wires the opt-in flag and local OTLP endpoint into
decdn_node. - Adds Molecule coverage, documentation, packaging, and CI updates.
Review findings:
ansible/galaxy/build.sh— nit, 1 vote: collection README omits the new role.ansible/molecule/default/verify.yml— moderate, 2 votes: disabled-path assertions never invokegrafana_alloy.ansible/molecule/grafana-cloud/verify.yml— moderate, 1 vote: sampling assertion expects25instead of rendered25.0.ansible/playbooks/site.yml— moderate, 2 votes: partialdecdn_nodedeployments can leave Alloy missing or stale.ansible/roles/grafana_alloy/defaults/main.yml— moderate, 1 vote: scrape target is not derived fromdecdn_metrics_port.ansible/roles/grafana_alloy/defaults/main.yml— moderate, 1 vote: environment and region default empty despite acceptance requirements.ansible/roles/grafana_alloy/defaults/main.yml— critical, 2 votes: overridable addresses can expose telemetry and admin endpoints.ansible/roles/grafana_alloy/tasks/install.yml— moderate, 3 votes: check mode executes a binary that may not exist.ansible/roles/grafana_alloy/tasks/install.yml— critical, 3 votes: substring version checks can accept the wrong installed version, including line 137.ansible/roles/grafana_alloy/tasks/main.yml— critical, 3 votes: release validation uses the wrong binary path.ansible/roles/grafana_alloy/tasks/main.yml— moderate, 3 votes: check mode still starts and validates the service.ansible/roles/grafana_alloy/tasks/main.yml— moderate, 1 vote: disabled mode retains the Alloy state directory and account.ansible/roles/grafana_alloy/tasks/preflight.yml— moderate, 2 votes: scrape interval validation is omitted, including line 43.ansible/roles/grafana_alloy/tasks/preflight.yml— critical, 1 vote: listener and admin addresses lack loopback validation, including line 84.ansible/roles/grafana_alloy/templates/alloy.service.j2— moderate, 1 vote:StateDirectoryis hard-coded instead of following the configured state path.ansible/roles/grafana_alloy/templates/alloy.service.j2— critical, 1 vote: Alloy storage path is not configured under the writable state directory.ansible/roles/grafana_alloy/templates/config.alloy.j2— moderate, 1 vote: configured sampling is not parent-based.ansible/roles/grafana_alloy/templates/config.alloy.j2— moderate, 1 vote: resource attributes use the attributes processor instead of the resource processor.
File summaries
| File | Description |
|---|---|
ansible/roles/grafana_alloy/templates/config.alloy.j2 |
Alloy metrics and OTLP pipelines. |
ansible/roles/grafana_alloy/templates/alloy.service.j2 |
Hardened systemd service definition. |
ansible/roles/grafana_alloy/tasks/preflight.yml |
Secret, credential, port, and configuration validation. |
ansible/roles/grafana_alloy/tasks/main.yml |
Role lifecycle and service management. |
ansible/roles/grafana_alloy/tasks/install.yml |
Release and manual binary installation. |
ansible/roles/grafana_alloy/README.md |
Setup and operational documentation. |
ansible/roles/grafana_alloy/meta/main.yml |
Role metadata. |
ansible/roles/grafana_alloy/handlers/main.yml |
Service reload and restart handlers. |
ansible/roles/grafana_alloy/files/grafana-alloy.env.example |
Credential environment template. |
ansible/roles/grafana_alloy/defaults/main.yml |
Role defaults and telemetry settings. |
ansible/roles/decdn_node/templates/node.toml.j2 |
Conditional OTLP endpoint wiring. |
ansible/roles/decdn_node/defaults/main.yml |
Mirrored feature flag. |
ansible/README.md |
Deployment and testing documentation. |
ansible/playbooks/site.yml |
Role ordering and deployment tags. |
ansible/molecule/validation/converge.yml |
Negative validation scenarios. |
ansible/molecule/grafana-cloud/verify.yml |
Positive integration assertions. |
ansible/molecule/grafana-cloud/prepare.yml |
Scenario fixtures. |
ansible/molecule/grafana-cloud/molecule.yml |
Scenario configuration. |
ansible/molecule/grafana-cloud/files/alloy-stub |
CI Alloy stub binary. |
ansible/molecule/grafana-cloud/converge.yml |
Positive-path convergence. |
ansible/molecule/default/verify.yml |
Disabled-path assertions. |
ansible/Makefile |
Scenario documentation updates. |
ansible/inventory/group_vars/decdn_nodes.yml |
Commented opt-in configuration. |
ansible/galaxy/galaxy.yml |
Collection metadata. |
ansible/galaxy/build.sh |
Collection role staging. |
.github/workflows/molecule.yml |
CI scenario-count update. |
Review details
Suppressed comments (11)
ansible/galaxy/build.sh:19
- This now stages a third role, but the README copied into the collection still says the artifact contains 'two roles' and lists only
baselineanddecdn_node. Consumers of the built Galaxy collection will not discover or understandgrafana_alloy; update the collection README in the same packaging change.
roles=(baseline decdn_node grafana_alloy)
ansible/molecule/grafana-cloud/verify.yml:63
- The template computes
0.25 | float * 100as the rendered float25.0, so this assertion looking forsampling_percentage = 25does not match the generated line and the positive scenario fails despite a valid value. Assert the float form (or render a deliberately normalized integer/decimal format).
'sampling_percentage = 25' in ga_config
ansible/roles/grafana_alloy/defaults/main.yml:56
- This is an independent
9090literal rather than the node'sdecdn_metrics_port. A supported inventory override such asdecdn_metrics_port: 9190leaves the node serving metrics on 9190 while Alloy still scrapes 127.0.0.1:9090, so enabling the feature silently exports no node metrics. Derive the target from the shared port or fail preflight when the two values differ.
grafana_alloy_scrape_target: 127.0.0.1:9090
ansible/roles/grafana_alloy/defaults/main.yml:68
- With only the documented opt-in flag, both
grafana_alloy_deployment_environmentandgrafana_alloy_regiondefault to empty and the template omits them. The linked issue's acceptance criteria require region and environment in the shipped identity attributes, so the default enabled deployment does not meet that contract unless operators provide additional values; make the required values derivable/mandatory or revise the requirement explicitly.
grafana_alloy_instance_id: "" # "" => derive from inventory_hostname
grafana_alloy_deployment_environment: "" # "" => attribute omitted entirely
grafana_alloy_region: "" # operator sets it = decdn_region ("" => omitted)
ansible/roles/grafana_alloy/tasks/install.yml:138
- The version backstop repeats the same substring comparison, so a binary reporting
v1.19.20also passes for the pinned1.19.2. This can mark the wrong binary healthy even after the package gate is corrected; validate the complete reported version rather than substring membership.
or (grafana_alloy_install_method == "release"
and grafana_alloy_version not in _ga_installed_version.stdout)
ansible/roles/grafana_alloy/tasks/main.yml:45
- The disabled path removes only the unit and config directory; it leaves the role-created
/var/lib/alloystate andalloyaccount/group on a host that was previously enabled. That does not implement the surrounding 'complete rollback' claim or the new verify comments about no state/user artifacts. Either remove these role-owned resources (with an explicit state-loss warning) or document and test the retained-state rollback contract.
- name: Remove the leftover unit and rendered configuration
ansible.builtin.file:
path: "{{ item }}"
state: absent
loop:
- /etc/systemd/system/alloy.service
- "{{ grafana_alloy_config_dir }}"
notify: Reload systemd
ansible/roles/grafana_alloy/tasks/preflight.yml:43
| intcoerces malformed values instead of validating their shape, so values such as1.5or1foopass this positive check and are then rendered as invalid Alloy configuration. Add a decimal-digit assertion before converting, so the promised preflight rejects bad knobs before creating host state.
- item.value | int >= 1
ansible/roles/grafana_alloy/tasks/preflight.yml:86
- The coupling check also uses
| int, so a value such as4317xor4317.0passes as 4317 and is rendered into the receiver endpoint verbatim. The node still exports to the literal127.0.0.1:4317, so this fails only later at Alloy config load; validate the port's numeric shape before comparing it.
ansible.builtin.assert:
that:
- grafana_alloy_otlp_grpc_port | int == 4317
ansible/roles/grafana_alloy/templates/alloy.service.j2:43
grafana_alloy_state_diris exposed and used for the managed directory/user home, but this unit hard-codesStateDirectory=alloy. Overriding the documented state path leaves systemd'sProtectSystem=strictwritable exception at/var/lib/alloywhile the configured path is elsewhere, so Alloy state can fail to write or be stored in a different directory. Derive the directive from the variable (with a constrained/var/libcontract) or make the path non-overridable.
# filter. StateDirectory owns {{ grafana_alloy_state_dir }} (the only writable
# path under ProtectSystem=strict): WAL/queue state lives there.
StateDirectory=alloy
StateDirectoryMode=0750
ansible/roles/grafana_alloy/templates/config.alloy.j2:100
- The component configured here is probabilistic trace-ID sampling; naming it
parent_based_tracesand documenting parent-decision inheritance does not make it parent-based. It will not honor an upstream sampled-parent decision, so the emitted traces can differ from the documented whole-tree parent-based policy. Use a parent-aware sampling design or change the documentation/acceptance behavior to match this processor.
{#- Parent-based sampling: spans without an explicit flag inherit their parent's
decision, so whole trace subtrees survive or drop together. Renders the
operator-facing 0..1 keep-ratio as the component's percentage. #}
otelcol.processor.probabilistic_sampler "parent_based_traces" {
sampling_percentage = {{ grafana_alloy_trace_sampling_ratio | float * 100 }}
ansible/roles/grafana_alloy/templates/config.alloy.j2:85
- The
otelcol.processor.attributescomponent writes span/log/datapoint attributes, not OTLP resource attributes. Routing all telemetry through this block therefore leavesservice.name, region, and the other required identity fields off the resource, so Grafana Cloud Application Observability cannot use them for service identity; use theotelcol.processor.resourcecomponent for this block.
otelcol.processor.attributes "decdn_identity" {
{% for attr_key in ga_res_attrs.keys() | sort if ga_res_attrs[attr_key] | length > 0 %}
action {
key = {{ attr_key | tojson }}
action = "upsert"
value = {{ ga_res_attrs[attr_key] | tojson }}
- Files reviewed: 26/26 changed files
- Comments generated: 10
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verified against the pinned Grafana Alloy v1.19.2 binary; every claim below
is reproducible with `make lint-alloy`.
Config (config.alloy.j2) — the file did not parse:
- Alloy's syntax rejects '#' outright (illegal character U+0023). All comments
are '//' now, and the Jinja '{#-' trim markers are gone: they were also
gluing '}' onto the next block.
- otelcol.processor.resource does not exist in Alloy (it is an upstream OTel
collector name). Resource identity is now OTTL `set(attributes[...])`
statements in the `resource` context of otelcol.processor.transform.
- sending_queue / retry_on_failure are exporter blocks, not client blocks.
Unit (alloy.service.j2) — the service crash-looped on start:
- Alloy defines no --config.expand-env, so '${GC_*}' was a literal string, not
an expansion. Credentials are read with sys.env("GC_…") instead, which still
validates when unset — exactly what the deploy-time gate needs.
- Alloy also defines no --server.grpc.listen-addr (that was Grafana Agent).
Dropped, with grafana_alloy_server_grpc_addr and its preflight gates.
Teardown scope — the disable path was destructive by default:
- With the flag false (the default on every host) the role deleted /etc/alloy,
/var/lib/alloy and the alloy account unconditionally, destroying an Alloy
installed by the upstream apt repo or any other tool. Teardown is now gated
on the managed-by marker this role's own unit template emits; anything else
is reported and left alone.
Path validation — a traversing path could delete /etc:
- '^/var/lib/[A-Za-z0-9._/-]+$' accepts /var/lib/../../etc. Path gates moved to
tasks/validate-paths.yml (imported by preflight AND the disable path, which
never ran preflight), with per-segment charset, explicit '..' rejection, and
coverage for config_dir / config_file / secret_file plus a containment check.
Tests — the suite was green on a deployment that could not start:
- Deploy gate: `alloy fmt --test` -> `alloy validate` (component graph, not
just syntax).
- New `make lint-alloy` + CI job `alloy-config`: renders the templates in three
variable combinations and validates them with the REAL digest-pinned binary,
plus asserts every ExecStart flag exists in `alloy run --help`. Reuses the
role's own version+sha256 pin, so a bump without a re-pin fails there instead
of on a host.
- Molecule: teardown-scope play (rollback completeness + foreign-install
survival), negative cases for path traversal and config containment, and
assertions that the config keeps sys.env() and the unit keeps neither dead
flag. Stub comments now state plainly what a green stub run does not prove.
Also fixes an inverted check-mode condition in the install debug message.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
altanoruc
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Reviewed 💬 (0 critical, 2 follow-ups, 2 suggestions)
PR: #57 — feat(ansible): opt-in Grafana Cloud observability via new grafana_alloy role
Author: @thiras
Files changed: 38 (+2189 -21)
State: already merged into main (b7ce735). Copilot's earlier findings look addressed in this SHA (loopback preflight, --storage.path, grafana_alloy_bin_effective, check-mode guards, scrape-interval gate, decdn_node tag, default scenario actually invoking the role). CI on the head commit is green (molecule, alloy-config, ansible-lint, kics).
This is follow-up, not a merge blocker.
🔴 Critical
None.
⚠️ Warnings
- ansible/roles/decdn_node/templates/node.toml.j2 —
decdn_otlp_endpointsilently wins over the Grafana Cloud injection. Alloy can be installed and healthy while the node keeps shipping somewhere else. Fail-loud if both knobs are set. - ansible/roles/grafana_alloy/tasks/main.yml / install.yml — unsupported arch is not a preflight gate. User/group/dirs are created first; a failed
.debinstall then has no unit, so marker-gated teardown will not clean up. Move the amd64/arm64 assert intopreflight.yml.
💡 Suggestions
- ansible/galaxy/README.md — table lists
grafana_alloy, but the usage playbook still only showsbaseline+decdn_node. - ansible/playbooks/site.yml —
--tags observabilitystill runs Alloy withoutdecdn_node, so node.toml can keep/omitotlp_endpointindependently.
✅ Looks Good
- Secrets never leave the host:
sys.env()+ root0600EnvironmentFile, grep-only shape checks, no${GC_*}placeholders. - Loopback contract is actually enforced (OTLP binds + admin addr), scrape target follows
decdn_metrics_port, systemd hardening +--storage.pathunderStateDirectory. - Teardown gated on the managed-by marker — default-off will not nuke a foreign Alloy.
- Test split is honest: stub molecule for plumbing,
make lint-alloyagainst the real pinned binary for config graph / ExecStart flags.
Reviewed by Hermes Agent
| admin_port = {{ decdn_admin_port }} | ||
| {% if decdn_otlp_endpoint not in ["", none] %} | ||
| otlp_endpoint = "{{ decdn_otlp_endpoint }}" | ||
| {% elif decdn_grafana_cloud_enabled | bool %} |
There was a problem hiding this comment.
Warning: decdn_otlp_endpoint is checked first, so a non-empty explicit endpoint silently disables this Grafana Cloud injection. Enable the flag with a leftover endpoint and Alloy comes up on 127.0.0.1:4317 while the daemon never talks to it. Fail-loud when both are set.
| ansible.builtin.import_tasks: preflight.yml | ||
|
|
||
| # --- User + directories ---------------------------------------------------- | ||
| - name: Create the alloy system group |
There was a problem hiding this comment.
Warning: user/group/dirs are created before install.yml proves this arch is supported. Unknown ansible_architecture becomes 'unsupported' and fails the sha256 pin later, with no unit file, so marker-gated teardown will not clean up. Lift the amd64/arm64 assert into preflight.yml.
| # operator gets a clear message instead of a 404 mid-play. | ||
| - name: Resolve the Debian package architecture | ||
| ansible.builtin.set_fact: | ||
| _ga_deb_arch: "{{ ({'x86_64': 'amd64', 'aarch64': 'arm64'}[ansible_architecture] | default('unsupported')) }}" |
There was a problem hiding this comment.
This is the first arch gate and it is not fail-loud — 'unsupported' becomes a pin miss later. Assert ansible_architecture in ['x86_64', 'aarch64'] in preflight before main.yml creates the alloy account and directories.
Implements #55 (Ansible path only — the Helm chart is deliberately untouched).
What
Adds an opt-in Grafana Cloud Application Observability integration for Ansible-deployed deCDN nodes: a dedicated, loopback-only Grafana Alloy agent that scrapes the node's
/metricsand receives the daemon's OTLP span exports, shipping both to your Grafana Cloud org.New role
ansible/roles/grafana_alloydecdn_grafana_cloud_enabledis defined identically in this role anddecdn_node, so inventory overrides it once and both sides react. Off by default; the off-path removes any prior install (clean rollback).v1.19.2) as.deb, sha256-verified against hand-pinned per-arch digests; dpkg-query-gated idempotence;manualmode copies a control-machine binary (used by CI).0600file (symlinks refused, same reasoning asdecdn.env)GC_PROM_REMOTE_WRITE_URL,GC_OTLP_ENDPOINT,GC_PROM_USERNAME,GC_API_TOKEN) — values never transit the control machine; cloud URLs must be httpsconfig.alloycarries${GC_*}placeholders expanded at load time via--config.expand-env; systemd reads the root-owned env file before dropping privileges.127.0.0.1:4317/4318, scrape target127.0.0.1:9090, and Alloy's own admin server explicitly pinned to loopback (upstream defaults it to all interfaces) — no firewall hole needed or added.prometheus.scrape→ identity relabel → remote_write;otelcol.receiver.otlp→ resource attributes → parent-based probabilistic sampler (default keep-ratio 0.25) → batch →otlphttpexporter with queue + retry/backoff.decdn-node.service.j2's sandboxing block, plus molecule-proven idempotent StateDirectory/task-mode agreement.Node-side wiring (minimal diff)
When the flag flips on,
node.toml.j2emitsotlp_endpoint = "http://127.0.0.1:4317"(unless the operator set one explicitly). No new node.toml keys beyond that emission → no schema-keys churn; the existingotlp_endpointshape validator gates both paths. The 4317 literal is guarded by a preflight assert + molecule assertions on both sides so the two roles can't drift apart silently.Playbook & packaging
site.yml:baseline → grafana_alloy → decdn_node(Alloy verified running before the node starts exporting).galaxy/build.shstages the third role into thedecdn.nodecollection.Tests
grafana-cloudscenario: positive end-to-end converge vs a stub binary (user/dirs, secret gate, rendered pipeline config, hardened unit, services running); verifies loopback-only bindings textually, identity labels/sampling/batch knobs,${GC_*}expansion (no secret literals outside the 0600 env file), andotlp_endpointin parsed node.toml.defaultscenario: disabled-path asserts — no alloy user/unit/config/state dirs, no fixture file, node.toml byte-identical to the pre-integration rendering.validationscenario: five negative cases (missing secret, symlinked secret, missing key, http:// URL, malformed sampling ratio) proving each bad input is rejected by its specific gate.make molecule JOBS=3green locally, ansible-lint production profile clean, KICS CRITICAL/HIGH 0).Docs
ansible/README.mdgets a deploy subsection + configuration-table row;group_vars/decdn_nodes.ymlcarries a commented opt-in block;roles/grafana_alloy/README.mddocuments setup, token rotation posture, rollback, and cost/cardinality guardrails (low-cardinality labels only — the bill scales with unique label sets).