FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

ate-setup: make observability optional and configurable by JeffLuoo · Pull Request #1289 · agent-substrate/substrate · GitHub

ate-setup: make observability optional and configurable - #1289

Open
Jeff Luo (JeffLuoo) wants to merge 4 commits into
agent-substrate:mainfrom
JeffLuoo:observability-optional
Open

ate-setup: make observability optional and configurable#1289
Jeff Luo (JeffLuoo) wants to merge 4 commits into
agent-substrate:mainfrom
JeffLuoo:observability-optional

Conversation

Jeff Luo (JeffLuoo) commented Aug 28, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Decouples Substrate from a hardcoded GKE OTel collector by making telemetry optional and explicit via --observability none|otlp|gke|kind (and --otlp-endpoint).

On clusters without a collector, Substrate now runs cleanly without export retry spam, while preserving request trace correlation IDs in logs and /metrics pull endpoints.


Key Changes

  • Explicit Modes (cmd/ate-setup):
    • none: No OTLP push; runs cleanly without a collector.
    • kind (default for local): Points to the in-cluster Kind collector.
    • gke: Points to GKE managed OTel addon (gke-managed-otel).
    • otlp: Points to a custom --otlp-endpoint.
  • Cluster State & Idempotence:
    • Persists active mode as an annotation (ate.dev/observability-mode) on the ate-otel-config ConfigMap.
    • Targeted redeploys (e.g. ate-setup deploy atelet) preserve the cluster's existing mode instead of resetting to defaults.
    • Pre-existing unannotated clusters have their mode inferred from the endpoint for backwards compatibility.
  • Graceful Zero-Collector Handling (internal/serverboot):
    • An empty endpoint or none disables OTLP export without failing back to localhost:4317.
    • Instantiates TracerProvider without an exporter rather than noop, keeping valid trace_ids in structured logs.
    • Preserves pull-based Prometheus /metrics endpoints across all components.
  • Safety & Restarts:
    • Preflight validation: Verifies in-cluster collector Service/Namespace exists before deploying workloads.
    • Minimal disruption: Only triggers rolling restarts of consumer workloads if the collector configuration actually changed.

Verification

  • Tests pass
  • Appropriate changes to documentation are included in the PR

Jeff Luo (JeffLuoo) commented Aug 28, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator Author

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 --observability none|otlp|gke|kind.

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 --observability none|otlp, and if otlp is selected, an endpoint needs to be passed from flag as well. Having GKE and Kind as options for the simplicity of configuring --otlp-endpoint.

name: ate-otel-config
namespace: ate-system
data:
OTEL_EXPORTER_OTLP_ENDPOINT: ""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

Jeff Luo (JeffLuoo) Aug 31, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread hack/install-ate.sh
Comment thread hack/install-ate.sh
Comment thread hack/observability.sh Outdated

Jeff Luo (JeffLuoo) commented Sep 8, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Because of #1503, I will refactor this change. cc - Da Huang (@git286)

Jeff Luo (JeffLuoo) changed the title install: make the telemetry stack optional and explicit ate-setup: make the telemetry stack optional and explicit Sep 8, 2026
Jeff Luo (JeffLuoo) changed the title ate-setup: make the telemetry stack optional and explicit ate-setup: make observability optional and configurable Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Impact on ai-on-gke/substrate-gke (https://github.com/ai-on-gke/substrate-gke)

This change has no effect on that repository today. Its installer keeps a pin to an upstream commit, Commit = "cbae8250" (installer/internal/snapshot/snapshot.go:59). That commit is older than this change. Thus nothing changes there before a person moves the pin.

After a move of the pin, that repository still needs no change. Its runs go run ./cmd/ate-setup deploy ate-system. It gives the image flags only, and no --observability flag. There are two cases:

  • A first install. The script runs setup-gcp bootstrap before the install. That step enables the managed OTel addon, and the reconcile for a cluster that exists). The install then finds the collector of the addon, and selects mode gke. Thus the telemetry of the quickstart stays as it is today.
  • An install on a cluster that has substrate. The ate-otel-config ConfigMap of that cluster holds the endpoint, and the install keeps the mode of it. A ConfigMap from an install before this change holds no annotation. The install then reads the mode from the endpoint in it.

return err
}
}
return e.Kube.RolloutRestart(ctx, NamespaceAteSystem, "atelet", now)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Does this work? Doesn't it need the suffix? You can likely use e.AteletDaemonSetName() I think.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed.

The restart now uses RestartAteletDaemonSets, which selects the DaemonSets by the app=atelet label. I did not use AteletDaemonSetName, because a cluster in a rolling upgrade has one DaemonSet for each version. Each of them reads the ConfigMap, thus each of them needs the restart.

I tested this on a live GKE cluster.

