Repository navigation
Add design proposal: Distributed tracing via OTLP and VictoriaTraces - #38
Conversation
Signed-off-by: Alexey Artamonov <[email protected]>
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds a distributed tracing design proposal covering VictoriaTraces storage, staged OTLP ingestion, Grafana correlation, per-application opt-in, tenant isolation, testing, and rollout behavior. ChangesDistributed tracing proposal
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a design proposal for integrating distributed tracing into the Cozystack monitoring stack using OTLP and VictoriaTraces. The feedback recommends making the replica counts configurable to prevent scheduling failures in resource-constrained environments, addressing cross-chart dependency issues when tenant apps have tracing enabled but no backend is configured, and supporting both VTCluster and VTSingle deployments to align with existing patterns.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| vtinsert: { replicaCount: 2 } | ||
| vtselect: { replicaCount: 2 } | ||
| vtstorage: | ||
| retentionPeriod: {{ .retentionPeriod | quote }} | ||
| replicaCount: 2 |
There was a problem hiding this comment.
Hardcoding replicaCount: 2 for the VictoriaTraces components in the template can lead to scheduling failures in single-node or resource-constrained environments (such as local development or small testing clusters) due to pod anti-affinity or resource limits. Making the replica counts configurable within the tracingStorages schema, or dynamically defaulting them based on the cluster topology, would improve deployment flexibility.
| vtinsert: { replicaCount: 2 } | |
| vtselect: { replicaCount: 2 } | |
| vtstorage: | |
| retentionPeriod: {{ .retentionPeriod | quote }} | |
| replicaCount: 2 | |
| vtinsert: { replicaCount: {{ .replicaCount | default 2 }} }\n vtselect: { replicaCount: {{ .replicaCount | default 2 }} }\n vtstorage:\n retentionPeriod: {{ .retentionPeriod | quote }}\n replicaCount: {{ .replicaCount | default 2 }} |
|
|
||
| ## Failure and edge cases | ||
|
|
||
| - Empty `tracingStorages` while a consumer expects a backend → loud render failure (mirrors `cozystack/cozystack#3181`), never a silent span black hole. |
There was a problem hiding this comment.
While failing the render of the monitoring chart on an empty tracingStorages list prevents a silent black hole within that release, tenant applications (which are separate Helm releases) won't be aware of this at render time. If a tenant app has tracing.enabled: true but tracingStorages is empty, the app will attempt to send traces to a non-existent collector or backend, potentially causing connection timeouts or DNS errors. Detailing how this cross-chart dependency is managed, or if the platform collector should always be present (perhaps with a dummy/noop exporter) to safely discard spans when no backend is configured, would make the architecture more robust.
| ## Open questions | ||
|
|
||
| - **VictoriaTraces maturity**: the CRDs ship in the operator, but is VictoriaTraces production-ready at the version Cozystack pins? If not, Grafana Tempo is the drop-in fallback (see Alternatives) — the collector and per-app surfaces are backend-agnostic, so only the backend template and datasource type change. | ||
| - **`VTCluster` vs `VTSingle`** as the default: cluster for HA parity with metrics/logs, or single for a lighter footprint on small clusters? |
There was a problem hiding this comment.
Regarding the choice between VTCluster and VTSingle: to align with Cozystack's existing patterns for VictoriaMetrics and VictoriaLogs, supporting both is highly recommended. A VTSingle deployment is ideal for edge, single-node, or development environments to minimize overhead, while VTCluster should be used for production environments requiring high availability and scalability.
…ture Direct-to-vtinsert MVP (mirrors logs) then an opt-in OpenTelemetry Collector gateway; ExternalName redirect via cozystack-basics; poller readiness gate on VTCluster; clarify WorkloadMonitor is operational-only so tracing opt-in lives in app values. Signed-off-by: Alexey Artamonov <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
design-proposals/distributed-tracing/README.md (2)
126-128: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftSpecify how RED/span metrics are produced.
Direct Stage A ingestion creates no metrics, and the proposal does not define a Collector spanmetrics connector, application metric source, exemplars, or Grafana
tracesToMetricsqueries. Grafana’s trace-to-metrics feature links to pre-existing metrics; it does not generate them automatically. (grafana.com)Also applies to: 193-195
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-proposals/distributed-tracing/README.md` around lines 126 - 128, Update the distributed tracing proposal’s RED/span metrics design to explicitly specify their source and production path, including whether they come from a Collector spanmetrics connector or application instrumentation. Define how exemplars are attached and provide the Grafana tracesToMetrics queries/configuration that consume the resulting metrics, covering both the trace-to-metrics and metrics-to-trace links.Source: MCP tools
72-90: 🩺 Stability & Availability | 🔵 TrivialDo not equate
vtstorage.replicaCountwith HA replication.VictoriaTraces cluster mode does not replicate data between storage nodes; losing one node can make complete queries fail. If “HA parity” is intended, document this limitation and use Collector replication to independent VictoriaTraces clusters or another supported durability design. (docs.victoriametrics.com)
Also applies to: 208-208
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-proposals/distributed-tracing/README.md` around lines 72 - 90, Update the VTCluster template documentation and configuration guidance around the tracingStorages render to state that vtstorage.replicaCount provides no data replication or HA durability. Document the limitation and direct HA deployments to replicate through Collectors into independent VictoriaTraces clusters or use another supported durability design; do not present the existing replicaCount settings as HA parity.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design-proposals/distributed-tracing/README.md`:
- Around line 72-103: The tracing template must distinguish an absent
tracingStorages value from an explicitly empty list and enforce the intended
behavior consistently. Update the VTCluster rendering template and related tests
so the chosen contract is encoded: preserve successful rendering when the list
is absent if that remains supported, but fail rendering for an explicitly empty
list; ensure tests cover both cases and the guard produces a clear failure.
- Around line 186-187: Update the “Backend storage exhausted” failure mode to
distinguish age-based retention from VictoriaTraces disk-retention limits,
referencing vtstorage.retentionDiskSpaceUsage or the equivalent retention flags.
Document that PVC exhaustion can block ingest before old traces are evicted, and
add capacity alerts plus a test covering PVC exhaustion.
- Around line 112-120: Update the Stage A design around the ExternalName
redirect to define how OTLP ports 4317/4318 and the OTLP/HTTP path are served by
vtinsert, whose defaults are port 10481 and /insert/opentelemetry/v1/traces.
Specify an explicit Service/listener port mapping or path-aware proxy, rather
than relying on ExternalName DNS aliasing alone, while preserving the existing
monitoring-enabled gating and tenant routing.
- Around line 114-120: Update the Stage B collector deployment design to
preserve trace affinity for tail sampling: run the sampling tier with a single
replica, or introduce trace-ID-aware routing/load balancing before multiple
Collector replicas. Do not rely on a regular Kubernetes Service alone to
distribute traffic to the Collector gateway.
- Around line 112-118: Update the distributed-tracing proposal so Stage A
explicitly injects VictoriaTraces AccountID and ProjectID headers using
authenticated workload identity, via the application, trusted proxy, or exporter
rather than relying on the ExternalName redirect. Extend Stage B’s Collector
design to map the authenticated tenant identity to those VictoriaTraces headers
in the OTLP exporter path, in addition to stamping the tenant resource
attribute.
---
Nitpick comments:
In `@design-proposals/distributed-tracing/README.md`:
- Around line 126-128: Update the distributed tracing proposal’s RED/span
metrics design to explicitly specify their source and production path, including
whether they come from a Collector spanmetrics connector or application
instrumentation. Define how exemplars are attached and provide the Grafana
tracesToMetrics queries/configuration that consume the resulting metrics,
covering both the trace-to-metrics and metrics-to-trace links.
- Around line 72-90: Update the VTCluster template documentation and
configuration guidance around the tracingStorages render to state that
vtstorage.replicaCount provides no data replication or HA durability. Document
the limitation and direct HA deployments to replicate through Collectors into
independent VictoriaTraces clusters or use another supported durability design;
do not present the existing replicaCount settings as HA parity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2261fb92-b1e9-4672-ad8a-0b19c956a39c
📒 Files selected for processing (1)
design-proposals/distributed-tracing/README.md
VerdictLGTM with non-blocking notes This is a Draft design proposal (docs-only, single
The design is conservative, single-vendor, additive/opt-in, and reuses established conventions. Good doc. The notes below are for the design discussion, none block merging a Draft. Findings (non-blocking)[MINOR] Stage A read-side tenant isolation is under-specified. Security section frames per-tenant [MINOR] Jaeger-compatible datasource type is the one unverified external claim. [NIT] Leftover authoring instruction in committed doc. Line 1 is [NIT] Sketch Notes on the prior gemini-code-assist review
Phase 5b (upgrade / fresh-install impact)N/A — the diff touches only |
…y, disk retention - Stage A: document AccountID/ProjectID tenant-header injection and the real vtinsert OTLP contract (port 10481, /insert/opentelemetry/v1/traces); ExternalName aliases DNS only. - Stage B: otlphttp exporter sets tenant headers; tail-sampling trace affinity (single replica or loadbalancingexporter routing_key=traceID). - Backend: configurable replicaCount; retentionDiskSpaceUsage vs age retention; durability caveat (replicaCount != data replication); absent-vs-empty contract. - Grafana: span-metrics need a spanmetrics connector; read-side tenant isolation. - Security/Failure/Testing/Open questions updated accordingly. - Drop leftover authoring comment; add application.group managedMetadata label. Signed-off-by: Alexey Artamonov <[email protected]>
|
Thanks for the reviews — IvanHunters and the bots. Pushed Addressed in the doc
Already covered (pre-existing)
Everything requiring live-cluster verification (Jaeger datasource type, exact operator field names like |
Signed-off-by: Alexey Artamonov <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design-proposals/distributed-tracing/README.md`:
- Around line 123-135: The Stage A-to-Stage B migration contract uses
incompatible ports and paths that an ExternalName alone cannot translate. Update
the Stage A and Stage B endpoint descriptions and all corresponding references
to choose one stable application-facing OTLP contract, adding explicit
proxy/listener port and path mapping in both stages, or explicitly require
applications to change endpoints during promotion. Ensure the guidance is
consistent across the nearby repeated sections.
- Around line 66-67: Update the retentionDiskSpaceUsage configuration and its
related references to accept only percentage values, since rendering uses
retentionMaxDiskSpaceUsagePercent. Remove byte-value examples and documentation,
and validate or clearly constrain inputs to percent formats consistently at the
additional referenced locations.
- Around line 125-131: Update the Stage B gateway design around the otlphttp
exporter so AccountID/ProjectID are injected per request from authenticated
tenant identity, not configured as static headers. Specify a trusted per-tenant
proxy or a Collector mechanism such as headers_setter using validated request
metadata, and ensure clients cannot spoof these routing headers; retain the
tenant resource attribute for query-time labeling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: afb3914c-e395-4e33-a027-b5da1dd9a7d8
📒 Files selected for processing (1)
design-proposals/distributed-tracing/README.md
…act, headers_setter - retentionDiskUsagePercent is percent-only (maps to retentionMaxDiskSpaceUsagePercent); drop the ambiguous bytes-or-percent field. - Fix the Stage A->B endpoint contradiction: fix the app-facing contract to a port-explicit OTLP/gRPC :4317 Service (bare ExternalName can't carry port/path); gRPC chosen because vtinsert and collector OTLP/HTTP paths differ. - Stage B tenant headers via headers_setter extension (from_context), not static otlphttp.headers which cannot derive per-tenant AccountID/ProjectID. Signed-off-by: Alexey Artamonov <[email protected]>
|
Pushed
These were all consequences of the first-pass half-fixes; the design is now internally consistent. Remaining live-cluster specifics (exact operator field/flag names, Jaeger datasource type) stay flagged as confirm-before-implementation. |
…ification Blocking fixes from /branch-review, verified against the pinned operator CRD: - VTCluster sub-spec keys are spec.insert/select/storage (prefix dropped), NOT vtinsert/vtselect/vtstorage; rendered workloads/services keep the vt prefix. - Disk cap field is spec.storage.retentionMaxDiskSpaceUsageBytes (bytes); no percent CR field exists. values field is now retentionDiskUsageBytes (bytes); earlier percent-only mapping was inverted. - Topology: monitoring installs into cozy-monitoring; ExternalName redirects live in cozy-monitoring and resolve to *.tenant-root.svc.cluster.local. - Nits: storageSize values key (avoid triple-storage stutter); WorkloadMonitor wording tightened + redis listed; confirm-before-impl caveat lists OTLP wire details + service-name prefix. Signed-off-by: Alexey Artamonov <[email protected]>
Keep the substantive points; remove the review-attribution asides so the document reads as a standalone design rather than a review-response artifact. Signed-off-by: Alexey Artamonov <[email protected]>
VerdictLGTM with non-blocking notes A well-researched, technically accurate design proposal for the third observability signal; verification against the actual Findings[MINOR] Checked against [MINOR] Verified against the real [MINOR] Security section ( The new OTLP Collector Deployment and the new per-app sidecar/agent containers (Kafka, RabbitMQ, MariaDB, Redis, Postgres) run inside Claim mismatches[PARTIAL] PR body states Caveats
Recommended follow-ups
|
…ence Signed-off-by: Alexey Artamonov <[email protected]>
…x ExternalName wording - Resolve read-side tenant isolation open question: structural isolation (isolated per-tenant stack + same-namespace vtselect datasource + NetworkPolicy, account 0), matching logs/metrics; vtselect has no authz so a datasource header pin is routing, not a security boundary; shared backend needs vmauth (deferred). Reconcile Grafana + Security sections. - Resolve VTCluster-vs-VTSingle: per-entry mode: cluster|single (default cluster). - Fix contradiction on the no-backend failure mode: Service has no backing endpoints, not 'ExternalName resolves to nothing' (design uses a port-explicit Service, not a bare ExternalName). - Soften the vtinsert-* service-name prefix to confirm-before-implementation. - Align write-path AccountID/ProjectID framing with the current account-0 + tenant-label convention. - Add revised-date suffix. Signed-off-by: Alexey Artamonov <[email protected]>
d83042a to
3cb278c
Compare
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
Verdict
LGTM
Third pass on this proposal. My earlier findings (leftover authoring line, missing application.group label in the sketch, the unverified query-string tenant precedence, and the under-specified read-side isolation) are all resolved. I re-verified the load-bearing claims against the current cozystack/cozystack main and they hold: VTCluster's prefix-less spec.insert/select/storage vs VLCluster's vl* keys, the spec.storage.retentionMaxDiskSpaceUsageBytes/retentionPeriod fields (no percent CR field), the #3181 empty-list fail-guard, the poller healthCheckExprs gate on status.updateStatus, and the _cluster.monitoring-enabled ExternalName topology.
Approving. Two non-blocking notes remain for the implementation phase.
Findings (non-blocking)
[MINOR] The default tracingStorages values ship the exact configuration the Failure section warns against. The sketch defaults retentionDiskUsageBytes: "" (no disk cap) with storageSize: 10Gi and a 14d age retention, while "Storage full" correctly notes that without a disk cap a full PVC blocks ingest before age eviction. Traces are typically higher-volume than logs, so a 10Gi PVC with no cap can fill well before 14d. The doc prescribes the mitigation (retentionMaxDiskSpaceUsageBytes + a capacity alert) but the proposed default does not apply it. Consider shipping a default disk cap (e.g. derived from storageSize) so the out-of-box config is safe, or state explicitly that the default is unsafe for a high-volume trace backend.
[MINOR] Pod-security posture for the new workloads is still unaddressed. The Stage-B Collector Deployment and the per-app sidecar/agent containers (Kafka, RabbitMQ, MariaDB, Redis, Postgres) run in tenant-* namespaces under PSS-restricted. The Security section covers trust boundaries, RBAC, and TLS but says nothing about non-root, dropped capabilities, or seccomp for these new containers — worth a line since every other new-workload class in this stack states its pod-security posture up front.
|
Really like how conservative this is — reusing the 1. The default config reproduces the disk-full failure mode it documents. The proposal applies "safe by default" to the render guard ( Suggestion: when storage:
retentionPeriod: {{ .retentionPeriod | quote }}
retentionMaxDiskSpaceUsageBytes: {{ .retentionDiskUsageBytes | default (/* ~0.85 * storageSize in bytes */) | quote }}(exact byte computation TBD, but the intent is: no entry should ship without a disk bound.) 2. In the sketch a single insert: { replicaCount: {{ .replicaCount | default 2 }} }
select: { replicaCount: {{ .replicaCount | default 2 }} }
storage: { replicaCount: {{ .replicaCount | default 2 }}, ... }These tiers are rarely sized identically — insert scales with ingest, select with query load, storage with retention/throughput — and Neither blocks the design — both are values-surface refinements. 👍 on the staged A→B ingest behind a stable |
…eplicas, storageSize rationale, PSS - Mattia #1: default is always disk-bounded — retentionMaxDiskSpaceUsageBytes derived from storageSize (~85%) when retentionDiskUsageBytes is unset, so the out-of-the-box config can't silently fill the PVC. Update Failure + Testing. - Mattia #2: replicaCount documented as a uniform shortcut with optional per-tier insert/select/storage overrides (as VMCluster), storage tier durability-adjacent. - IvanHunters: inline rationale for the storageSize key name (avoids spec.storage.storage stutter with the VTCluster CRD's nested storage object). - IvanHunters: add Pod Security Standards posture for the new Collector Deployment and per-app OTLP sidecars (restricted: non-root, drop ALL, seccomp RuntimeDefault, no privilege escalation). - Bump revised-date to 2026-07-23. Signed-off-by: Alexey Artamonov <[email protected]>
Design reviewThe proposal is careful and well-hedged — it flags unverified claims as BlockerSpan content (PII / secrets) is not addressed in the Security section. Should be resolved before implementation
Nits
Verdict: needs work (accept the direction, close the gaps). The blocker |
|
|
||
| Grafana datasources are provisioned as `GrafanaDatasource` CRs by grafana-operator, one per storage: `packages/system/monitoring/templates/vm/grafana-datasource.yaml` (type `prometheus`, per metrics tier) and `packages/system/monitoring/templates/vlogs/grafana-datasource.yaml` (type `victoriametrics-logs-datasource`, per logs storage). Every datasource attaches to Grafana through `instanceSelector: { matchLabels: { dashboards: grafana } }`. | ||
|
|
||
| Applications expose metrics today but there is no tracing surface. Most managed engines declare a `WorkloadMonitor` CR (clickhouse, kafka, rabbitmq, nats, mariadb, redis) and/or a native operator mechanism (postgres/CNPG `enablePodMonitor`, redis's `redis_exporter` sidecar + `VMServiceScrape`). The one app with an explicit observability toggle is foundationdb: `monitoring.enabled` in `packages/apps/foundationdb/values.yaml` gates whether its `WorkloadMonitor` renders — that toggle is the shape a `tracing.enabled` switch should copy. Note that `WorkloadMonitor` itself is not a fit for carrying tracing config: its controller (`internal/controller/workloadmonitor_controller.go`) reconciles it into `Workload` objects that track replicas/resources/operational status (and query Prometheus for resource usage) for the dashboard and billing surfaces — the actual metric scrape configs are rendered by the individual app charts, not by this controller, so tracing opt-in belongs in each app's `values.yaml`, not in `WorkloadMonitor`. |
There was a problem hiding this comment.
WorkloadMonitor is not related to metrics production or collection in almost any way. FoundationDB is wrong to hide its WorkloadMonitor behind a tenant-controllable toggle. WorkloadMonitors are meta-resources that hold a label selector to identify pods, services, and PVCs belonging to a certain app in order to bill them correctly.
There was a problem hiding this comment.
Corrected. Context now describes WorkloadMonitor accurately as a billing/ownership meta-resource — a label selector identifying an app's pods/services/PVCs, reconciled into Workload objects — not a metrics-collection mechanism. foundationdb's monitoring.enabled is explicitly called out as the wrong model (it hides a billing meta-resource behind a tenant toggle), not a template for tracing.
|
|
||
| Applications expose metrics today but there is no tracing surface. Most managed engines declare a `WorkloadMonitor` CR (clickhouse, kafka, rabbitmq, nats, mariadb, redis) and/or a native operator mechanism (postgres/CNPG `enablePodMonitor`, redis's `redis_exporter` sidecar + `VMServiceScrape`). The one app with an explicit observability toggle is foundationdb: `monitoring.enabled` in `packages/apps/foundationdb/values.yaml` gates whether its `WorkloadMonitor` renders — that toggle is the shape a `tracing.enabled` switch should copy. Note that `WorkloadMonitor` itself is not a fit for carrying tracing config: its controller (`internal/controller/workloadmonitor_controller.go`) reconciles it into `Workload` objects that track replicas/resources/operational status (and query Prometheus for resource usage) for the dashboard and billing surfaces — the actual metric scrape configs are rendered by the individual app charts, not by this controller, so tracing opt-in belongs in each app's `values.yaml`, not in `WorkloadMonitor`. | ||
|
|
||
| Crucially, the tracing backend needs no new operator. The victoria-metrics-operator Cozystack already runs (appVersion `v0.68.4`, `packages/system/victoria-metrics-operator`) ships the VictoriaTraces CRDs `VTCluster` and `VTSingle` (`packages/system/victoria-metrics-operator/charts/victoria-metrics-operator/crd.yaml`, CRDs `vtclusters.operator.victoriametrics.com` and `vtsingles.operator.victoriametrics.com`). `VTCluster` decomposes into VTInsert/VTStorage/VTSelect components — conceptually analogous to `VLCluster`'s insert/storage/select — but note the exact spec keys differ: `VLCluster` uses `vlinsert`/`vlselect`/`vlstorage`, whereas `VTCluster` **drops the prefix** and uses `spec.insert`/`spec.select`/`spec.storage` (verified against the CRD's printer columns `.spec.insert.replicaCount` etc.). The rendered workloads/services are *expected* to carry the `vt` prefix (`vtinsert-*`/`vtselect-*`/`vtstorage-*`), same as `VLCluster` yields `vlinsert-*` — this prefix is load-bearing for the Stage A `otel-traces:4317` target, so it stays a **confirm-before-implementation** item (see Open questions) rather than an asserted fact. So the template is *shaped* like `vlogs.yaml` but the sub-spec keys are not a verbatim copy — this is the one place the analogy misleads, and it is corrected throughout below. |
There was a problem hiding this comment.
I'm happy to adopt VictoriaTraces as the primary tracing engine, but at this point I'm concerned that we're leaning heavily on a single vendor for the entire observability stack. In the interest of vendor neutrality I would like an honest assessment if letting Cozystack admins pick and choose their observability solution is viable.
There was a problem hiding this comment.
Added an honest assessment in §1: the backend sits behind a backend-agnostic OTLP + Grafana-datasource seam, so an admin selects VictoriaTraces (default, no new operator), Tempo, or Jaeger at install time and the choice never leaks into applications or the tenancy design. Trade-off recorded in Alternatives.
|
|
||
| ### 1. Backend: VictoriaTraces (`tracingStorages`) | ||
|
|
||
| Add a `tracingStorages` list to the monitoring values, parallel to `metricsStorages` and `logsStorages`, in both `packages/system/monitoring/values.yaml` and `packages/extra/monitoring/values.yaml`: |
There was a problem hiding this comment.
I'm not sure if the existing schema for logs and metrics is the best model to copy. I expect a better design to emerge if the design work is done without the biases of the existing observability stack. The logs' and metrics' stores could adopt something new from the proposed tracing system.
There was a problem hiding this comment.
Agreed — dropped "mirror logs/metrics one-for-one" as the guiding principle (Overview + Alternatives). §2 is the concrete case where copying the scrape-based model would have designed straight into the tenant-isolation problem. I also noted the reverse flow you'd expect: the VTSingle single-binary mode is something the metrics/logs stacks could usefully adopt.
|
|
||
| The per-entry `replicaCount` is a **uniform shortcut**: it seeds all three cluster tiers so a simple config stays one line. Because the tiers are rarely sized identically in practice — `insert` scales with ingest, `select` with query load, `storage` with retention/throughput, and `storage.replicaCount` is durability-adjacent rather than a query-scaling knob — each tier accepts an optional explicit override (`insert`/`select`/`storage` `replicaCount`), mirroring how `VMCluster` exposes per-component counts; an unset tier falls back to the shortcut. The PVC-size key is named `storageSize` rather than the sibling `storage` (used by `metricsStorages`/`logsStorages`) on purpose: the `VTCluster` CRD already nests a second `storage` object under `spec.storage` (`spec.storage.storage.volumeClaimTemplate`), so reusing `storage` as the values key would read as `spec.storage.storage` stutter — the rename keeps the values surface unambiguous. | ||
|
|
||
| Render one backend CR per entry in a new `templates/vtraces/vtraces.yaml`, shaped like `templates/vlogs/vlogs.yaml` (but with the VictoriaTraces prefix-less sub-spec keys). The per-entry `mode` selects the CR kind: `cluster` (default) renders a `VTCluster` — the multi-component variant used below and the same choice `metricsStorages`/`logsStorages` make with `VMCluster`/`VLCluster` in every stack today; `single` renders a single-binary `VTSingle` for edge/dev/small clusters where the cluster footprint is unwarranted (its sub-spec is flatter — no separate insert/select/storage components — so `replicaCount` and the storage/retention fields apply to the one workload). Both carry: `managedMetadata` labels for application ownership, configurable replica counts, `retentionPeriod` from the entry, the storage-PVC claim-template label for the release-scoped post-delete cleanup hook, and the `cozystack/cozystack#3181` guard (see the absent-vs-empty contract below). |
There was a problem hiding this comment.
singlerenders a single-binaryVTSinglefor edge/dev/small clusters where the cluster footprint is unwarranted
This is an excellent idea and I would like to see this in the metrics stack too.
There was a problem hiding this comment.
Noted in §1 as a mode the metrics and logs stacks would benefit from adopting too.
|
|
||
| Render one backend CR per entry in a new `templates/vtraces/vtraces.yaml`, shaped like `templates/vlogs/vlogs.yaml` (but with the VictoriaTraces prefix-less sub-spec keys). The per-entry `mode` selects the CR kind: `cluster` (default) renders a `VTCluster` — the multi-component variant used below and the same choice `metricsStorages`/`logsStorages` make with `VMCluster`/`VLCluster` in every stack today; `single` renders a single-binary `VTSingle` for edge/dev/small clusters where the cluster footprint is unwarranted (its sub-spec is flatter — no separate insert/select/storage components — so `replicaCount` and the storage/retention fields apply to the one workload). Both carry: `managedMetadata` labels for application ownership, configurable replica counts, `retentionPeriod` from the entry, the storage-PVC claim-template label for the release-scoped post-delete cleanup hook, and the `cozystack/cozystack#3181` guard (see the absent-vs-empty contract below). | ||
|
|
||
| **Absent vs empty contract** (the two must not be conflated): an **absent/unset** `tracingStorages` key means "tracing disabled", and the template renders nothing and succeeds (so tracing stays opt-in and a cluster that never wanted traces is never broken). An **explicitly empty list** (`tracingStorages: []`) is a misconfiguration — a consumer asked for tracing but declared no backend — and **fails the render**, exactly like the logs `#3181` guard. Both cases must be covered by tests. |
There was a problem hiding this comment.
It might not be straightforward to declare the absence of a key in a Helm release. What would the default value in values.yaml be?
Also is it not premature to lock in in such great detail the exact shapes of Helm values at the early stage of a community proposal?
There was a problem hiding this comment.
Agreed on both. Removed the exact values schema and the absent-vs-empty Helm mechanics from the body into a non-normative Implementation notes appendix; Non-goals now states the value schema is settled in the implementation PRs. I deliberately did not lock in a default-key representation here, since that's exactly the kind of detail you asked to defer — the body states only the principle (tracing is opt-in; a backend requested-but-empty is a misconfiguration).
|
Thanks Timofei Larkin (@lllamnyp) — pushed Platform facts
Design gaps
Editorial
Non-blocking — all applied: the seam now reads "plus the backend-specific operational wiring"; the shared-central egress rule endpoint-selects only the collector (not all tenant pods, the way the virt-handler rule selects vmagent); and a one-line future-proofing note for the unwired Ready for another look. |
…ide via allow-to-ingress
branch-review caught that the Cilium snapshot conflated two rules: the
per-tenant -egress block pins cozystack.io/service: ingress to ancestor
namespaces (like vminsert/etcd), while the separate allow-to-ingress
policy (endpointSelector: {}) reaches ingress endpoints cluster-wide.
Word both accurately.
Signed-off-by: Alexey Artamonov <[email protected]>
Drop the specific client reference (hidora); the motivation is request-level visibility requested by Cozystack adopters generally. Signed-off-by: Alexey Artamonov <[email protected]>
Timofei Larkin (lllamnyp)
left a comment
Timofei Larkin (lllamnyp)
left a comment
There was a problem hiding this comment.
Approving. The 2026-07-31 – 2026-08-03 revisions resolve everything from my 2026-07-30 review, and I re-checked the corrected platform claims against cozystack/cozystack main rather than taking the fixes on trust: the global.target bundle override (chart-default cozy-monitoring, overridden to tenant-root in packages/core/platform/templates/bundles/system.yaml), the non-exhaustive egress framing with the label-gated apiserver egress and the cluster-wide allow-to-ingress scope, the corrected PSS posture, and the ClickHouse opentelemetry_span_log remove="1" finding — all accurate. The per-engine visibility table and the span-content/redaction bullet close the last substantive gaps, including the blocker IvanHunters raised on 2026-07-28.
One note for the record, not blocking: the isolation paragraph now enumerates vminsert/etcd as the reachable tenant-root endpoints; cozystack.io/service: ingress-labeled endpoints are reachable too (via both the ancestor-pinned -egress block and allow-to-ingress). The conclusion §2 rests on — no generic pod in tenant-root is reachable, cozy-monitoring is not reachable at all — is unchanged.
One broader remark, so this approval is not misread as an endorsement of the status quo. Cozystack's observability stack today serves platform admins far more than it serves the people who run workloads on Cozystack — it grew as an admin tool, and it shows. There is a real task out there to redesign observability around users, and it is bigger than any one signal. That is not this proposal's fault and not its job: within its chosen scope — adding the third signal alongside the logs and metrics we already have, without breaching tenancy — the design is sound, verified, and honest about its limits. When the user-facing rework happens, the seams established here (OTLP ingest as the contract, the per-tenant trust boundary, the pluggable backend) are the parts I expect to survive it.
Phase 2 of distributed tracing (design cozystack/community#38, epic #3761): a per-tenant OpenTelemetry Collector as the app-facing OTLP ingest gateway and trust boundary. Plain Deployment + ConfigMap + Service (no OTel operator exists in the platform), deployed in the tenant namespace so app->collector traffic is intra-namespace and never crosses the tenant boundary. - receivers OTLP gRPC :4317 / HTTP :4318; memory_limiter + head sampling (probabilistic_sampler, default 10%) + optional attribute redaction (PII/secret control point) + batch; exports OTLP/HTTP to the tracingStorages backend (vtinsert:10481 for cluster, vtsingle:10428 for single mode) - app-facing Service 'otel-traces' (4317/4318); PSS-restricted-compliant pod and container securityContext; config-checksum annotation to roll on config change - tracingCollector values (enabled/samplingPercentage/redactAttributes/resources) in extra (annotated) + system (image pinned via cozy-lib.image); regenerated values.schema.json / README / ApplicationDefinition; 13 helm-unittest cases Deferred to later phases (epic #3761): AccountID/ProjectID header injection for the shared-central topology (phase 4); engine integrations (phase 3). This PR stacks on the phase-1 backend PR #3762. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 4 of distributed tracing (design cozystack/community#38, epic #3761): the opt-in shared-central topology where per-tenant collectors export to a single shared VictoriaTraces in tenant-root instead of a per-tenant backend. - collector: tracingCollector.backend=local|central. In central mode it exports over OTLP/HTTP to vtinsert-generic (ExternalName -> tenant-root) and a resource processor stamps tenant=<namespace>, so all tenants share account 0 while staying distinguishable. No local backend/datasource is rendered in central mode (the #3181 empty-list and multi-backend guards become local-only). - tenant NetworkPolicy: a narrow CiliumClusterwideNetworkPolicy scoped to the otel-traces collector pods only, allowing egress to the ancestor vtinsert (mirrors the vminsert egress block but endpoint-scoped, so no other tenant workload gains the cross-boundary hop). Non-root tenants only. - cozystack-basics: vtinsert-generic ExternalName in cozy-monitoring -> tenant-root (mirrors vlinsert-generic), gated on monitoring-enabled. - backend enum in the schema; helm-unittest for central export + tenant stamp, central-mode local-backend/datasource skip, and the collector-scoped egress rule; regenerated schema/README/ApplicationDefinition. Read-side isolation on the shared backend (vmauth) is deferred future work per the design. Stacks on the phase-2 collector PR #3763. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 2 of distributed tracing (design cozystack/community#38, epic #3761): a per-tenant OpenTelemetry Collector as the app-facing OTLP ingest gateway and trust boundary. Plain Deployment + ConfigMap + Service (no OTel operator exists in the platform), deployed in the tenant namespace so app->collector traffic is intra-namespace and never crosses the tenant boundary. - receivers OTLP gRPC :4317 / HTTP :4318; memory_limiter + head sampling (probabilistic_sampler, default 10%) + optional attribute redaction (PII/secret control point) + batch; exports OTLP/HTTP to the tracingStorages backend (vtinsert:10481 for cluster, vtsingle:10428 for single mode) - app-facing Service 'otel-traces' (4317/4318); PSS-restricted-compliant pod and container securityContext; config-checksum annotation to roll on config change - tracingCollector values (enabled/samplingPercentage/redactAttributes/resources) in extra (annotated) + system (image pinned via cozy-lib.image); regenerated values.schema.json / README / ApplicationDefinition; 13 helm-unittest cases Deferred to later phases (epic #3761): AccountID/ProjectID header injection for the shared-central topology (phase 4); engine integrations (phase 3). This PR stacks on the phase-1 backend PR #3762. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 4 of distributed tracing (design cozystack/community#38, epic #3761): the opt-in shared-central topology where per-tenant collectors export to a single shared VictoriaTraces in tenant-root instead of a per-tenant backend. - collector: tracingCollector.backend=local|central. In central mode it exports over OTLP/HTTP to vtinsert-generic (ExternalName -> tenant-root) and a resource processor stamps tenant=<namespace>, so all tenants share account 0 while staying distinguishable. No local backend/datasource is rendered in central mode (the #3181 empty-list and multi-backend guards become local-only). - tenant NetworkPolicy: a narrow CiliumClusterwideNetworkPolicy scoped to the otel-traces collector pods only, allowing egress to the ancestor vtinsert (mirrors the vminsert egress block but endpoint-scoped, so no other tenant workload gains the cross-boundary hop). Non-root tenants only. - cozystack-basics: vtinsert-generic ExternalName in cozy-monitoring -> tenant-root (mirrors vlinsert-generic), gated on monitoring-enabled. - backend enum in the schema; helm-unittest for central export + tenant stamp, central-mode local-backend/datasource skip, and the collector-scoped egress rule; regenerated schema/README/ApplicationDefinition. Read-side isolation on the shared backend (vmauth) is deferred future work per the design. Stacks on the phase-2 collector PR #3763. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 2 of distributed tracing (design cozystack/community#38, epic #3761): a per-tenant OpenTelemetry Collector as the app-facing OTLP ingest gateway and trust boundary. Plain Deployment + ConfigMap + Service (no OTel operator exists in the platform), deployed in the tenant namespace so app->collector traffic is intra-namespace and never crosses the tenant boundary. - receivers OTLP gRPC :4317 / HTTP :4318; memory_limiter + head sampling (probabilistic_sampler, default 10%) + optional attribute redaction (PII/secret control point) + batch; exports OTLP/HTTP to the tracingStorages backend (vtinsert:10481 for cluster, vtsingle:10428 for single mode) - app-facing Service 'otel-traces' (4317/4318); PSS-restricted-compliant pod and container securityContext; config-checksum annotation to roll on config change - tracingCollector values (enabled/samplingPercentage/redactAttributes/resources) in extra (annotated) + system (image pinned via cozy-lib.image); regenerated values.schema.json / README / ApplicationDefinition; 13 helm-unittest cases Deferred to later phases (epic #3761): AccountID/ProjectID header injection for the shared-central topology (phase 4); engine integrations (phase 3). This PR stacks on the phase-1 backend PR #3762. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 4 of distributed tracing (design cozystack/community#38, epic #3761): the opt-in shared-central topology where per-tenant collectors export to a single shared VictoriaTraces in tenant-root instead of a per-tenant backend. - collector: tracingCollector.backend=local|central. In central mode it exports over OTLP/HTTP to vtinsert-generic (ExternalName -> tenant-root) and a resource processor stamps tenant=<namespace>, so all tenants share account 0 while staying distinguishable. No local backend/datasource is rendered in central mode (the #3181 empty-list and multi-backend guards become local-only). - tenant NetworkPolicy: a narrow CiliumClusterwideNetworkPolicy scoped to the otel-traces collector pods only, allowing egress to the ancestor vtinsert (mirrors the vminsert egress block but endpoint-scoped, so no other tenant workload gains the cross-boundary hop). Non-root tenants only. - cozystack-basics: vtinsert-generic ExternalName in cozy-monitoring -> tenant-root (mirrors vlinsert-generic), gated on monitoring-enabled. - backend enum in the schema; helm-unittest for central export + tenant stamp, central-mode local-backend/datasource skip, and the collector-scoped egress rule; regenerated schema/README/ApplicationDefinition. Read-side isolation on the shared backend (vmauth) is deferred future work per the design. Stacks on the phase-2 collector PR #3763. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 2 of distributed tracing (design cozystack/community#38, epic #3761): a per-tenant OpenTelemetry Collector as the app-facing OTLP ingest gateway and trust boundary. Plain Deployment + ConfigMap + Service (no OTel operator exists in the platform), deployed in the tenant namespace so app->collector traffic is intra-namespace and never crosses the tenant boundary. - receivers OTLP gRPC :4317 / HTTP :4318; memory_limiter + head sampling (probabilistic_sampler, default 10%) + optional attribute redaction (PII/secret control point) + batch; exports OTLP/HTTP to the tracingStorages backend (vtinsert:10481 for cluster, vtsingle:10428 for single mode) - app-facing Service 'otel-traces' (4317/4318); PSS-restricted-compliant pod and container securityContext; config-checksum annotation to roll on config change - tracingCollector values (enabled/samplingPercentage/redactAttributes/resources) in extra (annotated) + system (image pinned via cozy-lib.image); regenerated values.schema.json / README / ApplicationDefinition; 13 helm-unittest cases Deferred to later phases (epic #3761): AccountID/ProjectID header injection for the shared-central topology (phase 4); engine integrations (phase 3). This PR stacks on the phase-1 backend PR #3762. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 4 of distributed tracing (design cozystack/community#38, epic #3761): the opt-in shared-central topology where per-tenant collectors export to a single shared VictoriaTraces in tenant-root instead of a per-tenant backend. - collector: tracingCollector.backend=local|central. In central mode it exports over OTLP/HTTP to vtinsert-generic (ExternalName -> tenant-root) and a resource processor stamps tenant=<namespace>, so all tenants share account 0 while staying distinguishable. No local backend/datasource is rendered in central mode (the #3181 empty-list and multi-backend guards become local-only). - tenant NetworkPolicy: a narrow CiliumClusterwideNetworkPolicy scoped to the otel-traces collector pods only, allowing egress to the ancestor vtinsert (mirrors the vminsert egress block but endpoint-scoped, so no other tenant workload gains the cross-boundary hop). Non-root tenants only. - cozystack-basics: vtinsert-generic ExternalName in cozy-monitoring -> tenant-root (mirrors vlinsert-generic), gated on monitoring-enabled. - backend enum in the schema; helm-unittest for central export + tenant stamp, central-mode local-backend/datasource skip, and the collector-scoped egress rule; regenerated schema/README/ApplicationDefinition. Read-side isolation on the shared backend (vmauth) is deferred future work per the design. Stacks on the phase-2 collector PR #3763. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 4 of distributed tracing (design cozystack/community#38, epic #3761): the opt-in shared-central topology where per-tenant collectors export to a single shared VictoriaTraces in tenant-root instead of a per-tenant backend. - collector: tracingCollector.backend=local|central. In central mode it exports over OTLP/HTTP to vtinsert-generic (ExternalName -> tenant-root) and a resource processor stamps tenant=<namespace>, so all tenants share account 0 while staying distinguishable. No local backend/datasource is rendered in central mode (the #3181 empty-list and multi-backend guards become local-only). - tenant NetworkPolicy: a narrow CiliumClusterwideNetworkPolicy scoped to the otel-traces collector pods only, allowing egress to the ancestor vtinsert (mirrors the vminsert egress block but endpoint-scoped, so no other tenant workload gains the cross-boundary hop). Non-root tenants only. - cozystack-basics: vtinsert-generic ExternalName in cozy-monitoring -> tenant-root (mirrors vlinsert-generic), gated on monitoring-enabled. - backend enum in the schema; helm-unittest for central export + tenant stamp, central-mode local-backend/datasource skip, and the collector-scoped egress rule; regenerated schema/README/ApplicationDefinition. Read-side isolation on the shared backend (vmauth) is deferred future work per the design. Stacks on the phase-2 collector PR #3763. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 4 of distributed tracing (design cozystack/community#38, epic #3761): the opt-in shared-central topology where per-tenant collectors export to a single shared VictoriaTraces in tenant-root instead of a per-tenant backend. - collector: tracingCollector.backend=local|central. In central mode it exports over OTLP/HTTP to vtinsert-generic (ExternalName -> tenant-root) and a resource processor stamps tenant=<namespace>, so all tenants share account 0 while staying distinguishable. No local backend/datasource is rendered in central mode (the #3181 empty-list and multi-backend guards become local-only). - tenant NetworkPolicy: a narrow CiliumClusterwideNetworkPolicy scoped to the otel-traces collector pods only, allowing egress to the ancestor vtinsert (mirrors the vminsert egress block but endpoint-scoped, so no other tenant workload gains the cross-boundary hop). Non-root tenants only. - cozystack-basics: vtinsert-generic ExternalName in cozy-monitoring -> tenant-root (mirrors vlinsert-generic), gated on monitoring-enabled. - backend enum in the schema; helm-unittest for central export + tenant stamp, central-mode local-backend/datasource skip, and the collector-scoped egress rule; regenerated schema/README/ApplicationDefinition. Read-side isolation on the shared backend (vmauth) is deferred future work per the design. Stacks on the phase-2 collector PR #3763. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 4 of distributed tracing (design cozystack/community#38, epic #3761): the opt-in shared-central topology where per-tenant collectors export to a single shared VictoriaTraces in tenant-root instead of a per-tenant backend. - collector: tracingCollector.backend=local|central. In central mode it exports over OTLP/HTTP to vtinsert-generic (ExternalName -> tenant-root) and a resource processor stamps tenant=<namespace>, so all tenants share account 0 while staying distinguishable. No local backend/datasource is rendered in central mode (the #3181 empty-list and multi-backend guards become local-only). - tenant NetworkPolicy: a narrow CiliumClusterwideNetworkPolicy scoped to the otel-traces collector pods only, allowing egress to the ancestor vtinsert (mirrors the vminsert egress block but endpoint-scoped, so no other tenant workload gains the cross-boundary hop). Non-root tenants only. - cozystack-basics: vtinsert-generic ExternalName in cozy-monitoring -> tenant-root (mirrors vlinsert-generic), gated on monitoring-enabled. - backend enum in the schema; helm-unittest for central export + tenant stamp, central-mode local-backend/datasource skip, and the collector-scoped egress rule; regenerated schema/README/ApplicationDefinition. Read-side isolation on the shared backend (vmauth) is deferred future work per the design. Stacks on the phase-2 collector PR #3763. Signed-off-by: Alexey Artamonov <[email protected]>
Phase 4 of distributed tracing (design cozystack/community#38, epic #3761): the opt-in shared-central topology where per-tenant collectors export to a single shared VictoriaTraces in tenant-root instead of a per-tenant backend. - collector: tracingCollector.backend=local|central. In central mode it exports over OTLP/HTTP to vtinsert-generic (ExternalName -> tenant-root) and a resource processor stamps tenant=<namespace>, so all tenants share account 0 while staying distinguishable. No local backend/datasource is rendered in central mode (the #3181 empty-list and multi-backend guards become local-only). - tenant NetworkPolicy: a narrow CiliumClusterwideNetworkPolicy scoped to the otel-traces collector pods only, allowing egress to the ancestor vtinsert (mirrors the vminsert egress block but endpoint-scoped, so no other tenant workload gains the cross-boundary hop). Non-root tenants only. - cozystack-basics: vtinsert-generic ExternalName in cozy-monitoring -> tenant-root (mirrors vlinsert-generic), gated on monitoring-enabled. - backend enum in the schema; helm-unittest for central export + tenant stamp, central-mode local-backend/datasource skip, and the collector-scoped egress rule; regenerated schema/README/ApplicationDefinition. Read-side isolation on the shared backend (vmauth) is deferred future work per the design. Stacks on the phase-2 collector PR #3763. Signed-off-by: Alexey Artamonov <[email protected]>
… tracing Phase 1 of the distributed-tracing design proposal (cozystack/community#38): a per-tenant traces backend in the monitoring stack, mirroring the logs (VLCluster) implementation. No new operator — VTCluster/VTSingle already ship in victoria-metrics-operator. - tracingStorages values (extra + system), per-entry mode cluster|single - templates/vtraces/vtraces.yaml: VTCluster (default) / VTSingle (edge/dev), #3181-style empty-list render guard, retentionPeriod + optional disk cap (retentionMaxDiskSpaceUsageBytes), release-scoped PVC cleanup label - templates/vtraces/grafana-datasource.yaml: Jaeger-typed traces datasource (select service name/port/query path are confirm-before-implementation against the pinned VictoriaTraces version) - extra/monitoring readiness gate: VTCluster/VTSingle healthCheckExprs on status.updateStatus, alongside VLCluster - helm-unittest suites (required/empty-guard, cluster-vs-single, datasource, PVC label) + regenerated values.schema.json / README / ApplicationDefinition Deferred to follow-up PRs (tracked in the tracing epic): the per-tenant OTLP Collector, engine integrations, shared-central topology, and docs. Signed-off-by: Alexey Artamonov <[email protected]>
… tracing Phase 1 of the distributed-tracing design proposal (cozystack/community#38): a per-tenant traces backend in the monitoring stack, mirroring the logs (VLCluster) implementation. No new operator: VTCluster/VTSingle already ship in victoria-metrics-operator. - tracingStorages values (extra + system), per-entry mode cluster|single - templates/vtraces/vtraces.yaml: VTCluster (default) / VTSingle (edge/dev), #3181-style empty-list render guard, retentionPeriod + optional disk cap (retentionMaxDiskSpaceUsageBytes), release-scoped PVC cleanup label - templates/vtraces/grafana-datasource.yaml: Jaeger-typed traces datasource - extra/monitoring readiness gate: VTCluster/VTSingle healthCheckExprs on status.updateStatus, alongside VLCluster - helm-unittest suites (required/empty-guard, cluster-vs-single, datasource, PVC label) + regenerated values.schema.json / README / ApplicationDefinition The per-tenant OTLP Collector, engine integrations, shared-central topology and docs are out of scope and tracked in the tracing epic. Signed-off-by: Alexey Artamonov <[email protected]>
… tracing Phase 1 of the distributed-tracing design proposal (cozystack/community#38): a per-tenant traces backend in the monitoring stack, mirroring the logs (VLCluster) implementation. No new operator: VTCluster/VTSingle already ship in victoria-metrics-operator. - tracingStorages values (extra + system), per-entry mode cluster|single - templates/vtraces/vtraces.yaml: VTCluster (default) / VTSingle (edge/dev), #3181-style empty-list render guard, retentionPeriod + optional disk cap (retentionMaxDiskSpaceUsageBytes), release-scoped PVC cleanup label - templates/vtraces/grafana-datasource.yaml: Jaeger-typed traces datasource - extra/monitoring readiness gate: VTCluster/VTSingle healthCheckExprs on status.updateStatus, alongside VLCluster - helm-unittest suites (required/empty-guard, cluster-vs-single, datasource, PVC label) + regenerated values.schema.json / README / ApplicationDefinition The per-tenant OTLP Collector, engine integrations, shared-central topology and docs are out of scope and tracked in the tracing epic. Signed-off-by: Alexey Artamonov <[email protected]> Assisted-by: LLM
…g 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 -->
## What this PR does Phase 2 of distributed tracing (design cozystack/community#38, epic #3761), built on the phase-1 backend (#3762, merged): a per-tenant **OpenTelemetry Collector** — the app-facing OTLP ingest gateway. It lives in the tenant namespace (like a per-tenant vmagent), so app→collector traffic is intra-namespace and never crosses the tenant boundary. No OTel operator exists in the platform, so this is a plain Deployment + ConfigMap + Service (the fluent-bit pattern). ## Changes - **`templates/vtraces/collector.yaml`** — OTLP receivers (gRPC `:4317`, HTTP `:4318`) → `memory_limiter` + head sampling (`probabilistic_sampler`, default 10%) + optional `attributes/redact` (PII/secret control point) + `batch` → `otlphttp` exporter to the `tracingStorages` backend (`vtinsert:10481` cluster / `vtsingle:10428` single, endpoints per upstream docs). PSS-restricted securityContext, digest-pinned image via `cozy-lib.image`, config-checksum roll. - **app-facing Service `otel-traces`** (4317/4318). - **`tracingCollector` values** — `enabled` / `replicas` / `samplingPercentage` (schema-bounded 0-100) / `redactAttributes` / `resources`. A **fail-guard** rejects an enabled collector fronting more than one `tracingStorages` backend (it exports to a single backend by design). - **`otel-traces` WorkloadMonitor** and dashboard resource-map entry, rendered under the collector's own gate. The same resource-map Role also gets the pre-existing `alermanager` typo corrected to `alertmanager`, so the alertmanager WorkloadMonitor is readable from the dashboard. - helm-unittest coverage for the collector and its WorkloadMonitor; regenerated `values.schema.json` / `README.md` / `monitoring-rd` ApplicationDefinition. ## Verification - `helm unittest` — `system/monitoring` 110, `extra/monitoring` 15, all pass. - `helm template` renders cleanly; `make generate` idempotent. - Live check on a test cluster (single-mode backend): the collector starts with `redactAttributes` containing a numeric, a colon-bearing and an empty key; an OTLP/HTTP span sent to `otel-traces:4318` is accepted and is queryable from the backend through the Jaeger API, with the redacted attributes removed and the others kept. - Local `/branch-review` — **LGTM** (one blocker + four recommendations fixed, each with a test: multi-backend guard, memory_limiter cgroup limit, configurable replicas, sampling bounds, trust-boundary wording). ## Scope / deferred (epic #3761) `AccountID`/`ProjectID` header injection for the shared-central topology (phase 4) and engine integrations (phase 3) are follow-up PRs. ### Screenshots N/A, no UI change. ### Downstream repositories The diff touches only `packages/extra/monitoring`, `packages/system/monitoring` and `packages/system/monitoring-rd`. The website's monitoring reference page is regenerated from the package README by the release bot, and the Terraform provider models only the tenant's `monitoring` flag, not these values, so no follow-up is needed. - [x] 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): deploy a per-tenant OpenTelemetry (OTLP) collector for traces once a VictoriaTraces backend is configured. Applications send OTLP to the in-namespace `otel-traces` Service (gRPC `:4317`, HTTP `:4318`); the collector applies head sampling (`samplingPercentage`, default 10%) and optional attribute redaction (`redactAttributes`) before exporting to the backend. Off by default (no backend configured). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable trace storage in cluster or single-instance modes, with retention settings and Grafana datasources. * Added an enabled-by-default OpenTelemetry collector with configurable replicas, sampling, attribute redaction, and resource limits. * Added workload monitoring and automatic resource recommendations for configured trace components. * Added readiness health checks for both tracing backend modes. * **Bug Fixes** * Corrected dashboard permissions for alert monitoring and trace collector resources. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
4 participants
Summary
Adds a design proposal for distributed tracing in the Cozystack monitoring stack — the missing third observability signal alongside metrics (VictoriaMetrics) and logs (VictoriaLogs).
Revised 2026-07-29 following Timofei Larkin (@lllamnyp)'s review — reframed to lead with multitenancy, security, and vendor-neutrality, with premature Helm/implementation detail moved into a non-normative appendix.
Design highlights
tracing.enabledtoggle; the OTLP endpoint is platform-decided, never app-carried.Rendered:
design-proposals/distributed-tracing/README.mdStatus: Review — open for design review.
Summary by CodeRabbit