Repository navigation
feat(ansible): monitor the machine, not just the daemon, in grafana_alloy - #61
Conversation
…lloy The role shipped two signals: a scrape of the node's own /metrics and the daemon's OTLP push. Nothing observed the host, so a full disk, a saturated CPU, a dead NIC or a stopped decdn-node.service showed up as blank dashboards with no explanation, and a wedged agent was indistinguishable from a healthy, silent node. Add three signals, each behind a sub-knob under the existing decdn_grafana_cloud_enabled flag: - host metrics via Alloy's in-process prometheus.exporter.unix — a curated 13-collector set plus systemd scoped to the units that matter, labelled job="integrations/node_exporter" so Grafana Cloud's prebuilt Linux Server dashboards and alerts work unmodified; - journald -> Grafana Cloud Loki, dropping debug priority and a noisy-unit set while the __journal* fields still exist, with max_age bounding the post-outage replay; - prometheus.exporter.self, so agent health is visible. Everything stays loopback-only: the exporters run inside the agent, so no new listener, port or firewall hole. Two hardening directives are relaxed only when the signal that needs them is on — ProtectHome=read-only (else /home and /root usage is missing and a disk-full alert there can never fire) and SupplementaryGroups=systemd-journal adm (without both, journal collection silently yields nothing). Also make first-run setup cheaper: the five non-secret connection settings become inventory variables that fall back to their GC_* env key when empty, so a new host provisions one secret instead of five values. The API token deliberately has no variable — config.alloy is world-readable on the host — and preflight refuses anything token-shaped in the other five. Guardrails the real binary cannot provide, because Alloy accepts all of these as valid config: - an unknown collector name is silently ignored, not an error, so a typo would ship fewer metrics forever; names are checked against a committed inventory in the role's vars/; - an empty set_collectors reads as unset and re-enables ~45 default collectors, so a set that nothing survives is refused; - upstream applies disable_collectors BEFORE enable_collectors, so a vetoed collector would stay enabled and lose its scoping block; the veto is subtracted from both lists at render time instead. Tested by rendering eight variable combinations through the real pinned Alloy 1.19.2 (make lint-alloy), the grafana-cloud molecule scenario for plumbing and idempotence, and eight new negative cases in the validation scenario. BREAKING CHANGE: the daemon's own series now carry job="decdn-node" instead of the implicit job="prometheus.scrape.decdn_node"; set grafana_alloy_node_job: "" to keep the old identity. Upgrading a host also needs Loki credentials (inventory or env keys) unless logs are turned off; preflight names both the missing key and the variable that satisfies it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings affect journal access, credential validation, and numeric ID handling.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Extends opt-in Grafana Alloy monitoring with host metrics, journald logs, and Alloy health telemetry.
Changes:
- Adds machine, self, and journal telemetry pipelines.
- Adds credential fallbacks, collector guardrails, and conditional systemd hardening.
- Expands real-binary, Molecule, and documentation coverage.
File summaries
| File | Summary | Findings |
|---|---|---|
ansible/tests/alloy-config/validate.sh |
Validates rendered monitoring combinations and secret hygiene. | Moderate (1): Secret detection omits glsa_ and JWT-like eyJ tokens. |
ansible/tests/alloy-config/render.yml |
Renders eight configuration scenarios. | Nit (1): Legacy pipeline fixture inherits the new decdn-node job label. |
ansible/roles/grafana_alloy/vars/main.yml |
Adds the collector inventory. | — |
ansible/roles/grafana_alloy/templates/config.alloy.j2 |
Adds host, self, journal, and credential pipelines. | Moderate (2): Numeric instance IDs are not normalized before length checks and JSON encoding. |
ansible/roles/grafana_alloy/templates/alloy.service.j2 |
Adds conditional systemd hardening. | Moderate (1): A partial journal-group set is treated as sufficient and can silently prevent log collection. |
ansible/roles/grafana_alloy/tasks/preflight.yml |
Adds collector, endpoint, label, and credential validation. | Moderate (1): Numeric Prometheus IDs can fail raw length handling.Moderate (1): Numeric Loki IDs have the same issue. Moderate (3): Instance-ID validation accepts nonnumeric values. |
ansible/roles/grafana_alloy/tasks/main.yml |
Manages journal-reader group membership. | Moderate (2): Previously added journal memberships persist when logs are disabled. Moderate (2): Partial journal-group availability is allowed despite requiring both groups. |
ansible/roles/grafana_alloy/README.md |
Documents monitoring and credential behavior. | — |
ansible/roles/grafana_alloy/files/grafana-alloy.env.example |
Documents token-only provisioning. | — |
ansible/roles/grafana_alloy/defaults/main.yml |
Adds monitoring and endpoint defaults. | — |
ansible/README.md |
Updates observability documentation. | Nit (1): Signal inventory says four while Alloy health makes five. |
ansible/molecule/validation/converge.yml |
Adds negative validation cases. | — |
ansible/molecule/grafana-cloud/verify.yml |
Verifies pipelines, groups, and hardening. | — |
ansible/molecule/grafana-cloud/prepare.yml |
Updates credential fixtures. | — |
ansible/molecule/grafana-cloud/converge.yml |
Exercises Loki inventory configuration. | — |
AGENTS.md |
Documents the expanded Alloy role. | — |
Review details
Suppressed comments (6)
ansible/README.md:194
- This section says the agent ships four signals but lists the node metrics, machine metrics, journald, and OTLP; Alloy self-health is also enabled by default and described immediately below. The signal inventory is inconsistent—count it as five and include Alloy health in this list, or remove it from the later claim.
[Grafana Alloy](https://grafana.com/docs/alloy/) agent shipping four signals to your
Grafana Cloud org — the node's `/metrics`, **the machine itself** (CPU/memory/disk/
filesystem/network/load plus per-unit state, via Alloy's in-process `node_exporter`),
**journald**, and the daemon's OTLP spans (`127.0.0.1:4317`) — off by default, driven
by ONE mirrored inventory flag:
ansible/roles/grafana_alloy/tasks/preflight.yml:365
- The ID validation above coerces
grafana_alloy_prom_usernameto a string, so an unquoted YAML numeric ID such as1234567passes, but this conditional applieslengthto the raw integer. That makes a documented/valid numeric inventory value fail with a Jinja type error before the credential check; coerce it to a string here (and in the template).
required: "{{ grafana_alloy_prom_username | length == 0 }}"
ansible/roles/grafana_alloy/tasks/preflight.yml:377
- The same raw-value
lengthproblem exists for the Loki instance ID: the preceding validator accepts an unquoted numericgrafana_alloy_loki_username, but this expression then tries to takelengthof the integer. Normalize it before this conditional so valid numeric inventory values do not fail preflight.
required: "{{ grafana_alloy_logs_enabled | bool and grafana_alloy_loki_username | length == 0 }}"
ansible/roles/grafana_alloy/templates/alloy.service.j2:38
- This condition treats one existing group as sufficient even though the role's comments and README require both
systemd-journalandadm. On a host missing the other group, the unit gets a partialSupplementaryGroupssetting, no warning is emitted, and journald can start while collecting nothing—the silent failure this guard is meant to prevent. Handle partial availability explicitly (or fail when logs are enabled) rather than usinglength > 0.
{% if grafana_alloy_logs_enabled | default(false) | bool and (_ga_journal_groups | default([]) | length > 0) %}
ansible/tests/alloy-config/render.yml:179
- This fixture is described as reproducing the pre-machine-monitoring pipeline exactly, but it inherits the new default
grafana_alloy_node_job: decdn-node, so the daemon series still change identity from the old component-derived job. Setgrafana_alloy_node_job: ""for this legacy case or narrow the claim/assertions to only the added component groups.
grafana_alloy_logs_enabled: false
ansible/tests/alloy-config/validate.sh:112
- This hygiene check only recognizes
glc_, while the role's token gate explicitly treatsglsa_and JWT-likeeyJvalues as credentials too. A literal service-account/JWT token could therefore be rendered into a matrix config without failingmake lint-alloy; keep this render gate aligned with the preflight patterns.
if grep -qF 'glc_' "$config"; then
- Files reviewed: 16/16 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Numeric instance IDs are the common case (Grafana Cloud hands you a number), and YAML hands an unquoted one over as an int. Both the ga_endpoint macro and the credential-requirement flags took its length, and tojson would have emitted a bare number into a field Alloy types as a string. Coerce once in each place, and pin the behaviour by feeding an unquoted ID through both the render matrix and the grafana-cloud scenario. Turning the logs signal off dropped SupplementaryGroups from the unit but left the account in systemd-journal/adm from the earlier run — and systemd falls back to the account's groups, so the agent kept journal read access after the signal it was granted for was switched off. Take the grant back, preserving any other membership, and assert the transition in the scenario that created it. Also: warn when only one journal group exists instead of only when both are missing; correct the "requires both groups" claim (systemd-journal owns the files, adm gets an ACL — either grants read); keep validate.sh's literal-token scan in lockstep with the preflight gate (glsa_, eyJ); blank the node job in the legacy render case so "reproduces the old pipeline" is literally true; and describe the instance-ID contract as the endpoint's basic-auth username, which is what the charset actually allows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Reviewed the Copilot findings; fixed the valid ones in 57f6839. Fixed
Adjusted rather than applied
Re-validated: |
Why
roles/grafana_alloyshipped exactly two signals: a scrape of the deCDN daemon's own/metricsand the OTLP the daemon pushes at the loopback receivers. Nothing observed themachine. A full disk, a saturated CPU, a lost NIC, or a stopped
decdn-node.serviceshowed up as blank dashboards with nothing to say why — and a wedged agent was
indistinguishable from a healthy, silent node.
Second complaint: first-run setup made operators hand-write four Grafana Cloud values into a
root-owned
0600file on every host, even though only one of them is a secret.What
Three new signals, each behind its own sub-knob under the existing
decdn_grafana_cloud_enabledflag (itself stillfalseby default):prometheus.exporter.unix— 13 curated collectors +systemdscoped to(decdn-node|alloy|ssh|sshd)\.serviceintegrations/node_exporterdebug+ noisy units dropped,max_agebounding the post-outage replayintegrations/node_exporterprometheus.exporter.selfintegrations/alloyThe
integrations/node_exporterlabel is the point: Grafana Cloud's prebuilt LinuxServer dashboards and alert rules key off it, so they light up with no dashboard work.
Still loopback-only. The exporters run inside the agent — no new listener, no new port,
no firewall hole. Two systemd directives are relaxed only when the signal that needs them
is enabled, each with the rationale inline in the unit:
ProtectHometrue→read-only(host metrics on)/homeand/rootusage is missing, so a disk-full alert on them can never fireSupplementaryGroups+ systemd-journal adm(logs on)Everything else (
ProtectSystem=strict,NoNewPrivileges,SystemCallFilter=@system-service,PrivateDevices, …) stays; the curated collector set deliberately excludes the ones thatwould need capabilities or a widened syscall filter.
One secret per host
The five non-secret connection settings (prom URL/username, OTLP endpoint, Loki
URL/username) are now inventory variables that fall back to their
GC_…env key when leftempty, so a new host provisions one value:
GC_API_TOKENhas no inventory variable by design — the renderedconfig.alloyisworld-readable on the host — and preflight refuses anything token-shaped (
glc_,glsa_,eyJ, or a URL with embedded credentials) in the other five. No secret transits the controlmachine: the role only greps key shapes on the target, never reading values back.
Guardrails the binary cannot provide
alloy validateaccepts all three of these as perfectly valid config, so they are gated onthe control machine instead:
map lookup, so
diskstat(missing the finals) starts clean and ships no disk metricsforever. Names are checked against a committed inventory in the role's
vars/.set_collectorsreads as unset, re-enabling the full ~45-collector defaultset — the opposite of what an operator who disabled everything wanted. A set that nothing
survives is refused.
disable_collectorsdoes not win upstream: it is applied beforeenable_collectors,so vetoing
systemdwould leave it enabled while dropping itsunit_includescoping —per-unit series for every unit on the box. The veto is subtracted from both lists at render
time instead.
job="decdn-node"instead ofthe implicit
job="prometheus.scrape.decdn_node"(Alloy's component ID — animplementation detail no dashboard should hard-code). Set
grafana_alloy_node_job: ""tokeep the old value.
four-key env file has no Loki endpoint or instance ID, so preflight fails until you either
set
grafana_alloy_loki_url/_loki_usernamein inventory (recommended — nothing to doper host), append
GC_LOKI_URL/GC_LOKI_USERNAMEto the env file, or setgrafana_alloy_logs_enabled: false. The failure names both the missing key and thevariable that would satisfy it. The other four keys are untouched.
Testing
make lint-alloy— the gate that matters. Renders the templates in eight variablecombinations and feeds all of them to the real pinned Alloy v1.19.2:
alloy validate(component graph, not just syntax), an
ExecStart-flag check againstalloy run --help,and greps proving each Jinja branch actually switched — including that "every sub-knob off"
reproduces the pre-machine-monitoring pipeline and that the token is never rendered as a
literal. The molecule stub exits 0 for everything and cannot prove any of this.
grafana-cloud— plumbing + idempotence: the three pipelines render, the joblabels are right, both credential halves work, the unit carries the relaxations, and the
service account really joins the journal groups.
validation— eight new negative cases: unknown collector, empty effectivecollector set, every collector vetoed, cleartext Loki URL, access token in inventory,
service-account token in inventory, bad
max_age, logs on with no Loki credentials.make lint-ansible(production profile),make lint,make security-ansible(KICS: 0critical, 0 high), molecule
default(teardown still removes everything).Not in scope
The Helm chart (host monitoring on Kubernetes is the cluster's job), dashboards-as-code
(Grafana Cloud's integration ships its own), and per-process metrics for the daemon (the
processescollector conflicts withProtectProcand the daemon already exports/metrics).