Repository navigation
The Kubernetes credential provider exports metrics to the cluster collector - #3379
Eric Curtin (ericcurtin) wants to merge 5 commits into
Conversation
…lector The Deployment had no envFrom for ate-otel-config, so the OTLP exporter dialed localhost:4317. It now also sets the pod resource attributes like the other components. Signed-off-by: Eric Curtin <eric.curtin@docker.com>
|
Lior Lieberman (@LiorLieberman) Haven Xia (@HavenXia) PTAL when you get a chance. Thank you! |
|
Julian Gutierrez Oschmann (@juli4n) Jeff Luo (@JeffLuoo) PTAL when you get a chance. CI is green. Thank you! |
Jeff Luo (JeffLuoo)
left a comment
Jeff Luo (JeffLuoo)
left a comment
There was a problem hiding this comment.
Thanks for the fix. The envFrom change is correct. I found two gaps outside the diff that I think must be fixed before merge. Both come from the same cause: the provider is now a consumer of ate-otel-config, but the restart code and the docs do not include it.
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
|
Done: provider added to |
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
|
Jeff Luo (@JeffLuoo) Haven Xia (@HavenXia) PTAL when you get a chance, feedback addressed. Thank you! |
Jeff Luo (JeffLuoo)
left a comment
Jeff Luo (JeffLuoo)
left a comment
There was a problem hiding this comment.
Thanks, both blockers are fixed. I approve this PR. Because of the release freeze, it can merge on 11/18 or later.
Three small changes before the merge, if you agree:
cmd/ate-setup/internal/steps/overlay.go: use thek8sCredentialProviderDeploymentconstant instead of the string"k8s-credential-provider", here and inoverlay_test.go.TestK8sCredentialProviderManifestsAgreecompares that constant with the manifest. If someone renames the Deployment, the restart stops with no error, becauseRolloutRestartDeploymentignores a missing workload.docs/guides/observability.md: the provider calls onlyserverboot.InitMetrics, so it sends no traces and no OTLP logs. Please remove it from the list of components whose telemetry goes to Jaeger and to thedebugexporter, and add a sentence that says that it sends only metrics.manifests/ate-install/ate-otel-config.yaml: "so restart it after editing this file" makes it look as if the other consumers restart automatically. They do not (see the NOTE below it). Suggestion: "...comes from manifests/egress-credential-injection, so a restart of the bundle workloads does not restart it."
…y metrics Signed-off-by: Eric Curtin <eric.curtin@docker.com>
|
Jeff Luo (@JeffLuoo) Done in ffcf842, thank you! |
Without envFrom for ate-otel-config the provider's OTLP exporter dialed localhost:4317, so its metrics were lost. Adds it, plus the pod resource attributes the other components set.
Fixes #3282
AI use: AI-assisted (opencode). I reviewed the diff and ran the tests.