Skip to content

refactor(emit): drop aggregation_id from OTAP YAML + Telegraf TOML wire - #251

Merged
zzylol merged 1 commit into
mainfrom
retire-edge-sketch-aggregation-id
May 15, 2026
Merged

zzylol merged 1 commit into
mainfrom
retire-edge-sketch-aggregation-id

Conversation

@zzylol

@zzylol zzylol commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Mirrors the PR-5 wire-cleanup pattern (#244 / #246 / #250), now for the edge-side runtime emitters. The patched asap-otel processors (any runtime: OTel collector / OTAP / Telegraf) don't consume the controller-allocated aggregation_id string — sid identity is content-addressed at the backend via (metric_name, attrs_fingerprint, agg_kind_canonical), and policy_fp content-matches via (metric, sketch_kind, config, group_by_keys). The controller-allocated string is dead weight on the wire.

The OTel-collector YAML emitter (emit_edge_yaml) was already clean — an existing test asserts the agent YAML does NOT contain aggregation_id:. This PR catches the two remaining edge-runtime emitters:

  • emit/otap.rs::build_asap_sketches_config — drops the aggregation_id map entry from the OTAP DAG YAML.
  • emit/telegraf.rs::emit_processors_allsketches — drops the aggregation_id = ”…” TOML line.

EdgeSketchProcessor.aggregation_id stays on the struct as internal emitter plumbing for cross-stage references during the DAG walk (SketchAgg → BackendAggregation → BackendReadout); it just doesn't reach the wire from any runtime emitter anymore.

Verified asap-otel doesn't consume the field

grep -rln "aggregation_id\|AggregationId" /mydata/ASAPCollector/opentelemetry-collector-contrib/processor/{ddsketch,hll,kll,countsketch,countminsketch}processor/ → zero matches. The patched processors don't read the field on any runtime; the agent silently ignores it on the OTel-collector path too.

Test plan

  • cargo test --lib -p control_plane: 690 passed; 0 failed
  • cargo test --tests --bins -p control_plane: 27 passed; 0 failed
  • No tests asserted the field was present in OTAP / Telegraf output, so no test-flips needed.

🤖 Generated with Claude Code

Mirrors #244 / #246 / #250's PR-5 wire-cleanup pattern, now for the
edge-side runtime emitters. The patched asap-otel processors (any
runtime — OTel collector / OTAP / Telegraf) don't consume the
controller-allocated `aggregation_id` string: sid identity is
content-addressed at the backend via `(metric_name,
attrs_fingerprint, agg_kind_canonical)`, and `policy_fp` content-
matches via `(metric, sketch_kind, config, group_by_keys)`. The
controller-allocated string is dead weight on the wire.

The OTel-collector YAML emitter (`emit_edge_yaml`) was already
clean — there's an existing test at stage_config.rs asserting the
agent YAML does NOT contain `aggregation_id:`. This PR catches the
two remaining edge-runtime emitters that still spelled it out:

- `emit/otap.rs::build_asap_sketches_config` (line ~287): drop
  the `aggregation_id` map entry; keep `sketch_kind` etc.
- `emit/telegraf.rs::emit_processors_allsketches` (line ~158):
  drop the `aggregation_id = "…"` TOML line; keep `sketch_kind`.

`EdgeSketchProcessor.aggregation_id` stays on the struct as
internal emitter plumbing for cross-stage references during the
DAG walk (`SketchAgg → BackendAggregation → BackendReadout`); it
just doesn't reach the wire from any runtime emitter anymore.

Tests: 690 lib + 27 binary tests pass. (No test asserted the field
was present in OTAP / Telegraf output, so nothing to flip.)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 9e811c4 into main May 15, 2026
@zzylol
zzylol deleted the retire-edge-sketch-aggregation-id branch July 17, 2026 20:06
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