ci: weekly drift check between /stats and upstream mapping files - #21001
Draft
carsonip wants to merge 11 commits into
Draft
ci: weekly drift check between /stats and upstream mapping files#21001carsonip wants to merge 11 commits into
carsonip wants to merge 11 commits into
Conversation
Adds a Monday 06:00 UTC GitHub Actions cron that:
1. Builds apm-server from main and starts it with TBS enabled, then
captures /stats from the http.enabled endpoint.
2. Sparse-clones the three upstream repos (elastic/elasticsearch,
elastic/beats, elastic/integrations) and runs the
elastic/apm-tools stats-to-mapping regen tool over the five
downstream mapping files.
3. Computes a per-file git diff. If any file changed, uploads the
full diff and the stats.json as a workflow artifact.
4. Opens a single tracking issue (labels: bug,
monitoring-metrics-drift) with a per-file change summary, the
workflow-run URL, and a copy-pasteable local reproduction
recipe. If an open issue with the monitoring-metrics-drift label
already exists, no new issue is created so a single drift event
does not produce a weekly stream of duplicates.
Workflow uses minimal permissions: contents: read for checkout, and
issues: write only on the drift-check job. workflow_dispatch is
exposed so the job can be run on demand.
Part of elastic#21000.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Consolidate on a single name across the tool, the workflow, and the issue label. The tool is "stats-to-mapping"; the workflow and the label drop the "monitoring-metric" prefix to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
🤖 GitHub commentsJust comment with:
|
The "Tracked by elastic#21000" line was misleading: elastic#21000 tracks the broader drift-detection proposal that this workflow implements. A drift issue this workflow opens is an independent occurrence of metric drift, not a sub-task of elastic#21000. Remove the cross-reference from the auto-generated issue body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
This pull request does not have a backport label. Could you fix it @carsonip? 🙏
|
…EAM_FILES UPSTREAM_FILES is now the single source of truth for the five files the workflow tracks. The sparse-checkout step groups its lines by repo into a bash associative array; the stats-to-mapping invocation prefixes each line with $WORK/upstream/ via mapfile; the issue-body recipe prefixes each line with /tmp/ for the local repro. Adding, removing, or renaming a tracked file now means editing one list instead of four. Drop the per-file cd/cd- dance in the drift loop in favor of git -C. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
carsonip
added
the
backport-skip
- Single source of truth for the apm-server config: write it to a
file in the CI run, then read it back (with path.data rewritten)
when generating the recipe in the drift issue. The CI run and the
recipe can no longer disagree about what config produced the
captured /stats.
- Split "Run apm-server, capture /stats" into "Write apm-server
config" + "Run apm-server, capture /stats". The split makes the
config the source-of-truth artifact other steps reference.
- Replace the on-disk PID file with a bash variable plus an EXIT
trap that cleans up apm-server even if curl fails.
- Hoist STATS_PORT to a job-level env so the magic 15066 lives in
one place.
- Recipe: `export PATH="$PATH:$(go env GOPATH)/bin"` after
`go install`, since contributors without that already in PATH
would otherwise see "stats-to-mapping: command not found".
- Issue title and lead paragraph now state the drifted file count
(`stats-to-mapping drift in N file(s) (date)`) so a reader
scanning the issue list grasps the scope without opening it.
Workflow output renamed `drift` -> `drifted_files` (a count) and
the conditional steps gate on != '0'.
No behavior change in what's detected or when an issue is opened.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
v1v
reviewed
Apr 28, 2026
v1v
left a comment
v1v
left a comment
Member
There was a problem hiding this comment.
Wonder if those steps should be added to a specific Makefile so it can be easy tested locally?
Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
The drift-check job's build / capture / clone / regen / diff steps
were inline shell. Move them into script/stats-to-mapping-drift.sh and
expose a top-level Makefile target stats-to-mapping-drift, so:
- Local reproduction is a single `make stats-to-mapping-drift`
invocation. Previously a contributor had to copy the recipe out
of a generated drift issue.
- The CI workflow shrinks to GitHub-Actions-specific concerns:
artifact upload, drift-issue creation. Same source of truth for
UPSTREAM_FILES, the apm-server config, and the diff format
whether the run is local or in CI.
The drift issue's reproduction recipe now points at the Makefile
target instead of inlining the apm-server config and clone commands.
Add a "Manual intervention required for new EA fields" section to
the drift issue body. The Elastic Agent integration package data
stream is TSDB; new numeric fields need a metric_type annotation
before the integrations PR can ship. The regen tool emits FIXME for
new fields (see elastic/apm-tools regen change), so the issue
explicitly tells the resolver to grep for FIXME.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The drift script previously did its own go build into $WORK; that duplicates the apm-server make target's logic (build flags, version ldflags, FIPS handling). Make the make target depend on apm-server instead, and have the script consume ./apm-server (the canonical output of `make apm-server`). One build pipeline, no second copy. The script's APM_SERVER_BIN env var still works as an override for anyone driving the script outside make. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Makefile: drop "without waiting for the next Monday cron" line
and the "depends on apm-server" sentence (the dependency itself
is documented by the prerequisite, no narration needed).
- Workflow header: drop "Background: elastic#21000".
The header should describe what this file does, not which issue
motivated it.
- Script: drop "config is reused below" comment; the recipe in the
drift issue now points at `make stats-to-mapping-drift`, not at
a literal copy of this config.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous capture loop relied on a bare `test -s "$WORK/stats.json"` post-loop check; on failure the operator saw `set -e` exit with no context. Print an explicit failure message and the last 50 lines of apm-server.log so the run that triggered the failure is debuggable from the Actions log without re-running locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mallendem
previously approved these changes
May 6, 2026
Previously the drift script ran go install ...@latest, which writes the binary into $GOPATH/bin and requires that directory be on $PATH. On a developer machine that pollutes the global Go bin dir; in CI it only worked because actions-setup-go places GOPATH/bin on PATH. Default to a shallow clone of apm-tools into $WORK/apm-tools and a go build into $WORK/stats-to-mapping. Nothing escapes $WORK. Set APM_TOOLS_DIR to point at an existing local apm-tools checkout to skip the clone — useful when iterating on the regen tool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
carsonip
marked this pull request as draft
May 6, 2026 18:10
Contributor
💚 Build Succeeded
History
|
5 tasks
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
5 participants
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.
Motivation/summary
Implements the scheduled drift-detection job proposed in #21000 — the part of that issue that catches drift between apm-server's
/statsendpoint and the five upstream mapping files when nobody has manually run the regen tool. The active-developer side of the same issue is implemented in elastic/apm-tools#245; this PR is the CI side.How and when it runs
A single new workflow at
.github/workflows/stats-to-mapping-drift.yml. Triggers:schedule: cron: '0 6 * * MON'— every Monday at 06:00 UTC.workflow_dispatch— on-demand from the Actions tab orgh workflow run.Each run, in order:
go build ./x-pack/apm-server).go install github.com/elastic/apm-tools/cmd/stats-to-mapping@latest./statsfrom thehttp.enabledendpoint, then stops apm-server. The capturedstats.jsonis the source of truth for the comparison.elastic/elasticsearch,elastic/beats,elastic/integrations) with--depth 1 --filter=blob:noneandsparse-checkout, so only the relevant subtrees materialise.git diffs each one to detect drift.UPSTREAM_FILES(job-level env) is the single source of truth for the file list: the sparse-checkout, the regen invocation, the drift loop, and the recipe in the issue body all derive from it. Adding or removing a tracked file means editing one list.The apm-server config used to capture
/statsis written to a file in the CI run and then read back (withpath.datarewritten) when generating the recipe, so the issue's repro recipe and the CI run cannot disagree about what config produced the captured/stats.What the run produces
If no file drifted, the workflow exits green with no other output.
If at least one file drifted:
drift-diff(30-day retention) — fullgit diffof every drifted file, plus the capturedstats.json.bug+stats-to-mapping-drift, body containing:<repo>/<path> — +N / -M).The issue is opened only if no open issue with the
stats-to-mapping-driftlabel already exists. A sustained drift event therefore produces one issue, not a weekly stream of duplicates.Permissions
contents: read.drift-checkjob:contents: readplusissues: write(required only to file the drift issue). No write access to the codebase, no PR creation, no other scopes.Checklist
/statsfield mappings drift across 3 upstream repos with no automated sync #21000.How to test these changes
After merge, trigger the workflow manually from the Actions tab (
workflow_dispatch). With current upstream state and a /stats capture frommain, the workflow is expected to detect drift and file one issue.Locally, run the recipe written into a generated drift issue's body to obtain the same diffs.
Related issues
/statsfield mappings drift across 3 upstream repos with no automated sync #21000.go install ...stats-to-mapping@latestuntil that PR is merged; the cron is not scheduled to fire until the next Monday after merge./statsan exhaustive enumeration of every counter apm-server defines. Without it, the workflow would compare an incomplete/statsagainst the upstream files and produce false positives.