// Only mode otlp keeps its address in the ConfigMap. Each other
// mode holds it in a manifest.
if mode == config.ObservabilityOTLP {
endpoint = e.observability.clusterEndpoint

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would this get reverted?

echo "Overriding OTEL_EXPORTER_OTLP_ENDPOINT with ${ATE_OTLP_ENDPOINT}"
run_kubectl -n ate-system patch configmap ate-otel-config --type=merge \
-p "{\"data\":{\"OTEL_EXPORTER_OTLP_ENDPOINT\":\"${ATE_OTLP_ENDPOINT}\"}}"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good catch, fixed!

case config.ObservabilityGKE:
return e.Cfg.Manifest("otel", "gke", "ate-otel-config.yaml")
default:
return e.Cfg.Manifest("otel", "none", "ate-otel-config.yaml")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We are missing OTEL_METRIC_EXPORT_INTERVAL, would this reintroduce #634?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Issue #634 does not come back, but I found a different problem near it. Mode otlp holds its address in no manifest, thus it used the file of mode none and lost the tick. A kind cluster with its own collector went back to the 60s default of the SDK. The patch of the shell script kept the tick, because it changed the endpoint only.

Fixed in latest patch -> Mode otlp now uses the kind file on a kind install, and the none file on each other install. The tick belongs to the cluster, and not to the collector.

The ate-otel-config ConfigMap named the collector of the GKE managed OTel
addon on each cluster that is not kind. A cluster without that addon has no
gke-managed-otel namespace, 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, and it must not assume one vendor.

ate-setup now takes --observability none|otlp|gke|kind. With no flag, a first
install tests the cluster for the collector of the managed addon and takes mode
gke when it is there, and mode none when it is not. Mode none 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 under manifests/ate-install/otel/
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.

The mode is state of the cluster: the install writes it as the
ate.dev/observability-mode annotation and reads it back, thus a deploy of one
component keeps the collector that the cluster has. No kustomize bundle carries
a copy of the ConfigMap, thus the installer is the one writer and no later
apply replaces the selected mode.

internal/serverboot builds no OTLP exporter when the endpoint is empty, when
OTEL_SDK_DISABLED is true, or when the standard exporter variables say none.
The tracer provider stays registered, thus the spans keep their IDs and the
logs keep the trace_id that joins them.

hack/install-ate.sh keeps its two cases, the kind collector for a kind install
and the GKE managed one for each other install. It follows the files to their
new place and gets no mode flag, because ate-setup replaces it.
Two workloads kept the collector of the install before them, with no
message, when the mode changed.

An atelet DaemonSet carries the version suffix of the install that made
it (atelet-${SUBSTRATE_VERSION_SUFFIX}), thus the restart of a DaemonSet
named atelet reached nothing at all, and reported nothing: an absent
DaemonSet is not an error, because a deploy of one component has only
that component. The restart now goes by the app=atelet label, which also
covers the second DaemonSet that a cluster in a rolling upgrade holds.

atenet-egress reads the ConfigMap with envFrom, as its own manifest says,
and it was absent from the list of Deployments. Both egress manifests
declare one Deployment of that name, thus one entry covers the shipped
gateway and the sdsmint one.

A test now holds the list to the manifests: it scans them for each
workload that takes the ConfigMap through envFrom, and fails when one of
them has no restart. A second test stamps the workloads of a cluster in a
rolling upgrade and reads the annotation back.
The ate-otel-config ConfigMap has more than one writer, thus its mode
annotation and its endpoint can disagree. hack/install-ate.sh patches the
collector of a measurement into the data and left the annotation of the
mode before it. ate-setup then read that annotation, rendered the file of
that mode, and put its collector back over the measurement, with no
message. It also found a change and restarted each consumer for it.

Both sides now hold the two together. The patch of the shell script
stamps the mode annotation, and the two exporter switches with it,
because the file of mode none turns them off. ate-setup takes the
endpoint over an annotation that disagrees with it, with a warning: the
components read the endpoint, thus the endpoint is the collector of the
cluster and the annotation is the stale half. Mode otlp is out of that
test, because no manifest holds its address and the two cannot disagree.

The restart list of the shell script gains atenet-egress, which reads the
ConfigMap and was absent from it, as in ate-setup.
…n collector

Mode otlp holds its address in no manifest, thus it takes the file of
another mode and the install writes the address into it. That file was
the one of mode none on each install, thus a kind cluster that named its
own collector lost OTEL_METRIC_EXPORT_INTERVAL and
OTEL_METRIC_EXPORT_TIMEOUT and went back to the 60s export tick of the
SDK. The patch of the shell script kept them, because it replaced the
endpoint only.

The two belong to the cluster and not to the collector: a component is
invisible to the collector until its first tick, and the metrics e2e
suite asserts against the scrape of the collector on a bounded deadline,
which is the flake that the 10s tick removed. Mode otlp now takes the
file of mode kind on a kind install, and the file of mode none on each
other install.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL