Add design proposal: Distributed tracing via OTLP and VictoriaTraces#38
Add design proposal: Distributed tracing via OTLP and VictoriaTraces#38scooby87 wants to merge 3 commits into
Conversation
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
📝 WalkthroughWalkthroughChangesDistributed 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 <aleksei.artamonov@aenix.io>
![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
| Render one `VTCluster` per entry in a new `templates/vtraces/vtraces.yaml`, lifted from `templates/vlogs/vlogs.yaml`: `managedMetadata` labels for application ownership, replica counts on `vtinsert`/`vtselect`/`vtstorage`, `vtstorage.retentionPeriod` from the entry, the storage-PVC claim-template label for the release-scoped post-delete cleanup hook, and — reusing the `cozystack/cozystack#3181` lesson — a guard that **fails the render on an empty list** so a misconfiguration is loud, not a silent black hole for spans. | ||
|
|
||
| ```yaml | ||
| {{- range .Values.tracingStorages }} | ||
| --- | ||
| apiVersion: operator.victoriametrics.com/v1 | ||
| kind: VTCluster | ||
| metadata: | ||
| name: {{ .name }} | ||
| spec: | ||
| managedMetadata: | ||
| labels: | ||
| apps.cozystack.io/application.kind: Monitoring | ||
| apps.cozystack.io/application.name: {{ $.Release.Name }} | ||
| vtinsert: { replicaCount: 2 } | ||
| vtselect: { replicaCount: 2 } | ||
| vtstorage: | ||
| retentionPeriod: {{ .retentionPeriod | quote }} | ||
| replicaCount: 2 | ||
| storage: | ||
| volumeClaimTemplate: | ||
| metadata: | ||
| labels: | ||
| apps.cozystack.io/application.name: {{ $.Release.Name }} | ||
| spec: | ||
| {{- with .storageClassName }} | ||
| storageClassName: {{ . }} | ||
| {{- end }} | ||
| resources: | ||
| requests: | ||
| storage: {{ .storage }} | ||
| {{- end }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the empty-storage behavior internally consistent and implement the guard.
The shown template has no required/fail check, so an empty tracingStorages list silently renders zero resources. That contradicts the stated loud failure behavior and the testing requirement, while Line 174 says an absent list should render successfully. Decide whether absence disables tracing or is invalid, then encode that distinction in the template and tests.
Also applies to: 174-174, 185-193
🤖 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 - 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.
| **Stage A (MVP) — direct to `vtinsert`.** Applications push OTLP straight to the VictoriaTraces insert service. Add an `otel-traces` (or `vtinsert-generic`) **ExternalName redirect** to `packages/system/cozystack-basics/templates/monitoring-external-services.yaml`, alongside the existing `vlinsert-generic`/`vminsert-*` entries, pointing `*.cozy-monitoring.svc` → `*.tenant-root.svc.cluster.local`, gated on the same `_cluster.monitoring-enabled` flag (from `monitoring.rootEnabled`). This is the exact mechanism logs and metrics already use; central-vs-isolated tenants work identically with zero new machinery. | ||
|
|
||
| **Stage B — toggleable OpenTelemetry Collector gateway.** Add an OpenTelemetry Collector (gateway **Deployment** in `cozy-monitoring`, following the `packages/system/monitoring-agents` pattern) in front of `vtinsert` for the capabilities direct ingest can't provide: central sampling, rate-limiting, and per-`tenant` resource attribution. It is the "OpenTelemetry Collector/Agent via an option" the client explicitly requested, and it is opt-in. | ||
|
|
||
| - Receivers: `otlp` on gRPC `4317` and HTTP `4318`. | ||
| - Processors: `batch`, a `tail_sampling`/`probabilistic_sampler` governed by the platform default, and `resource` to stamp `tenant`. | ||
| - Exporter: OTLP to the `vtinsert` service of the `tracingStorages` backend. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant proposal file around the cited ranges.
file='design-proposals/distributed-tracing/README.md'
wc -l "$file"
sed -n '100,130p' "$file"
printf '\n---\n'
sed -n '168,190p' "$file"
# Locate any references to VictoriaTraces tenant headers or OTLP gateway details in the repo.
printf '\n=== matches ===\n'
rg -n "AccountID|ProjectID|tenant resource|resource attribute|tail_sampling|probabilistic_sampler|ExternalName redirect|vtinsert|otel-traces|OpenTelemetry Collector" design-proposals packages -SRepository: cozystack/community
Length of output: 13517
🌐 Web query:
VictoriaTraces OTLP tenant AccountID ProjectID headers resource attribute OpenTelemetry Collector
💡 Result:
In VictoriaMetrics (including VictoriaTraces and VictoriaLogs), you can specify tenant information for OTLP data ingestion using HTTP headers, which eliminates the need to hardcode tenant IDs in the URL path [1][2][3]. When using an OpenTelemetry Collector, you can configure these headers within the exporter settings (e.g., otlphttp) [4][5]. Tenant Identification via Headers: The following HTTP headers are used to define the tenant: - AccountID: Specifies the account ID of the tenant [1][3]. - ProjectID: Specifies the project ID of the tenant [1][3]. If these headers are omitted, VictoriaMetrics typically defaults to the 0:0 tenant ID [2]. Note that for multi-tenant setups, these IDs are generally expected to be unsigned 32-bit integers [6]. Configuration Example: To pass these headers via an OpenTelemetry Collector using the otlphttp exporter, add them to the headers section of your configuration [4][5]: exporters: otlphttp: traces_endpoint: http://:10428/insert/opentelemetry/v1/traces headers: AccountID: "123" ProjectID: "456" Additional Notes: - Resource Attributes: VictoriaTraces and VictoriaLogs automatically map certain OTLP resource attributes (such as service.name) as stream or internal fields [4][5]. Other resource attributes are stored as regular fields or tags within the ingested data [4]. - gRPC Support: Since gRPC operates over HTTP/2, it also supports passing these headers for multitenancy [1][3]. - Alternative Methods: While headers are a common way to handle multitenancy, you can also include tenant IDs directly in the ingestion URL path (e.g., /insert/<AccountID>:<ProjectID>/opentelemetry/...) [7][8]. Using headers is often preferred for maintaining consistent configurations across different environments [2].
Citations:
- 1: https://new.docs.victoriametrics.com/victoriatraces/data-ingestion/
- 2: lib/httpserver: support multitnenacy via headers VictoriaMetrics/VictoriaMetrics#10782
- 3: https://docs.victoriametrics.com/victoriatraces/data-ingestion/
- 4: https://new.docs.victoriametrics.com/victoriatraces/data-ingestion/opentelemetry/
- 5: https://github.com/VictoriaMetrics/VictoriaLogs/blob/master/docs/victorialogs/data-ingestion/opentelemetry.md
- 6: Multitenancy Headers passed for VictoriaLogs digits work but alphabet and special characters fail when passed via otlp http exporter VictoriaMetrics/helm-charts#2257
- 7: https://new.docs.victoriametrics.com/victoriametrics/data-ingestion/opentelemetry-collector/
- 8: support opentelemetry collector? VictoriaMetrics/VictoriaMetrics#5142
🌐 Web query:
site:docs.victoriametrics.com victoriaTraces OTLP AccountID ProjectID headers
💡 Result:
In VictoriaTraces, you can specify the AccountID and ProjectID to identify a tenant during OTLP data ingestion using HTTP request headers [1][2]. When using OTLP/HTTP or OTLP/gRPC (which runs over HTTP/2), you should include the following headers in your requests: - AccountID: The account ID of the tenant [3][4]. - ProjectID: The project ID of the tenant [3][4]. If these headers are not provided, VictoriaTraces defaults both values to 0 [2]. Configuration Example: If you are using an OpenTelemetry Collector to export traces to VictoriaTraces, you can configure these headers within your exporter configuration [5]. For example, in an OTLP/HTTP exporter: exporters: otlphttp: traces_endpoint: http://:10428/insert/opentelemetry/v1/traces headers: AccountID: "123" ProjectID: "456" Note that VictoriaTraces also supports passing these parameters via HTTP query string parameters, which take priority over the HTTP headers if both are provided [4].
Citations:
- 1: https://docs.victoriametrics.com/victoriatraces/
- 2: https://docs.victoriametrics.com/victoriatraces/index.html
- 3: https://docs.victoriametrics.com/victorialogs/data-ingestion/
- 4: https://new.docs.victoriametrics.com/victoriatraces/data-ingestion/
- 5: https://docs.victoriametrics.com/victoriatraces/data-ingestion/opentelemetry/
Stage A must set VictoriaTraces tenant headers.
VictoriaTraces routes OTLP by AccountID/ProjectID headers; an ExternalName redirect and a Collector resource processor do not set them. Direct OTLP from apps will hit the default tenant unless the app or a trusted proxy/exporter injects those headers from authenticated workload identity, so Stage B also needs explicit header mapping rather than only stamping tenant as a resource attribute.
🤖 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 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.
Source: MCP tools
| **Stage A (MVP) — direct to `vtinsert`.** Applications push OTLP straight to the VictoriaTraces insert service. Add an `otel-traces` (or `vtinsert-generic`) **ExternalName redirect** to `packages/system/cozystack-basics/templates/monitoring-external-services.yaml`, alongside the existing `vlinsert-generic`/`vminsert-*` entries, pointing `*.cozy-monitoring.svc` → `*.tenant-root.svc.cluster.local`, gated on the same `_cluster.monitoring-enabled` flag (from `monitoring.rootEnabled`). This is the exact mechanism logs and metrics already use; central-vs-isolated tenants work identically with zero new machinery. | ||
|
|
||
| **Stage B — toggleable OpenTelemetry Collector gateway.** Add an OpenTelemetry Collector (gateway **Deployment** in `cozy-monitoring`, following the `packages/system/monitoring-agents` pattern) in front of `vtinsert` for the capabilities direct ingest can't provide: central sampling, rate-limiting, and per-`tenant` resource attribution. It is the "OpenTelemetry Collector/Agent via an option" the client explicitly requested, and it is opt-in. | ||
|
|
||
| - Receivers: `otlp` on gRPC `4317` and HTTP `4318`. | ||
| - Processors: `batch`, a `tail_sampling`/`probabilistic_sampler` governed by the platform default, and `resource` to stamp `tenant`. | ||
| - Exporter: OTLP to the `vtinsert` service of the `tracingStorages` backend. | ||
|
|
||
| Applications always target a stable in-cluster endpoint (e.g. `…cozy-monitoring.svc:4317`); flipping from Stage A to Stage B only re-points that ExternalName from `vtinsert` to the collector, so the migration is transparent to every traced app. A gateway **Deployment** (not a DaemonSet) is correct here because OTLP is push-based over the network — apps are the agents; the collector plays the centralized `vtinsert`/vmagent role, not the node-local fluent-bit role. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant proposal section
sed -n '100,140p' design-proposals/distributed-tracing/README.md
# Find any in-repo references to vtinsert, otlp, and the mentioned services
rg -n "vtinsert|otlp|4317|4318|10481|opentelemetry/v1/traces|ExternalName|monitoring-external-services" design-proposals packages -g '!**/node_modules/**'Repository: cozystack/community
Length of output: 11456
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the monitoring external service template and VTCluster/service definitions
git ls-files | rg 'monitoring-external-services\.yaml|vtraces|vtcluster|vtinsert|monitoring-agents|monitoring.*yaml$|cozystack-basics'
# Show the relevant template(s) if present
for f in \
$(git ls-files | rg 'monitoring-external-services\.yaml|vtcluster|vtinsert|vtraces'; true)
do
echo "### $f"
wc -l "$f" || true
done
# Search for concrete OTLP listener/service port declarations and ExternalName usage
rg -n "4317|4318|10481|ExternalName|vtinsert-generic|otel-traces|opentelemetry/v1/traces|listener|service:" . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: cozystack/community
Length of output: 157
🌐 Web query:
VictoriaMetrics VictoriaTraces vtinsert OTLP port 10481 opentelemetry/v1/traces ExternalName path gRPC 4317 HTTP 4318
💡 Result:
In VictoriaTraces, the vtinsert component is responsible for receiving trace spans via the OpenTelemetry protocol (OTLP) [1][2]. OTLP/HTTP Ingestion The vtinsert component typically listens for OTLP/HTTP ingestion requests on port 10481 by default [1][3][2]. The data should be sent to the following path: /insert/opentelemetry/v1/traces [1][4]. OTLP/gRPC Ingestion While port 10481 is the default for general HTTP ingestion [2], OTLP/gRPC ingestion is enabled by explicitly configuring the -otlpGRPCListenAddr flag on the vtinsert component (or the corresponding configuration in the VictoriaMetrics Operator) [5][6]. By convention, OTLP/gRPC commonly utilizes port 4317 [6], and OTLP/HTTP often uses port 4318 when configured as a standard OTLP endpoint [6]. However, for VictoriaTraces specifically, ensure the endpoint configuration matches the -otlpGRPCListenAddr value provided to the service [6]. Summary Table Component: vtinsert Function: OTLP Trace Ingestion [1][2] Default HTTP Path: /insert/opentelemetry/v1/traces [1][4] Default HTTP Port: 10481 [1][2] gRPC Port: Configurable (commonly 4317) [6]
Citations:
- 1: https://docs.victoriametrics.com/operator/resources/vtcluster/
- 2: https://new.docs.victoriametrics.com/victoriatraces/cluster/
- 3: https://docs.victoriametrics.com/victoriatraces/cluster/
- 4: https://docs.victoriametrics.com/victoriatraces/quick-start/
- 5: https://github.com/VictoriaMetrics/helm-charts/blob/3e7afa93/charts/victoria-traces-cluster/templates/vtinsert-server.yaml
- 6: https://new.docs.victoriametrics.com/victoriatraces/data-ingestion/opentelemetry/
Define the Stage A OTLP service contract.
ExternalName only aliases DNS; it does not make vtinsert speak 4317/4318 or translate the OTLP/HTTP path. vtinsert defaults to 10481 and /insert/opentelemetry/v1/traces, so this needs an explicit Service/listener mapping (or a path-aware proxy) before the direct-ingest path will work as written.
🤖 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 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.
Source: MCP tools
| **Stage B — toggleable OpenTelemetry Collector gateway.** Add an OpenTelemetry Collector (gateway **Deployment** in `cozy-monitoring`, following the `packages/system/monitoring-agents` pattern) in front of `vtinsert` for the capabilities direct ingest can't provide: central sampling, rate-limiting, and per-`tenant` resource attribution. It is the "OpenTelemetry Collector/Agent via an option" the client explicitly requested, and it is opt-in. | ||
|
|
||
| - Receivers: `otlp` on gRPC `4317` and HTTP `4318`. | ||
| - Processors: `batch`, a `tail_sampling`/`probabilistic_sampler` governed by the platform default, and `resource` to stamp `tenant`. | ||
| - Exporter: OTLP to the `vtinsert` service of the `tracingStorages` backend. | ||
|
|
||
| Applications always target a stable in-cluster endpoint (e.g. `…cozy-monitoring.svc:4317`); flipping from Stage A to Stage B only re-points that ExternalName from `vtinsert` to the collector, so the migration is transparent to every traced app. A gateway **Deployment** (not a DaemonSet) is correct here because OTLP is push-based over the network — apps are the agents; the collector plays the centralized `vtinsert`/vmagent role, not the node-local fluent-bit role. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced doc section and any adjacent design context.
git ls-files design-proposals/distributed-tracing/README.md
wc -l design-proposals/distributed-tracing/README.md
sed -n '90,150p' design-proposals/distributed-tracing/README.mdRepository: cozystack/community
Length of output: 5387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any discussion of tail sampling / collector deployment / load balancing in the repo.
rg -n --hidden --glob '!**/.git/**' -S \
'tail_sampling|tailsampling|trace-aware|load[- ]balanc|Collector|4317|4318|ExternalName|vtinsert|cozy-monitoring' \
design-proposals distributed-tracing packages .github README.mdRepository: cozystack/community
Length of output: 7739
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the upstream collector docs for tail-sampling deployment constraints.
python3 - <<'PY'
import urllib.request, ssl
urls = [
"https://raw.githubusercontent.com/open-telemetry/opentelemetry-collector-contrib/main/processor/tailsamplingprocessor/README.md",
"https://raw.githubusercontent.com/open-telemetry/opentelemetry-collector-contrib/main/processor/tailsamplingprocessor/README.md?plain=1",
]
ctx = ssl.create_default_context()
for u in urls:
try:
with urllib.request.urlopen(u, context=ctx, timeout=20) as r:
data = r.read().decode('utf-8', errors='replace')
print(f"URL: {u}\n")
for needle in ["load balance", "load-balanced", "trace-id", "trace ID", "single instance", "multiple collectors", "sampling decision"]:
if needle.lower() in data.lower():
print(f"FOUND: {needle}")
# print focused lines around matching areas
lines = data.splitlines()
for i, line in enumerate(lines):
if any(k in line.lower() for k in ["load balance", "trace-id", "trace id", "sampling decision"]):
start = max(0, i-3); end = min(len(lines), i+4)
print("---")
for j in range(start, end):
print(f"{j+1}: {lines[j]}")
print("\n====\n")
except Exception as e:
print(f"ERROR for {u}: {e}")
PYRepository: cozystack/community
Length of output: 711
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '110,130p' design-proposals/distributed-tracing/README.mdRepository: cozystack/community
Length of output: 3155
🌐 Web query:
OpenTelemetry Collector tail sampling multiple replicas spans same trace must reach one instance load balancing consistent hashing official docs
💡 Result:
To ensure that all spans for the same trace reach the same OpenTelemetry Collector instance for accurate tail-based sampling, you must implement a two-tiered Collector architecture [1][2]. Standard load balancers (such as Kubernetes Services) perform round-robin distribution, which splits spans of a single trace across multiple instances, rendering tail sampling ineffective [3][4]. The recommended solution is to use the loadbalancingexporter in the first tier [1][2]. This exporter uses consistent hashing to ensure that all spans with the same trace ID are deterministically routed to the same downstream (second-tier) Collector instance [5][3][4]. Key implementation details: 1. Two-Tiered Architecture: - Tier 1: Configured with the loadbalancingexporter. This tier receives spans from your application and forwards them to the second tier based on consistent hashing [1][3]. - Tier 2: Configured with the tail sampling processor. Because all spans of a trace are guaranteed to reach the same instance in this tier, the processor can accurately evaluate sampling policies (e.g., waiting for all spans to arrive before deciding whether to keep or drop the trace) [1][2][4]. 2. Configuration: - In the loadbalancingexporter (Tier 1), set the routing_key to traceID [1][6]. This ensures consistent routing for traces [5][7]. - Ensure the downstream (Tier 2) instances are properly defined in the loadbalancingexporter's configuration (e.g., using a static list of endpoints or a DNS resolver) [5][3]. By using this approach, you maintain the state required for tail sampling while allowing the Collector pipeline to scale horizontally [2][4].
Citations:
- 1: https://opentelemetry.io/docs/collector/deploy/gateway/
- 2: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/tailsamplingprocessor/README.md
- 3: https://devcheolu.com/en/posts/mqAJZO1YBO614afgoD6A
- 4: https://devcheolu.com/en/posts/uIHEKhBvZKlt2W8cp95G
- 5: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/loadbalancingexporter/README.md
- 6: https://www.elastic.co/observability-labs/blog/opentelemetry-collector-reference-architectures
- 7: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter/loadbalancingexporter
Keep tail-sampling trace-affine.
A regular Kubernetes Service can split spans from the same trace across Collector replicas, so tail-sampling will make decisions on incomplete traces. Keep the sampling tier at one replica, or add trace-ID-aware routing/load balancing in front of it.
🤖 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 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.
Source: MCP tools
| - OTLP endpoint unreachable from an app (backend or collector down) → the app's exporter drops/retries per OTLP defaults; the app itself does not crash and serving is unaffected. | ||
| - Backend storage exhausted → oldest traces evicted per `retentionPeriod`; ingest backpressures at the receiver (`vtinsert` in Stage A, the collector in Stage B), not the app. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File excerpt =="
sed -n '160,210p' design-proposals/distributed-tracing/README.md | cat -n
echo
echo "== Search for VictoriaTraces disk/retention mentions in repo =="
rg -n "retentionPeriod|free-space|disk-usage|disk usage|VictoriaTraces|vtinsert|retention" design-proposals -S || trueRepository: cozystack/community
Length of output: 17495
🌐 Web query:
VictoriaTraces retentionPeriod free-space threshold disk-usage retention flags docs
💡 Result:
VictoriaTraces manages data retention through time-based and disk-based mechanisms, which are applied independently [1][2]. Time-based Retention The -retentionPeriod flag defines the duration for which data is kept [3][1]. Data older than this period is automatically deleted [1][2]. - Default: 7 days [1][2]. - Supported range: 1 day (1d) to 100 years (100y) [3][1]. - Syntax: Supports suffixes s (seconds), h (hours), d (days), w (weeks), M (months), and y (years) [1][2]. If no suffix is provided, the value is treated as months [4][5]. Disk-based Retention You can also limit data retention based on disk space usage [4][6]. - Flags: - -retention.maxDiskSpaceUsageBytes: Sets a maximum size in bytes [1][2]. - -retention.maxDiskUsagePercent: Sets a maximum percentage of disk usage (values 1-100) [2]. - Behavior: These flags are mutually exclusive [2]. - Note: Time-based retention (-retentionPeriod) and disk space limits are applied independently [1][2]. If you only set a disk space limit, VictoriaTraces will still drop data older than the default 7-day retention period unless -retentionPeriod is set to a sufficiently large value (e.g., 100y) [1][2]. Helm Configuration When using VictoriaTraces Helm charts, these settings are typically configured via: - server.retentionPeriod or vtstorage.retentionPeriod [4][5] - server.retentionDiskSpaceUsage or vtstorage.retentionDiskSpaceUsage [4][5]
Citations:
- 1: https://docs.victoriametrics.com/victoriatraces/index.html
- 2: https://new.docs.victoriametrics.com/victoriatraces/
- 3: https://docs.victoriametrics.com/victoriatraces/
- 4: https://docs.victoriametrics.com/helm/victoria-traces-single/
- 5: https://docs.victoriametrics.com/helm/victoriatraces-cluster/
- 6: https://new.docs.victoriametrics.com/helm/victoria-traces-single/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import sys, json, urllib.request, urllib.parse
queries = [
"VictoriaTraces retentionPeriod free-space threshold disk-usage retention flags",
"site:docs.victoriametrics.com VictoriaTraces disk usage retention free-space threshold",
]
for q in queries:
print(f"\n## QUERY: {q}")
url = "https://html.duckduckgo.com/html/?" + urllib.parse.urlencode({"q": q})
req = urllib.request.Request(url, headers={"User-Agent": "Mozilla/5.0"})
try:
with urllib.request.urlopen(req, timeout=20) as resp:
data = resp.read().decode("utf-8", "replace")
# Print only a small snippet with likely result titles/urls
import re
for m in re.finditer(r'<a rel="nofollow" class="result__a" href="([^"]+)">(.+?)</a>', data):
href, title = m.group(1), re.sub('<.*?>', '', m.group(2))
print(title)
print(href)
print("---")
break
else:
print("No parsed result.")
except Exception as e:
print("ERR", e)
PYRepository: cozystack/community
Length of output: 621
Account for PVC exhaustion explicitly. retentionPeriod only covers age-based pruning; bounded PVC usage needs the separate VictoriaTraces disk-retention settings (vtstorage.retentionDiskSpaceUsage / -retention.maxDiskSpaceUsageBytes or -retention.maxDiskUsagePercent). Update the failure mode to reflect that full disks can still block ingest before old traces are evicted, and add capacity alerts plus a PVC-exhaustion test.
🤖 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 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.
Source: MCP tools
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 <aleksei.artamonov@aenix.io>
|
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 |
2 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).
The proposal centers on VictoriaTraces (its
VTCluster/VTSingleCRDs already ship in the victoria-metrics-operator Cozystack deploys), so no new operator is introduced. It mirrors the existing VictoriaLogs deployment, datasource-provisioning, and values conventions one-for-one.Design highlights
cozy-monitoring, following themonitoring-agentspattern.tracingStoragesvalues list →VTCluster, lifted from theVLClustertemplate; retention configurable, default 14d.tracing.enabledtoggle (off by default), modelled on foundationdb'smonitoring.enabled; native OTLP where the engine supports it (ClickHouse, NATS), sidecar/agent otherwise.Grafana Tempo is documented as the drop-in fallback if VictoriaTraces proves immature.
Rendered:
design-proposals/distributed-tracing/README.mdStatus: Draft — opening for early feedback.
Summary by CodeRabbit