fix(processors): retire dead MetricSuffix field — no sketch processor renames metric names - #382
Merged
Merged
Conversation
… renames metric names
Closes the audit thread on "do asapcollector processors add a metric-
name suffix on the wire?" The runtime answer is no — the 2026-05
refactor moved sketch-type identification from name suffixes to the
OTLP `pdata.Metric` variant tag (DDSketch / KLLSketch / HLLSketch /
CountSketch / CountMinSketch), and every production-path encode site
now preserves the input metric name:
* DDSketch (`shim_helpers.go::appendSketchMetrics`): `env.MetricName = inputName`
* KLL transmit_sketch=true (`encode.go::sketchMetricName`): returns base
* HLL (`encode.go::cardinalityMetricName`): returns base
* CountSketch / CountMin: no MetricSuffix field at all
But the dead `MetricSuffix string` config field plus the
configuration plumbing left behind made the code look like a suffix
SHOULD be applied. This PR removes the dead config field across
DDSketch / KLL / HLL, deletes the only remaining application site
(the KLL fallback `transmit_sketch=false` quantile-CDF path that
production never enables), and strips `metric_suffix:` lines from
every agent + gateway YAML in `deploy/mvp-{singlenode,multinode}/`.
Code touched:
* `ddsketchprocessor/{config,shim_helpers,processor_test}.go` —
field + 12 test references gone
* `kllprocessor/{config,factory,encode,processor_test}.go` —
field + fallback application + test references gone
* `hllprocessor/{config,factory,processor_test}.go` —
field + dedicated TestBatchModeMetricSuffix test gone
* Stale doc table in `processor/ddsketchprocessor/README.md`
* 13 agent yamls + 2 gateway yamls: `metric_suffix:` lines stripped
* README + inline-comment updates calling out the refactor
Build verification:
* `bash build_asap_otel.sh --skip-patches` — full OCB build of the
asap-otel binary succeeds with the patched processors compiled
in (binary written to opentelemetry-collector-contrib-patch/cmd/asap-otel/asap-otel)
* `go build ./...` per-processor passes
* `go test ./...` per-processor has pre-existing failures from the
SAME 2026-05 proto refactor (tests still reference removed wire
fields `dp.Count`, `dp.Cardinality`, `dp.Precision` etc) —
verified unchanged on clean main; out of scope for this PR.
A few yaml comment lines still mention `metric_suffix` historically
(README-allsketches-demo.md, backend-inference-kll.yaml,
asap-otel-agent-allsketches.yaml, backend-inference.yaml) — they're
inline comments documenting the previous behavior, harmless at runtime.
Companion to ASAPQuery-backend #275 (`revert+retire(query): delete
dead resolve_sketch_metric_alias rewrite`) which removed the
backend-side counterpart of this same retired suffix scheme.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the audit thread on "do asapcollector processors add a metric-name suffix on the wire?". Runtime answer: no. The 2026-05 refactor moved sketch-type identification from name suffixes to the OTLP `pdata.Metric` variant tag (DDSketch / KLLSketch / HLLSketch / CountSketch / CountMinSketch), and every production-path encode site preserves the input metric name:
But the dead `MetricSuffix string` config field plus the configuration plumbing left behind made the code look like a suffix SHOULD be applied. This PR removes the dead field across DDSketch / KLL / HLL, deletes the only remaining application site (the KLL fallback `transmit_sketch=false` quantile-CDF path that production never enables), and strips `metric_suffix:` lines from every agent + gateway YAML in `deploy/mvp-{singlenode,multinode}/`.
What's removed
Net: 27 files, +45 / -123.
Build verification
Residual
A few yaml comment lines still mention `metric_suffix` historically (`README-allsketches-demo.md`, `backend-inference-kll.yaml`, `asap-otel-agent-allsketches.yaml`, `backend-inference.yaml`) — they're inline comments documenting the previous behavior, harmless at runtime.
Related
Companion to ASAPQuery-backend #275 (`revert+retire(query): delete dead resolve_sketch_metric_alias rewrite`) which removed the backend-side counterpart of this same retired suffix scheme.
🤖 Generated with Claude Code