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]

Thanks for addressing my comments! I let Claude lead the re-review and reviewed everything it said was addressed along with what it flagged - it helped flag some things that weren't fully addressed but it also flagged some new issues (sorry about that!)

I'll let you and @rene-dekker work through them together and decide the best way to proceed.

[pasanw]

Claude: Glad this got pulled into a shared helper. Right now though only the OTel component uses it — Dex (dex.go, parseHostPortFromURL + resolveEgressRuleForDestination) and Guardian (inline in guardianCalicoSystemPolicy) still have their own copies of the same URL→host:port→egress-rule logic. The ask was one shared piece across all three, so it'd be good to move those two onto ParseExternalDestination too — otherwise the duplication's only half gone.

[tianfeng92]

Fair — the duplication is only half gone. I have dex and guardian converted onto ParseExternalDestination locally, but both have a much wider blast radius than otel and I'd rather not fold that into this PR's test surface. Splitting it into a follow-up so it can be reviewed on its own; this PR keeps the helper plus the one caller.

[tianfeng92]

The shared piece exists and OTel is on it:

https://github.com/tigera/operator/blob/ba5aeb9d4/pkg/render/common/networkpolicy/networkpolicy.go#L346-L410

ParseExternalDestination handles bare host:port and URLs with scheme defaults; ExternalDestinationEntityRule picks the tightest rule available — a /32 or /128 for a literal IP, a Services match for an in-cluster <svc>.<ns>.svc name, a Domains rule for a hostname when the license carries egress-access-control, and port-only otherwise.

Moving Dex and Guardian across is going into its own PR rather than this one. Their conversion is already written (26 lines replacing 83), but they sit on the authentication and management-cluster tunnel paths, where a regression costs more than it does on log export, and each has behaviour the OTel path doesn't — Dex's 0.0.0.0/0 any-destination case, Guardian's HTTP proxy handling. Folding that in here would also widen an already large diff.

Tian is picking that up as a follow-up, so the duplication does get finished off, just not on this PR.

[pasanw]

Claude: This map hashes the config, the trusted bundle, and the receiver keypair (this line) — but not the exporter-side certs: the user-supplied ExporterCA (otel-collector-ca) and the client keypair (otel-collector-client-certs). And the exporter TLS block has no reload_interval. So it's the same rotation bug we just fixed on the receiver, reopened on the export side: if a user rotates the exporter CA or client cert, the controller re-copies the new PEM but the pod template is byte-identical, so the StatefulSet doesn't roll. otelcol only reads these files at startup, so it keeps presenting/trusting the stale cert and mTLS to the customer backend breaks until an unrelated restart. Can we fold the ExporterCA + client keypair hashes into this map too, same as the receiver keypair?

[tianfeng92]

Good catch, fixed. Both are folded into the same map now:
https://github.com/tigera/operator/blob/322d8b439/pkg/render/otelcollector/component.go#L591-L597

On reload_interval for the exporter block: I left it off deliberately. Unlike the receiver's client_ca_file_reload, these are user-managed Secrets/ConfigMaps with no rotation schedule we control, so the hash annotation is the mechanism rather than the backstop — a rotation changes the pod template and rolls the StatefulSet. Happy to add the poll too if you'd rather have both.

[pasanw]

Claude: This gate keeps the ServiceMonitor on OpenTelemetryEnabled && !LicenseExpired, but the collector is also torn down when the license just lacks the otel-collector feature (valid, not expired) — that case isn't mirrored here, so the ServiceMonitor sticks around selecting a Service that's gone, which is the dangling-monitor situation this gating was meant to avoid. Related: in monitor_controller.go:286, a transient LogCollector read error leaves openTelemetryEnabled=false, which drops into the delete branch and recreates on the next reconcile — so a brief apiserver blip flaps the monitor. Both come down to "enabled" needing to track the same conditions the otel controller uses to actually deploy the collector.

[tianfeng92]

Both fixed, and chasing the second half turned up a third bug.

Feature gate mirrored, so "enabled" means licensed, not just configured:
https://github.com/tigera/operator/blob/322d8b439/pkg/controller/monitor/monitor_controller.go#L282-L295

The transient read now requeues instead of reading as "disabled":
https://github.com/tigera/operator/blob/322d8b439/pkg/controller/monitor/monitor_controller.go#L286-L292

The third one: the ServiceMonitor was rendering a target Prometheus was never allowed to reach. Nothing opened egress to the collector's metrics port, so the calico-system default-deny dropped it and the scrape just reported up=0 — no error naming the cause. Found it on a cluster; the ServiceMonitor looked correct and no otelcol_* metrics ever arrived. Now paired under the same gate:
https://github.com/tigera/operator/blob/322d8b439/pkg/render/monitor/monitor.go#L1509-L1517

Verified on a cluster after the fix: up{job="otel-collector"} went 0 -> 1 and the collector's own queue/throughput metrics started landing. There's a test asserting the ServiceMonitor and the egress rule appear and disappear together.

[pasanw]

Claude: Coming back to this after digging into the TLS side properly. The custom-CA handling is fine and matches the syslog convention, and the rendered config is correct/secure at the OTel level (verified against v0.157.0 — verification can't be turned off, and https verifies on both transports). But the mTLS client cert has a real limitation worth raising.

Right now there's one shared client cert (otel-collector-client-certs) for every exporter that turns mutualTLS on. So every backend we send to is shown the same client cert. That's fine for one backend, or if they all accept the same one — but this feature fans out to multiple external backends (Datadog, a corporate collector, etc.), and different backends usually want different client certs. With one shared cert you can't do that: one identity for all, take it or leave it.

tlsroute already solves this the flexible way — each destination points at its own cert (per-target SecretKeySelector). So this is the one spot where we diverge from the closest existing pattern.

The fix is small if we want it: leave the CA as-is (a shared bundle already trusts many backends, so nothing to change there), and swap the mutualTLS boolean for a per-exporter client-cert Secret name — "here's the Secret with this exporter's cert." The render already loops over the exporters one at a time and validates each, so it's extending that loop rather than new structure; the only new bit is mounting each exporter's Secret instead of one shared one. Bonus: it also lands us on the tlsroute convention.

Also (minor, same field): mutualTLS: true on an http:// endpoint is silently ignored — the cert block gets skipped because the endpoint is plaintext, with no warning, and you're still forced to supply the Secret. Worth at least a validation error so it isn't a silent no-op.

[tianfeng92]

Agreed on the limitation — one shared cert means one identity for every backend, and fanning out to several backends is exactly when that breaks down.

Deferring rather than doing it here: it changes the API shape (bool -> per-exporter Secret ref) and I'd rather land that with the tlsroute-style plumbing and its own tests.

One thing I did fix from this comment: mutualTLS: true on an http:// endpoint is no longer a silent no-op, it's a validation error:
https://github.com/tigera/operator/blob/322d8b439/pkg/controller/otelcollector/controller.go#L499-L501

Filing a follow-up for the per-exporter cert. Worth flagging it should land before GA — swapping the field afterwards is a breaking change.

[tianfeng92]

Correcting my earlier reply on this thread: I said I was deferring the per-exporter client cert to a follow-up. That's no longer accurate — @rene-dekker asked for the same thing independently, so I've implemented it rather than deferring.

mutualTLS is gone. Each exporter now carries its own TLS and auth:

https://github.com/tigera/operator/blob/ba5aeb9d4/api/v1/opentelemetry_types.go#L92-L127

exporters:
  - name: datadog
    endpoint: https://otlp.datadoghq.com:4318
    protocol: http
    auth:
      headers:
        - name: DD-API-KEY
          valueFrom:
            secretKeyRef: { name: otlp-datadog-auth, key: api-key }
    tls:
      caConfigMapName: corp-ca
      clientCertSecretName: corp-client-mtls

Two notes on the implementation:

Header values are read from Secrets into environment variables and referenced as ${env:...}, so credentials never appear in the rendered ConfigMap. There's a test asserting the secret value is absent from it.

I used caConfigMapName rather than the caSecretName in Rene's sketch — a CA isn't secret, and syslog/splunk already carry user CAs as ConfigMaps. The client cert stays a Secret since it holds a private key. Happy to switch if you'd both rather it were a Secret for symmetry.

The silent no-op you flagged is now a validation error, covering the CA as well as the client cert:
https://github.com/tigera/operator/blob/ba5aeb9d4/api/v1/logcollector_types.go#L340-L350

Deployed and verified on a cluster — logs and metrics both still flowing. The per-exporter TLS and auth paths themselves are unit-tested but not yet exercised against a real TLS backend.

[rene-dekker]

I agree with Pasan's feedback in that I think that the TLS and Authn for exporters need more flexibility in their configuration. I think both authentication and transport may be set differently per sink and so each may need their own sub struct for configuration.

From what I understand there are a few ways you may want to configure them:

• mTLS
• TLS with headers (can be bearer token, or other headers like X-SF-Token, DD-API-KEY)
• no TLS (leverage it with a gateway for example)

Perhaps something of this shape would work better:

  otelCollector:                                                                                                                                                                          
    exporters:                                                                                                                                                                            
      - name: datadog                                                                                                                                                                     
        endpoint: https://otlp.datadoghq.com:4318                                                                                                                                         
        protocol: http                                                                                                                                                                    
        auth:                                                                                                                                                                             
          headers:                                                                                                                                                                        
            - name: DD-API-KEY                                                                                                                                                            
              valueFrom:                                                                                                                                                                  
                secretKeyRef: { name: otlp-datadog-auth, key: api-key }                                                                                                                                                                                                                   
        tls:                                                                                                                                                                              
          caSecretName: corp-ca
          clientCertSecretName: corp-client-mtls # when set, implies tls.

Alternatively, make a transport sub struct and include both tls settings and protocol there.

[rene-dekker]

Another note is that Boolean fields in the API spec are not idiomatic; we should refrain from using them.

[rene-dekker]

We could end up with Enabled: Disabled, I think that reads a bit weird, maybe look at other instances in our repo for these kinds of flags, I think we use other field names, like state | status | mode.