install: make the telemetry stack optional and explicit - #1289
install: make the telemetry stack optional and explicit#1289Jeff Luo (JeffLuoo) wants to merge 5 commits into
Conversation
The ate-otel-config ConfigMap named the collector of the GKE managed OTel addon for each cluster that is not kind. On a cluster without that addon the namespace does not exist, thus each component failed to find the collector one time each minute and the telemetry stopped with no message. It reads as a fault of the network, and not as an absent dependency. Substrate must also run on a cluster that has no collector at all, and it must not assume one vendor. hack/install-ate.sh now takes --observability none|otlp|gke|kind. Mode none is the default and exports nothing; each component still serves its own /metrics endpoint. Mode otlp uses the address of --otlp-endpoint, mode gke the collector of the addon, and mode kind the in-cluster collector that a kind install applies, which is also its default. One file supplies the ate-otel-config ConfigMap for each mode, and a preflight stops the install with a message when the collector of the mode is absent. No bundle holds a copy of that ConfigMap now, thus the install applies it one time, ahead of the workloads. That removes the patch after each apply and the rollout restart that came with it. An install on GKE must now give --observability=gke.
|
Benjamin Elder (@BenTheElder) as we discussed in #1258, I have this PR to make observability stack more configurable by breaking it down into four options If we want to further make it abstract for the concept of cluster type like kind and GKE, I can change the implementation to just two options |
| name: ate-otel-config | ||
| namespace: ate-system | ||
| data: | ||
| OTEL_EXPORTER_OTLP_ENDPOINT: "" |
There was a problem hiding this comment.
The main exporters in serverboot.go are built unconditionally, and the OTel Go SDK's env parsing treats an empty variable as unset. So when the mode is none, the exporter falls back to the SDK default endpoint, localhost:4317, which will error, right?
There was a problem hiding this comment.
I plan to do this in a follow-up PR.
Note that it is not a regression: before this PR the same components failed at the same rate against gke-managed-otel. I added a line to docs/observability.md that says mode none stops the export but not the SDK error log yet and will clean it up in my follow-up PR.
| # No bundle carries a copy of this ConfigMap. Thus this is the one apply of it, | ||
| # and the install needs no patch after the bundle and no restart of the | ||
| # workloads that read it. | ||
| apply_otel_config() { |
There was a problem hiding this comment.
Every deploy path calls apply_otel_config, including the targeted ones (-- deploy-atelet etc.). The mode only lives in the flags, so if I installed with
--observability=gke a while ago and later run ./hack/install-ate.sh --deploy-atelet without repeating the flag, this quietly applies the none ConfigMap over my gke one. The redeployed component loses telemetry right away and the rest lose it whenever they next restart, which makes it really hard to trace back. Could we leave the ConfigMap alone (or read the mode back from the cluster) when --observability wasn't explicitly given and a config already exists?
There was a problem hiding this comment.
Good catch.
The install now writes the mode on the ConfigMap as the ate.dev/observability-mode annotation, and reads it back when the command line gives no mode. Order: --observability, then --otlp-endpoint, then the mode of the cluster, then kind on a kind install, then none. Thus --deploy-atelet after an install with --observability=gke keeps mode gke, and the install reports Observability: mode gke (from the cluster).
| @@ -591,9 +559,6 @@ deploy_ate_system() { | |||
| run_kubectl rollout status deployment/atenet-router -n ate-system --timeout="$(rollout_timeout)" | |||
| run_kubectl rollout status deployment/atenet-egress -n ate-system --timeout="$(rollout_timeout)" | |||
| run_kubectl rollout status daemonset/atelet -n ate-system --timeout="$(rollout_timeout)" | |||
|
|
|||
| # After the bundle, which carries its own copy of ate-otel-config. | |||
| apply_otel_endpoint_override | |||
There was a problem hiding this comment.
There's no way to switch modes on an existing cluster anymore. The components only read ate-otel-config when a pod starts — updating the ConfigMap does nothing to pods that are already running. So if I re-install with a different --observability mode, the ConfigMap changes but no pod restarts (nothing in the pod spec changed), and everything keeps using the old endpoint. The deleted apply_otel_endpoint_override handled this: when the endpoint changed, it restarted the four workloads that read the config. We need that restart back in some form, the docs say re-installing is how you change modes, and right now that silently does nothing.
There was a problem hiding this comment.
Updated:
- apply_otel_config now compares the new ConfigMap with the one in the cluster and records a change. restart_otel_consumers will restartsystem components when the collector changed.
- Updated documentation to align with what code does.
A deploy of one component read the mode from the flags only. Thus `--deploy-atelet`, with no --observability after an install with --observability=gke, put the ConfigMap of the default mode over the one that worked. The component that the command redeployed lost its collector at once, and each other component lost it at its next restart, far from the cause. The install now writes the mode on the ConfigMap, as the ate.dev/observability-mode annotation, and reads it back when the command line gives no mode. A ConfigMap from an install that came before the modes carries no annotation, thus the mode comes from the endpoint in it. The report of the install names the mode and its source. A change of the collector also needs a restart, which this change adds back: the components read the ConfigMap with envFrom, thus a new ConfigMap on its own changes no pod template, starts no rollout, and each running pod keeps the collector of the install before it. The restart comes after the waits for the rollouts, because a restart during the rollout of the bundle makes the two compete.
| # note_otel_config_change compares the ConfigMap of this install with the one in | ||
| # the cluster. Nothing to compare on a first install, thus no change: the | ||
| # workloads that come after it read the new ConfigMap when they start. | ||
| note_otel_config_change() { |
There was a problem hiding this comment.
AI finding:
The cluster snapshot this compares against is read once and never updated, so with more than one deploy target the restart fires twice. E.g. on a mode-none cluster, ./hack/install-ate.sh --observability=gke --deploy-atelet --deploy-atenet: the atelet path sees none→gke and restarts all four consumers, then the atenet path compares against the same cached "none" (the ATE_OBSERVABILITY_CLUSTER_READ guard blocks a re-read), decides the collector changed again, and restarts everything a second time.
Since every ate-controller restart also rolls all the WorkerPools, that's the running workers and actors replaced twice in one install.
Could we update the cached mode/endpoint after the apply (or after the restart), so the second comparison sees the value we just wrote?
There was a problem hiding this comment.
Fixed.
note_otel_config_change now takes the values it applied as the values of the cluster, in the same call, thus each target after the first compares with what the first one wrote. Your suggestion, with the update at the comparison and not after the restart, so a deploy path that ends before the restart cannot leave a stale snapshot behind.
| # | ||
| # An empty endpoint is an error for the same reason. It is the value of | ||
| # --observability=none, in which the control plane has no collector; the actors | ||
| # then need --otlp-endpoint, or a collector for the whole install. |
There was a problem hiding this comment.
I think this breaks the benchmark automation. orchestrator.py tears down and reinstalls with plain --deploy-ate-system without --observability flag, so the fresh install always ends up in mode none with an empty endpoint. So every automated run dies at setup before any benchmark starts?
…marks running Two faults of the change before this one. A command line can name more than one deploy target, and each target applies the ate-otel-config ConfigMap. The comparison with the cluster used a snapshot that the first target read and no target refreshed, thus each target after the first found the same change again and restarted the four workloads a second time. Each of those restarts rolls every WorkerPool, thus one install replaced the running workers, and the actors on them, two times. The comparison now takes the values it applied as the values of the cluster. The benchmark automation reinstalls substrate for each test, with no mode on the command line, thus each install came up with no collector. benchmarking/workloads/deploy.sh reads the collector of the control plane and gives it to the actors, and it stops when there is none, thus each automated run stopped at setup. The orchestrator now names the collector of the GKE managed addon, which is where that harness runs. ATE_OBSERVABILITY selects a different mode, and a test that names its own collector in ateArgs keeps it.
The documentation of mode none said that each component still serves its own /metrics endpoint. Only ateapi, atelet, and atenet-router do: ate-controller and the ateoms push their metrics and run no metrics server, thus they emit nothing at all in that mode. An operator who reads the old text would look for an endpoint that does not exist. Two more items of the same review: A mode that comes from the ate.dev/observability-mode annotation, and that is not a known one, reported "--observability must be ..." for a value that no flag gave. The message now names the source, and tells the operator which annotation holds the value. hack/ate-dev-env.sh.example gains a commented ATE_OBSERVABILITY, because `go run ./tools/setup-gcp` enables the GKE managed OTel addon on the cluster it makes, thus mode gke is the one that most developers want there, and the dev env file is where they can set it one time.
The exporters were built whatever the environment said. The OTel SDK reads an empty OTEL_EXPORTER_OTLP_ENDPOINT as an absent one and falls back to its own default of localhost:4317, thus a component on a cluster with no collector logged a failed export once each tick, for its whole life. The telemetry stack is optional, thus a cluster that wants no telemetry must also be quiet. serverboot now builds no OTLP exporter when OTEL_SDK_DISABLED is true, when OTEL_TRACES_EXPORTER or OTEL_METRICS_EXPORTER is none, or when the endpoint of the signal and the endpoint of each signal are both empty. The two exporter variables are the standard OTel switches, and --observability=none writes them on the ate-otel-config ConfigMap together with the empty endpoint. The endpoint has no part in the decision for an ateom, which exports over the socket of atelet's relay; the relay makes the same decision for itself. The providers stay registered with no exporter, and do not become no-op ones. Thus the spans keep their IDs and the logs keep the trace_id that joins them, the Prometheus surface of ateapi, atelet, and atenet-router stays, and each call site keeps one code path.
The ate-otel-config ConfigMap named the collector of the GKE managed OTel addon for each cluster that is not kind. On a cluster without that addon the namespace does not exist, thus each component failed to find the collector one time each minute and the telemetry stopped with no message. It reads as a fault of the network, and not as an absent dependency. Substrate must also run on a cluster that has no collector at all, and it must not assume one vendor.
hack/install-ate.shnow takes--observability none|otlp|gke|kind. Modenoneis the default and exports nothing; each component still serves its own/metricsendpoint. Modeotlpuses the address of--otlp-endpoint, modegkethe collector of the addon, and modekindthe in-cluster collector that a kind install applies, which is also its default. One file supplies theate-otel-configConfigMap for each mode, and a preflight stops the install with a message when the collector of the mode is absent.Part of #1258