agent-substrate / agent-substrate/substrate

OTLP exporters hardcode WithInsecure(), so the SDK's TLS config never applies

未關閉
#741 5 則留言 0 個 reaction 已指派 1 人 已被 @git286 認領 在 GitHub 檢視
area/observability kind/feature prio/P0
主要語言
Go
星號
1.8k
分支
316
平均合併
2 天 43 分鐘
30 天內合併 PR
287

描述

Related: #563

`serverboot.go:141` builds the trace exporter with an unconditional `otlptracegrpc.WithInsecure()`, and `serverboot.go:182` does the same for metrics. `internal/benchmarking/boomer/trace/trace.go:89` repeats it.

The OTel SDK reads env config first and applies explicit options after, so that one option overrides everything the exporter already knows how to do:

```go
cfg = ApplyGRPCEnvConfigs(cfg) // OTEL_EXPORTER_OTLP_CERTIFICATE, CLIENT_CERTIFICATE,
for _, opt := range opts { // CLIENT_KEY, INSECURE, and http:// vs https:// inference
cfg = opt.ApplyGRPCOption(cfg) // ...all discarded here
}
```

So `OTEL_EXPORTER_OTLP_ENDPOINT=https://collector:4317` silently exports plaintext. No error, no warning. Already noted in otel-collector.md:458 as a known constraint.

The comment justifies it with GKE managed OTel, which is fair — that collector genuinely has no TLS receiver you can configure. But `serverboot` boots every binary in every environment, and we document a self-managed DaemonSet path (otel-collector.md:64) where TLS and mTLS work fine. A GKE Preview limitation shouldn't be compiled in for everyone.

## Fix

Delete the `WithInsecure()` calls and let scheme inference plus the standard env vars do their job. Log the resolved transport next to the existing "Tracing initialized" line so this is visible without reading source. Then fix the "TLS is not supported" text in otel-collector.md:458 and tracing.md:49.

Not proposing we enable TLS by default or wire pod certs into the exporter — managed OTel can't accept either. This is just about not blocking people whose collector can.

## Compat

No manifest changes needed. Every Go-SDK endpoint we ship already has an explicit `http://` scheme (the four in `ate-install/`, the five under `kind/`), which infers plaintext. Unchanged on GKE and Kind. The scheme-less `AGENTGATEWAY_OTLP_ADDRESS` is agentgateway's, not ours.

One thing to check first: with no `OTEL_EXPORTER_OTLP_ENDPOINT` set, the SDK defaults to `localhost:4317` with `Insecure: false`, so those processes would start attempting TLS. Need to confirm before merging:

- [ ] do ateom-gvisor / ateom-microvm inherit the env from atelet? They call `InitTracing` but have no manifest of their own
- [ ] atecontroller — ate-controller.yaml:96 sets the endpoint but I don't see it in the `InitTracing` call sites
- [ ] local `go run` and any test that boots a real exporter

Failure mode is background export errors rather than a crash, but worth sorting out rather than finding in prod.

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。