Skip to content

fix(opampextension): skip no-op RemoteConfig + advertise ReportsRemoteConfig (ASAPCollector#381 Issue #4) - #389

Merged
zzylol merged 1 commit into
mainfrom
b1-issue4-skip-noop-remote-config
May 18, 2026
Merged

zzylol merged 1 commit into
mainfrom
b1-issue4-skip-noop-remote-config

Conversation

@zzylol

@zzylol zzylol commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the apply-loop the smoke test surfaced after ASAPQuery-backend #279 + #281 landed. Two fixes:

  1. No-op short-circuit in processRemoteConfig: compare incoming body to on-disk file before writing. If identical, just report APPLIED and return — no restart. Pairs with the deterministic emit from ASAPQuery-backend feat(asap-gorilla): Phase 1 block format encoder + decoder + index file #281 (same semantic content → same bytes, so byte comparison is accurate).
  2. Advertise ReportsRemoteConfig capability alongside AcceptsRemoteConfig. Without it, opampClient.SetRemoteConfigStatus(APPLIED) returns ErrReportsRemoteConfigNotSet — the client lib won't record the applied hash, server keeps resending.

Smoke flow post-fix

  1. Agent boots from bootstrap YAML
  2. Receives push, bytes differ, applies + exits
  3. Restarts, loads new YAML
  4. Receives push, bytes IDENTICAL (deterministic emit), no-op fires → keeps running
  5. Pipeline emits sketches; smoke test Axis D registers them

🤖 Generated with Claude Code

…eConfig (ASAPCollector#381 Issue #4)

Two paired fixes to close the apply-loop the smoke test surfaced
after ASAPQuery-backend PR #279 landed:

(1) No-op short-circuit in processRemoteConfig

Before writing the pushed body to disk + exiting, compare to the
current on-disk file. If they match byte-for-byte, just report
APPLIED and return — don't restart. This handles two scenarios:

  * Defense-in-depth against a controller resending the same
    config (which can happen any time the OpAMP server's
    LastRemoteConfigHash cache is empty or stale — e.g. on
    reconnect before the agent has reported APPLIED).
  * The deterministic-emit guarantee from ASAPQuery-backend #281
    means same-semantic-content → same bytes, so this check is
    accurate for the controller's emit.

(2) Advertise ReportsRemoteConfig capability

In addition to AcceptsRemoteConfig, set
`AgentCapabilities_AgentCapabilities_ReportsRemoteConfig` when
`remote_config_path` is configured. Without this bit,
`opampClient.SetRemoteConfigStatus(APPLIED)` returns
`ErrReportsRemoteConfigNotSet` (opamp-go
client/internal/clientcommon.go:21), the client lib refuses to
record the applied hash, and the server has no way to learn the
config was processed — keeping it stuck in the resend loop. Both
bits are needed for the full feedback cycle.

End-to-end smoke (post-#281 deterministic emit + this PR):
  * Agent boots from bootstrap YAML
  * Receives RemoteConfig push, bytes differ, applies + exits
  * Restarts, loads new YAML
  * Receives RemoteConfig push, bytes IDENTICAL (deterministic emit)
  * No-op short-circuit fires → SetRemoteConfigStatus(APPLIED) →
    keeps running, no more restarts
  * Pipeline emits sketches normally; smoke test Axis D registers
    them

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit c9cd819 into main May 18, 2026
@zzylol
zzylol deleted the b1-issue4-skip-noop-remote-config branch May 18, 2026 01:30
zzylol added a commit that referenced this pull request May 18, 2026
…locks (#391)

PR #383 (and #389 follow-up) added the `RemoteConfigPath` field to the
patched opampextension: when set, the extension applies pushed
AgentRemoteConfig by writing the YAML to disk and exiting, so a
restart-policy-enabled supervisor (Docker `restart: unless-stopped`,
systemd, k8s) brings the agent back up with the new config. The
mvp-smoke-test agent uses this field via its overlay-mounted
`agent.yaml`, but the 13 stock agent yamls in
`deploy/mvp-singlenode/configs/asap-otel-agent-*.yaml` still ship
without it, so OpAMP-apply silently no-ops in stock single-node deploys
(the controller's push is received, the extension reports back, but
nothing is persisted across the implied restart — the next boot reloads
the same static bootstrap yaml).

This change adds `remote_config_path: /etc/otel/config.yaml` (the same
path the collector boots from via `--config=/etc/otel/config.yaml`, and
the same convention the smoke-test uses) to the `extensions.opamp:`
block of every stock yaml that already declares one.

Files touched (13):
  - asap-otel-agent-allsketches.yaml
  - asap-otel-agent-b0-prometheus.yaml
  - asap-otel-agent-b0-victoriametrics.yaml
  - asap-otel-agent-b1-serf-prometheus.yaml
  - asap-otel-agent-b3-delta-direct.yaml
  - asap-otel-agent-b5-gorilla-prometheus.yaml
  - asap-otel-agent-b5-gorilla-victoriametrics.yaml
  - asap-otel-agent-b6-asap-single-sketch.yaml
  - asap-otel-agent-b6-gorilla-s3.yaml
  - asap-otel-agent-cms-direct.yaml
  - asap-otel-agent-cs-direct.yaml
  - asap-otel-agent-hll-direct.yaml
  - asap-otel-agent-kll-direct.yaml

The 6 stock yamls WITHOUT an `extensions.opamp:` block
(`b0a-raw-stream`, `b0b-raw-batched`, `b2-full`, `b3-delta`,
`b4-tunable`, `kafka-fragment`) are intentionally left alone — adding
`remote_config_path` would require also wiring up `extensions:` +
`service.extensions:`, which is out of scope for this sweep.

OPERATIONAL CAVEAT (flagged inline + in each yaml's comment): the
stock `agents-N{1,10,100}.yml` compose overlays mount
`/etc/otel/config.yaml` read-only (`:ro` suffix on the bind mount) and
do NOT set a restart policy on the agent service. As-shipped, the
extension's disk-write will fail with EROFS and the agent process will
exit without coming back. To actually exercise OpAMP-apply persistence
in a stock deploy, the operator must additionally:

  1. Drop `:ro` from the agent's config volume mount (so the
     opampextension can write back to the same path the collector
     boots from), and
  2. Add `restart: unless-stopped` (or equivalent) to the agent
     service so the container respawns after the extension exits.

The smoke-test's `mvp-smoke-test/compose/smoke-overlay.yml` does both
via `volumes: !override` + `restart: unless-stopped` — that is the
reference pattern. The compose-side wiring update is deferred to a
follow-up so this PR stays narrowly scoped to the agent yaml schema.

YAML validity verified with `python3 -c "import yaml; ..."` over all
19 stock agent yamls; the 13 modified files parse with
`extensions.opamp.remote_config_path == "/etc/otel/config.yaml"`.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant