feat(otelcollector): render OTel Collector

[tianfeng92]

Summary

Render the OTel Collector as a StatefulSet in calico-system, configured via LogCollector.spec.otelCollector.

• Design doc
• Calico-private PR #12834

The collector receives logs from fluent-bit via OTLP and optionally federates Prometheus metrics, forwarding both to user-configured OTLP endpoints. It is added to the LogCollector CR rather than AdditionalStores because it is operator-managed infrastructure (StatefulSet, ConfigMap, RBAC, certs) with its own lifecycle, not a pointer to an external system.

Release Note

Add OpenTelemetry Collector component to export logs and metrics via OTLP.

Test plan

• Unit tests for render, controller, and fluent-bit pipeline OTel output
• Validated on MCM cluster — all three log types exporting, clean collector startup

EV-6862

[hjiawei]

How are these memory limits decided? Are they from the vendor recommendations?

[tianfeng92]

The memory_limiter processor follows the OTel Collector best practices: limit_mib should be ~80% of the container memory limit (409 ≈ 80% of 512Mi), and spike_limit_mib at ~25% of limit_mib (100 ≈ 25% of 409). This headroom lets the GC reclaim memory before the container hits OOMKill. See: https://github.com/open-telemetry/opentelemetry-collector/blob/main/processor/memorylimiterprocessor/README.md

The 512Mi container limit itself is a conservative starting point — the collector is mostly I/O bound (receiving and forwarding telemetry), so memory usage is dominated by in-flight batches rather than computation.

[pasanw]

We shouldn't mark the CR as not found in this case. We should enforce it at the CRD level (via kubebuilder annotations) that the OTelCollector field must be set. We can have a defensive check here if you want, but it should SetDegraded with a message rather that OnCRNotFound

[pasanw]

Move this beneath the block where we find the CR

[pasanw]

We need to update the annotations of the pod to use HashAnnotations() and HashAnnotationKey(). We should also add a watch of tigera-ca-private into the controller to ensure we reconcile when the CA rotates.

[pasanw]

What is our deletion strategy to ensure that if otel is disabled, we don't keep the otel collector and all of its resources around?

[pasanw]

From what I understand, this message is not true? Forwarding of metrics continues since we do not tear down the deployment.

What is our stance here? It seems like it's that metric forwarding does not require a license. But if the license is missing or doesn't have the feature, we don't deploy the collector at all which conflicts with that stance

[pasanw]

Claude: This renders the wrong OTel setting. The tlsInsecure field is documented as "disables TLS verification" (keep TLS on, just don't check the cert), but tls.insecure: true means no TLS at all — plaintext. The setting for "skip verification" is insecure_skip_verify, which we never render.

So for a self-signed target like the field doc suggests:

• http:// or a scheme-less host:port → plaintext; telemetry (incl. flow data) goes out unencrypted.
• https:// grpc → the scheme forces TLS and overrides insecure, so the self-signed cert fails verification and the connection breaks; tlsInsecure did nothing.

Either way the field can't do what it promises. If plaintext is actually what we want, that's a separate thing (or just an http:// scheme).

[pasanw]

What is our overall TLS strategy here? Do we need to support a destination that requires mTLS? Do we need to support a user having a custom CA? There is precedent for this in other components (e.g. syslog forwarding, guardian connection)

[pasanw]

Claude: A few kubebuilder-marker gaps on the new API surface, grouping them here:

• This list needs list markers. exporters[] is our first named object list, but it has no +listType=map / +listMapKey=name, and nothing enforces name uniqueness. So duplicate names silently collide (same exporter key), and the list merges as atomic-replace under server-side apply. Add both markers and make name required + unique.
• Required fields aren't marked. Name and Endpoint (otelcollector_types.go:67,70) have no +optional, so they're effectively required — but the doc wants an explicit // +required.
• Defaults live in prose. metrics.enabled (otelcollector_types.go:52) and tlsInsecure (:80) state their defaults in the comment but have no +kubebuilder:default, unlike protocol (:74) which does. Add the marker (or drop the prose claim so they don't drift).

[pasanw]

Claude: These two probes are identical and neither sets InitialDelaySeconds, so there's no boot grace — the liveness check starts immediately and could restart the collector while it's still starting up.

The convention in this repo is to give boot grace with InitialDelaySeconds (we don't use startup probes — they show up once in the whole render tree, InitialDelaySeconds ~20 times), and to tune liveness longer than readiness. Guardian/dex/manager/apiserver set liveness to 90 and readiness to 10.

Suggest matching that: give healthProbe a delay param and wire LivenessProbe: healthProbe(90) / ReadinessProbe: healthProbe(10). The 90 on liveness is the boot grace; the 10 on readiness lets it start taking traffic quickly. Keeps the same :13133 endpoint, just splits the timing the way the other components do.

[pasanw]

Do we have a requirement for supporting HA? Right now there are many gaps to make this support HA in a comprehensive way: no anti-affinity, no pod disruption budgets, no explicit strategy for dealing with multiple replicas causing metric duplication.

The latter needs some research (Claude mentions target allocator could fix the metrics side, and that StatefulSets should use a headless service for PVCs which could eventually break the logs side).

If we don't have this explicit requirement I would recommend moving it out of scope