Skip to content

Commit 2d2186d

Browse files
lexfreilexfrei
authored
feat(monitoring): VictoriaTraces backend + Grafana datasource (tracing phase 1) (#3762)
## What this PR does Phase 1 of distributed tracing (design: cozystack/community#38; epic: #3761): a **VictoriaTraces backend and Grafana traces datasource** in the monitoring stack, mirroring the logs (`VLCluster`) implementation. No new operator: `VTCluster`/`VTSingle` already ship in victoria-metrics-operator. - **`tracingStorages`** values in `extra/monitoring` (annotated, schema-generating) and `system/monitoring` (defaults), per-entry `mode: cluster|single`. **Empty by default, so tracing is off out of the box**: nothing produces spans until the collector ships, so no backend is provisioned until an operator adds an entry. - **`templates/vtraces/vtraces.yaml`**: `VTCluster` (default) or `VTSingle` (edge/dev/small); `retentionPeriod` plus optional `retentionMaxDiskSpaceUsageBytes`; a release-scoped **PVC cleanup label**, reused by the existing post-delete hook, so no hook change is needed. - **Render-time validation** (`_helpers.tpl`): the installed VictoriaTraces CRDs are the trimmed ones, so a bad value would otherwise reach the operator and fail after admission. Each `tracingStorages` entry is checked before anything renders: `name` must be an RFC 1123 label of at most 42 characters (the `vtstorage-<name>-<hash>` pod revision label is capped at 63) and unique across the list, `mode` must be `cluster` or `single`, `storage` must be a positive quantity (a zero size makes the operator mount an EmptyDir in single mode, and a negative one fails when the PVC is created), and `retentionDiskUsageBytes` must be a VictoriaMetrics `BytesString` (the operator rejects the Kubernetes-quantity `8Gi` grammar that `storage` uses). The `storage` field is also typed as a quantity in the schema, and the rendered name is quoted so a numeric name stays a string. - **`templates/vtraces/grafana-datasource.yaml`**: a Jaeger-typed traces datasource per entry (built-in Grafana core datasource, no plugin). - **VPA**: a `VerticalPodAutoscaler` for each trace component (`vtinsert`/`vtselect` Deployments, `vtstorage` StatefulSet, `vtsingle` Deployment), matching the metrics/logs sizing, with `updateMode: Initial`. - **Readiness gate**: `VTCluster`/`VTSingle` `healthCheckExprs` on `status.updateStatus`, alongside the existing `VLCluster` gate. - **VictoriaLogs datasource fix** (`templates/vlogs/grafana-datasource.yaml`): the range over `logsStorages` emitted no `---` between documents, so two or more entries collapsed into one document and only the last datasource survived. Each entry is now its own document. - **Tests and schema**: helm-unittest suites for off-by-default, cluster vs single, datasource type and both query URLs, retention, PVC label, VPA targets, every validation guard with its boundaries, and per-entry vlogs datasource documents, plus regenerated `values.schema.json`, `README.md` and the `monitoring-rd` ApplicationDefinition. #### Upgrade impact With default values the only rendered object that changes is the `monitoring-system` HelmRelease: it gains the `VTCluster`/`VTSingle` `healthCheckExprs` and `tracingStorages: []`. Flux evaluates those expressions only against objects in the release inventory, so they are inert while tracing is off. No VictoriaTraces object is created on upgrade. Installs with two or more `logsStorages` entries additionally gain the `GrafanaDatasource` objects that were previously dropped (see the VictoriaLogs fix above); nothing else under VictoriaLogs or VictoriaMetrics changes. #### Verification - `helm unittest`: `system/monitoring` (89 tests) and `extra/monitoring` (11 tests) pass. Each validation guard and each branch of the zero-storage pattern was mutation-checked: reverting it turns its own test red. - `helm template` renders `VTCluster`/`VTSingle`/datasource/VPA cleanly. `tracingStorages: []` (the default) renders nothing. An invalid, duplicate or over-long `name`, a zero or negative `storage`, or a bad `retentionDiskUsageBytes` (`8Gi`) is refused at render with an actionable message; a numeric `name` renders as a string. `make generate` is idempotent. - **Datasource URLs verified against upstream docs** ([VictoriaTraces and Grafana](https://docs.victoriametrics.com/victoriatraces/querying/grafana/)): cluster `vtselect:10471/select/jaeger`, single `vtsingle:10428/select/jaeger`. `jaeger` is the stable, built-in Grafana datasource; the Tempo Query API (`/select/tempo`, `type: tempo`, VictoriaTraces v0.9.4+) has a richer TraceQL UX but is upstream-experimental, so Jaeger is the default. Both URLs are pinned by unit tests. - The `retentionDiskUsageBytes` pattern, the zero-storage check and the name limit were checked against victoria-metrics-operator v0.68.4 (`BytesString.UnmarshalJSON`, `isStorageEmpty`, the `vtstorage-<name>` StatefulSet name) and the Kubernetes `ControllerRevisionName` naming. #### Scope / deferred (tracked in #3761) Per-tenant OTLP Collector, engine integrations, shared-central topology, and docs are follow-up PRs. This PR intentionally does **not** enable the OTLP/gRPC listener: apps can use `vtinsert`'s native OTLP/HTTP, and the gRPC listener lands with the collector. Builds on the now-merged design proposal cozystack/community#38. ### Screenshots Not a UI change. ### Downstream repositories Walked against the diff, no box ticked yet: - cozystack/website: `monitoring` is in the website `Makefile` `SERVICES` list, so its reference page is regenerated from `packages/extra/monitoring/README.md` automatically on the next stable tag. A user guide for tracing belongs to the docs follow-up in #3761; no website issue or PR exists for it yet. - cozystack/terraform-provider-cozystack: the provider has no `Monitoring` resource (only the tenant's `monitoring` flag), so the new schema field does not reach it. - The remaining repositories in the trigger map are not reached by this diff: no platform values, variants, installer values, `hack/` tooling, node prerequisites, ApplicationDefinition semantics or telemetry names change. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(monitoring): add a VictoriaTraces backend and a Grafana traces datasource to the monitoring stack (distributed tracing, phase 1). Tracing is off by default; enable it per tenant by adding a `tracingStorages` entry (`cluster` or `single` mode). Also fix the VictoriaLogs Grafana datasources: with two or more `logsStorages` entries, every entry except the last was silently dropped, so existing installs with several log stores gain the missing datasources on upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable trace storage with cluster or single-node deployment modes. * Supports retention periods, persistent storage sizing, storage classes, and optional disk-usage limits. * Automatically configures Grafana tracing data sources for each enabled trace backend. * Monitoring readiness reflects the health of configured trace storage services. * Invalid trace storage settings are rejected during configuration. * **Bug Fixes** * Every VictoriaLogs storage now gets its own Grafana datasource; with several log storages, all but the last were previously dropped. * **Tests** * Added coverage for storage rendering, retention settings, validation guards, Grafana endpoints, health checks, and cleanup behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 47df27d + e8779e4 commit 2d2186d

16 files changed

Lines changed: 1068 additions & 31 deletions

‎packages/extra/monitoring/README.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,19 @@
5353
| `logsStorages[i].storageClassName` | StorageClass used to store the data. | `string` | `replicated` |
5454

5555

56+
### Traces storage configuration
57+
58+
| Name | Description | Type | Value |
59+
| -------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | ------------ |
60+
| `tracingStorages` | Configuration of traces storage instances. Empty by default (tracing off): nothing writes spans until the trace collector ships, so no backend is provisioned. Add an entry to enable a VictoriaTraces backend. | `[]object` | `[]` |
61+
| `tracingStorages[i].name` | Name of the storage instance: a lowercase RFC 1123 label of at most 42 characters. Treat as immutable once a backend exists: the VTCluster/VTSingle, its Services, PVCs and Grafana datasource are all named after it, so renaming an entry deploys a new, empty backend and removes the old one, the same way changing mode does (single mode loses the old PVC, cluster mode leaves the old vtstorage PVCs Bound until the whole Monitoring release is deleted). | `string` | `""` |
62+
| `tracingStorages[i].mode` | Deployment mode: "cluster" renders a VTCluster; "single" renders a single-binary VTSingle for edge/dev/small clusters. Treat as immutable once a backend exists: mode selects which Kind is rendered under the same name, so changing it on a running backend re-identifies the object rather than reconfiguring it. Switching from "single" to "cluster" prunes the VTSingle and the operator garbage-collects its PVC (stored traces are lost); switching from "cluster" to "single" (or removing the entry) leaves the vtstorage PVCs Bound and billed until the whole Monitoring release is deleted. The API cannot guard this (transition rules are not available on an array-item field), so this note is the only guard: migrate deliberately, accepting the loss. | `string` | `cluster` |
63+
| `tracingStorages[i].retentionPeriod` | Retention period for traces. | `string` | `14d` |
64+
| `tracingStorages[i].storage` | Persistent volume size. | `quantity` | `10Gi` |
65+
| `tracingStorages[i].storageClassName` | StorageClass used to store the data. | `string` | `replicated` |
66+
| `tracingStorages[i].retentionDiskUsageBytes` | Optional disk-usage cap as a VictoriaMetrics BytesString (e.g. "8GB" or "8GiB"), bounding disk use on top of the age-based retentionPeriod. Uses byte-suffix grammar (kb/mb/gb/tb, KiB/MiB/GiB/TiB), NOT the Kubernetes-quantity grammar of the sibling storage field, so "8Gi" is rejected by the operator. In cluster mode the cap is per vtstorage replica (replicaCount 2), so the effective total is twice the value. | `string` | `""` |
67+
68+
5669
### Alerta configuration
5770

5871
| Name | Description | Type | Value |

‎packages/extra/monitoring/templates/helmrelease.yaml‎

Lines changed: 54 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,50 +10,75 @@ spec:
1010
name: cozystack-monitoring-application-default-monitoring-system
1111
namespace: cozy-system
1212
interval: 5m
13-
# Aligned with the outer cozyrds `release.cozystack.io/helm-install-timeout: "15m"`
14-
# (packages/system/monitoring-rd/cozyrds/monitoring.yaml) so the inner HR does
15-
# not fail at 10m while the OIDC users-Job is still inside its
16-
# activeDeadlineSeconds: 900 (10m Grafana wait + reconcile budget).
13+
{{- /*
14+
Aligned with the outer cozyrds `release.cozystack.io/helm-install-timeout: "15m"`
15+
(packages/system/monitoring-rd/cozyrds/monitoring.yaml) so the inner HR does
16+
not fail at 10m while the OIDC users-Job is still inside its
17+
activeDeadlineSeconds: 900 (10m Grafana wait + reconcile budget).
18+
*/}}
1719
timeout: 15m
1820
install:
1921
remediation:
2022
retries: -1
2123
upgrade:
2224
remediation:
2325
retries: -1
24-
# `poller` waitStrategy is required for healthCheckExprs to be evaluated.
25-
# Without it the HR flips Ready as soon as helm applies the VLCluster CR,
26-
# before the victoria-metrics-operator has stood up the log store — which is
27-
# exactly how a missing/broken VictoriaLogs stayed invisible (issue #3181:
28-
# fluent-bit drops logs at a dead-end vlinsert-generic while the release is
29-
# Ready). Gating on the VLCluster's own status makes that failure loud.
26+
{{- /*
27+
`poller` waitStrategy is required for healthCheckExprs to be evaluated.
28+
Without it the HR flips Ready as soon as helm applies the VLCluster CR,
29+
before the victoria-metrics-operator has stood up the log store, which is
30+
exactly how a missing/broken VictoriaLogs stayed invisible (issue #3181:
31+
fluent-bit drops logs at a dead-end vlinsert-generic while the release is
32+
Ready). Gating on the VLCluster's own status makes that failure loud.
33+
*/}}
3034
waitStrategy:
3135
name: poller
32-
# VLCluster's status.conditions carry no stable Ready-type contract for this
33-
# operator version, so key on status.updateStatus instead. The CRD types it
34-
# as a free-form string; the operator's known values are operational,
35-
# expanding, failed and paused. Map `operational` -> current and `expanding`
36-
# -> in-progress; the `failed` predicate is a catch-all for everything else
37-
# (the operator's `failed` and `paused`, plus any future/unknown value), so
38-
# an unexpected status can never be silently read as Ready. A paused or
39-
# failed log store is not ingesting, so the release must not report Ready;
40-
# cozystack never sets spec.paused on these VLClusters, so `paused` is not
41-
# reachable in normal operation. Before the operator writes any updateStatus
42-
# (fresh apply) all three predicates are false, so the poller holds the
43-
# release not-ready until it becomes operational or hits `timeout:` — the
44-
# real backstop. Matching by kind covers every logsStorages entry. (CEL
45-
# evaluation is exercised live, not in helm-unittest, which cannot run it.)
46-
#
47-
# The gate keys on a VLCluster existing, so an empty logsStorages would leave
48-
# nothing to match. That configuration is rejected at render time by the
49-
# system/monitoring chart (templates/vlogs/vlogs.yaml fails on an empty
50-
# logsStorages), so the "Ready with no log store" hole cannot be reached.
36+
{{- /*
37+
VLCluster's status.conditions carry no stable Ready-type contract for this
38+
operator version, so key on status.updateStatus instead. The CRD types it
39+
as a free-form string; the operator's known values are operational,
40+
expanding, failed and paused. `operational` maps to current and `expanding`
41+
to in-progress; the `failed` predicate is a catch-all for everything else
42+
(the operator's `failed` and `paused`, plus any future/unknown value), so
43+
an unexpected status can never be silently read as Ready. A paused or
44+
failed log store is not ingesting, so the release must not report Ready;
45+
cozystack never sets spec.paused on these VLClusters, so `paused` is not
46+
reachable in normal operation. Before the operator writes any updateStatus
47+
(fresh apply) all three predicates are false, so the poller holds the
48+
release not-ready until it becomes operational or hits `timeout:`, the
49+
real backstop. Matching by kind covers every logsStorages entry. (CEL
50+
evaluation is exercised live, not in helm-unittest, which cannot run it.)
51+
52+
The gate keys on a VLCluster existing, so an empty logsStorages would leave
53+
nothing to match. That configuration is rejected at render time by the
54+
system/monitoring chart (templates/vlogs/vlogs.yaml fails on an empty
55+
logsStorages), so the "Ready with no log store" hole cannot be reached.
56+
57+
The traces backend (VTCluster / VTSingle) shares this operator and the same
58+
status.updateStatus contract, so it is gated identically below. Unlike logs,
59+
tracingStorages is EMPTY by default (tracing off), so usually nothing renders
60+
and these exprs are inert: Flux evaluates them only against CRs in the
61+
release inventory. "Ready with no trace store" is intentional and harmless:
62+
nothing produces spans until the collector ships, so no data is dropped
63+
(hence no fail-on-empty guard yet, unlike the logs #3181 guard which fires
64+
because fluent-bit ships logs unconditionally).
65+
*/}}
5166
healthCheckExprs:
5267
- apiVersion: operator.victoriametrics.com/v1
5368
kind: VLCluster
5469
inProgress: has(status.updateStatus) && status.updateStatus == 'expanding'
5570
failed: has(status.updateStatus) && status.updateStatus != 'operational' && status.updateStatus != 'expanding'
5671
current: has(status.updateStatus) && status.updateStatus == 'operational'
72+
- apiVersion: operator.victoriametrics.com/v1
73+
kind: VTCluster
74+
inProgress: has(status.updateStatus) && status.updateStatus == 'expanding'
75+
failed: has(status.updateStatus) && status.updateStatus != 'operational' && status.updateStatus != 'expanding'
76+
current: has(status.updateStatus) && status.updateStatus == 'operational'
77+
- apiVersion: operator.victoriametrics.com/v1
78+
kind: VTSingle
79+
inProgress: has(status.updateStatus) && status.updateStatus == 'expanding'
80+
failed: has(status.updateStatus) && status.updateStatus != 'operational' && status.updateStatus != 'expanding'
81+
current: has(status.updateStatus) && status.updateStatus == 'operational'
5782
valuesFrom:
5883
- kind: Secret
5984
name: cozystack-values

‎packages/extra/monitoring/tests/helmrelease_test.yaml‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,45 @@ tests:
4242
- equal:
4343
path: spec.healthCheckExprs[0].failed
4444
value: "has(status.updateStatus) && status.updateStatus != 'operational' && status.updateStatus != 'expanding'"
45+
46+
# The traces backend (VTCluster/VTSingle from tracingStorages) is gated the
47+
# same way and by the same operator status contract. tracingStorages is empty
48+
# by default, so these exprs are usually inert (no CR to match); when a backend
49+
# is configured they gate readiness on it. Pin both entries so a
50+
# dropped/reordered block or a typo'd CEL expression fails a test rather than
51+
# silently regressing the gate.
52+
- it: gates readiness on the VTCluster status.updateStatus
53+
asserts:
54+
- equal:
55+
path: spec.healthCheckExprs[1].apiVersion
56+
value: operator.victoriametrics.com/v1
57+
- equal:
58+
path: spec.healthCheckExprs[1].kind
59+
value: VTCluster
60+
- equal:
61+
path: spec.healthCheckExprs[1].current
62+
value: "has(status.updateStatus) && status.updateStatus == 'operational'"
63+
- equal:
64+
path: spec.healthCheckExprs[1].inProgress
65+
value: "has(status.updateStatus) && status.updateStatus == 'expanding'"
66+
- equal:
67+
path: spec.healthCheckExprs[1].failed
68+
value: "has(status.updateStatus) && status.updateStatus != 'operational' && status.updateStatus != 'expanding'"
69+
70+
- it: gates readiness on the VTSingle status.updateStatus
71+
asserts:
72+
- equal:
73+
path: spec.healthCheckExprs[2].apiVersion
74+
value: operator.victoriametrics.com/v1
75+
- equal:
76+
path: spec.healthCheckExprs[2].kind
77+
value: VTSingle
78+
- equal:
79+
path: spec.healthCheckExprs[2].current
80+
value: "has(status.updateStatus) && status.updateStatus == 'operational'"
81+
- equal:
82+
path: spec.healthCheckExprs[2].inProgress
83+
value: "has(status.updateStatus) && status.updateStatus == 'expanding'"
84+
- equal:
85+
path: spec.healthCheckExprs[2].failed
86+
value: "has(status.updateStatus) && status.updateStatus != 'operational' && status.updateStatus != 'expanding'"

‎packages/extra/monitoring/values.schema.json‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,64 @@
323323
}
324324
}
325325
},
326+
"tracingStorages": {
327+
"description": "Configuration of traces storage instances. Empty by default (tracing off): nothing writes spans until the trace collector ships, so no backend is provisioned. Add an entry to enable a VictoriaTraces backend.",
328+
"type": "array",
329+
"default": [],
330+
"items": {
331+
"type": "object",
332+
"required": [
333+
"mode",
334+
"name",
335+
"retentionPeriod",
336+
"storage",
337+
"storageClassName"
338+
],
339+
"properties": {
340+
"mode": {
341+
"description": "Deployment mode: \"cluster\" renders a VTCluster; \"single\" renders a single-binary VTSingle for edge/dev/small clusters. Treat as immutable once a backend exists: mode selects which Kind is rendered under the same name, so changing it on a running backend re-identifies the object rather than reconfiguring it. Switching from \"single\" to \"cluster\" prunes the VTSingle and the operator garbage-collects its PVC (stored traces are lost); switching from \"cluster\" to \"single\" (or removing the entry) leaves the vtstorage PVCs Bound and billed until the whole Monitoring release is deleted. The API cannot guard this (transition rules are not available on an array-item field), so this note is the only guard: migrate deliberately, accepting the loss.",
342+
"type": "string",
343+
"default": "cluster",
344+
"enum": [
345+
"cluster",
346+
"single"
347+
]
348+
},
349+
"name": {
350+
"description": "Name of the storage instance: a lowercase RFC 1123 label of at most 42 characters. Treat as immutable once a backend exists: the VTCluster/VTSingle, its Services, PVCs and Grafana datasource are all named after it, so renaming an entry deploys a new, empty backend and removes the old one, the same way changing mode does (single mode loses the old PVC, cluster mode leaves the old vtstorage PVCs Bound until the whole Monitoring release is deleted).",
351+
"type": "string"
352+
},
353+
"retentionDiskUsageBytes": {
354+
"description": "Optional disk-usage cap as a VictoriaMetrics BytesString (e.g. \"8GB\" or \"8GiB\"), bounding disk use on top of the age-based retentionPeriod. Uses byte-suffix grammar (kb/mb/gb/tb, KiB/MiB/GiB/TiB), NOT the Kubernetes-quantity grammar of the sibling storage field, so \"8Gi\" is rejected by the operator. In cluster mode the cap is per vtstorage replica (replicaCount 2), so the effective total is twice the value.",
355+
"type": "string"
356+
},
357+
"retentionPeriod": {
358+
"description": "Retention period for traces.",
359+
"type": "string",
360+
"default": "14d"
361+
},
362+
"storage": {
363+
"description": "Persistent volume size.",
364+
"default": "10Gi",
365+
"pattern": "^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$",
366+
"anyOf": [
367+
{
368+
"type": "integer"
369+
},
370+
{
371+
"type": "string"
372+
}
373+
],
374+
"x-kubernetes-int-or-string": true
375+
},
376+
"storageClassName": {
377+
"description": "StorageClass used to store the data.",
378+
"type": "string",
379+
"default": "replicated"
380+
}
381+
}
382+
}
383+
},
326384
"alerta": {
327385
"description": "Configuration for the Alerta service.",
328386
"type": "object",

0 commit comments

Comments
 (0)