Repository navigation
Conversation
apm-server's history of removed and renamed metrics (the Jaeger removal in apm-server#14791, agentcfg.elasticsearch counter renames, etc.) means the upstream mapping files accumulate entries the current /stats no longer reports. Replacing the apm-server / output subtree wholesale silently deleted those entries; downstream dashboards and queries that still referenced the old field names broke the next time the regen output was committed upstream. Switch both modifyMonitoring* (JSON) and upsertYAML (YAML) to a deep merge against the existing upstream subtree: - entries that only exist upstream are kept, - entries that only exist in /stats are added, - entries that exist in both recurse if they're groups, and prefer the new entry's typing if they're leaves. JSON merge runs on the parsed map[string]any tree of the existing parent[member].properties value before splicing the new bytes back. YAML merge parses the existing entry's body into a []item tree (parseEntryFields) and feeds it through mergeFields alongside the new tree from /stats; renderItem emits the merged result. Existing upstream order is preserved; new entries append after. Goldens regenerated. Includes unit tests for mergeProperties and mergeFields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
parseEntryFields broke out of the surrounding entry the moment it encountered a line indented deeper than the expected mapping-pair column. In the elastic/integrations EA file that meant a single field with a "description: >" / continuation line caused every following sibling to be silently dropped during the merge — e.g. apm_server.server (and everything beneath it) disappeared because sampling.transactions_dropped's description preceded it. Treat lines deeper than the expected sibling indent as folded-scalar or other content this parser doesn't model and skip them, rather than breaking the loop. Lines shallower than the expected indent still terminate the parse correctly. Verified by per-path comparison: every upstream field is preserved across all five regen targets after this change. Adds a focused regression test using the EA-shaped entry that triggered the bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces parseEntryFields and extractMetricTypes — both line-based scanners that depended on the upstream files' specific layout (4- column-per-level indent, "- name:" / "type:" siblings, no folded scalars across the entry, etc.) — with single yaml.Unmarshal calls into a small yamlEntry struct. Removes ~165 lines of bespoke parser code and its silent-data-loss failure modes (e.g. a "description: >" continuation line breaking out of the surrounding entry and dropping every following sibling). extractMetricTypes' dotted leaf paths now include the root entry's name as the leading component, to match the fullPath renderItem builds when emitting the merged tree. gopkg.in/yaml.v3 was already a transitive dep; promote to direct. 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
The first cut of `stats-to-mapping` replaced the `apm-server` and `output` subtrees of the upstream mapping files wholesale. apm-server's history of removed and renamed metrics — the Jaeger removal in elastic/apm-server#14791, the `agentcfg.elasticsearch` counter renames, etc. — means the upstream files have accumulated entries the current `/stats` no longer reports. A wholesale replace silently deletes those entries; downstream dashboards and queries that still reference the old field names break the next time the regen output is committed.
This PR switches the regen to a deep merge against the existing upstream subtree:
JSON merge runs on the parsed `map[string]any` of the existing `parent[member].properties` value before splicing the new bytes back. YAML merge parses the existing entry's body into a `[]item` tree (`parseEntryFields`) and feeds it through `mergeFields` alongside the new tree from `/stats`; `renderItem` emits the merged result. Existing upstream order is preserved; new entries append after.
What changes in the goldens
The regen now keeps fields like `agentcfg.elasticsearch.cache.entries.count`, `agentcfg.elasticsearch.fetch.{es,fallback,invalid,unavailable}`, and the entire `jaeger.grpc.collect.*` subtree that the previous goldens had stripped. No fields are removed; the only "deletion" lines in the diff are reorderings (existing upstream entries keep their input position, so e.g. `otlp.grpc.logs.response.errors.{count,ratelimit,timeout,unauthorized}` retain their pre-merge order while new sibling errors like `closed,decode,forbidden,…` append after).
Test plan
🤖 Generated with Claude Code