[mongodb_atlas] Fix CEL state wipe and missing group_id on error paths - #20226
Conversation
✅ Elastic Docs Style Checker (Vale)No issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
There was a problem hiding this comment.
Pull request overview
This PR fixes state-handling bugs in the MongoDB Atlas integration’s CEL-based data streams that could wipe required state keys (notably group_id) and cause subsequent evaluations to crash, and it adds dedicated regression/system test coverage plus an updated mock Atlas server to reliably exercise the previously broken paths.
Changes:
- Replace
{}terminal returns with explicit reset states that always includeeventsand preserve required state keys acrossprocess,disk,hardware,mongod_database, andmongod_audit. - Preserve
group_id(and related pagination keys) on non-200 measurement error paths to prevent follow-on evaluation failures. - Add Go-based CEL regression tests and refresh the Docker mock server + system test configs to reproduce the bugs deterministically.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mongodb_atlas/manifest.yml | Bumps integration version to 1.3.1. |
| packages/mongodb_atlas/changelog.yml | Documents the bugfix release and links the tracked issue. |
| packages/mongodb_atlas/data_stream/process/agent/stream/input.yml.hbs | Ensures terminal/error states preserve events and required keys (incl. group_id). |
| packages/mongodb_atlas/data_stream/hardware/agent/stream/input.yml.hbs | Same state preservation fixes for the hardware CEL program. |
| packages/mongodb_atlas/data_stream/disk/agent/stream/input.yml.hbs | Fixes terminal state wipes and improves host/disk cursor progression behavior. |
| packages/mongodb_atlas/data_stream/mongod_database/agent/stream/input.yml.hbs | Replaces {} terminal return with explicit reset state for log stream. |
| packages/mongodb_atlas/data_stream/mongod_audit/agent/stream/input.yml.hbs | Same terminal reset-state change for audit log stream. |
| packages/mongodb_atlas/data_stream/process/_dev/test/system/test-empty-processes-config.yml | Adds system test config to exercise empty-process list path. |
| packages/mongodb_atlas/data_stream/process/_dev/test/system/test-non200-meas-config.yml | Adds system test config to exercise non-200 measurement responses. |
| packages/mongodb_atlas/data_stream/hardware/_dev/test/system/test-empty-processes-config.yml | Adds empty-processes system test config for hardware stream. |
| packages/mongodb_atlas/data_stream/hardware/_dev/test/system/test-non200-meas-config.yml | Adds non-200 measurement system test config for hardware stream. |
| packages/mongodb_atlas/data_stream/disk/_dev/test/system/test-empty-processes-config.yml | Adds empty-processes system test config for disk stream. |
| packages/mongodb_atlas/data_stream/disk/_dev/test/system/test-non200-meas-config.yml | Adds non-200 measurement system test config for disk stream. |
| packages/mongodb_atlas/data_stream/mongod_database/_dev/test/system/test-empty-processes-config.yml | Adds empty-processes system test config for mongod_database stream. |
| packages/mongodb_atlas/data_stream/mongod_audit/_dev/test/system/test-empty-processes-config.yml | Adds empty-processes system test config for mongod_audit stream. |
| packages/mongodb_atlas/_dev/scripts/go.mod | Introduces a Go module for CEL regression testing scripts. |
| packages/mongodb_atlas/_dev/scripts/cel_eval_test.go | Adds a Go test suite that runs mito against an in-process mock server and asserts state invariants. |
| packages/mongodb_atlas/_dev/scripts/mito-config.yaml | Adds local mito configuration documentation for running CEL programs manually. |
| packages/mongodb_atlas/_dev/scripts/process/process.cel | Adds a checked-in CEL program copy used by the regression tests. |
| packages/mongodb_atlas/_dev/scripts/process/state-normal.json | Adds baseline state fixture for process scenarios. |
| packages/mongodb_atlas/_dev/scripts/process/state-empty-processes.json | Adds empty-processes state fixture for process scenario. |
| packages/mongodb_atlas/_dev/scripts/process/state-non200-meas.json | Adds non-200 measurement state fixture for process scenario. |
| packages/mongodb_atlas/_dev/scripts/hardware/hardware.cel | Adds a checked-in CEL program copy used by the regression tests. |
| packages/mongodb_atlas/_dev/scripts/hardware/state-normal.json | Adds baseline state fixture for hardware scenarios. |
| packages/mongodb_atlas/_dev/scripts/hardware/state-empty-processes.json | Adds empty-processes state fixture for hardware scenario. |
| packages/mongodb_atlas/_dev/scripts/hardware/state-non200-meas.json | Adds non-200 measurement state fixture for hardware scenario. |
| packages/mongodb_atlas/_dev/scripts/disk/disk.cel | Adds a checked-in CEL program copy used by the regression tests. |
| packages/mongodb_atlas/_dev/scripts/disk/state-normal.json | Adds baseline state fixture for disk scenarios. |
| packages/mongodb_atlas/_dev/scripts/disk/state-empty-processes.json | Adds empty-processes state fixture for disk scenario. |
| packages/mongodb_atlas/_dev/scripts/disk/state-non200-meas.json | Adds non-200 measurement state fixture for disk scenario. |
| packages/mongodb_atlas/_dev/scripts/mongod_database/mongod_database.cel | Adds a checked-in CEL program copy used by the regression tests. |
| packages/mongodb_atlas/_dev/scripts/mongod_database/state-normal.json | Adds baseline state fixture for mongod_database scenario. |
| packages/mongodb_atlas/_dev/scripts/mongod_audit/mongod_audit.cel | Adds a checked-in CEL program copy used by the regression tests. |
| packages/mongodb_atlas/_dev/scripts/mongod_audit/state-normal.json | Adds baseline state fixture for mongod_audit scenario. |
| packages/mongodb_atlas/_dev/deploy/docker/mock_server/main.go | Rewrites the mock Atlas API server in Go and adds special group IDs to trigger error paths. |
| packages/mongodb_atlas/_dev/deploy/docker/mock_server/go.mod | Adds a Go module for the Docker mock server build. |
| packages/mongodb_atlas/_dev/deploy/docker/mock_server/.gitignore | Ignores the built mock_server binary. |
| packages/mongodb_atlas/_dev/deploy/docker/Dockerfile | Updates Docker build to compile and run the new Go-based mock server (Go 1.21). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
**WHAT:** Replace `{}` terminal returns with explicit reset states in the
`process`, `disk`, `hardware`, `mongod_database`, and `mongod_audit` CEL
programs. Preserve `group_id` and pagination keys on all non-200 measurement
error paths. Rewrite the mock Atlas API server in Go. Add system test configs
for the two previously-broken error paths.
**WHY:** Two related bugs caused integration runs to fail with
`"unexpected missing events array from evaluation"` or
`"failed eval: ERROR: no such key: group_id"`. Both share the same root
cause: state keys written by one evaluation are lost, so the next evaluation
crashes trying to read them.
Closes elastic#17927
32456f9 to
bad70ee
Compare
…ixes - mongod_database, mongod_audit: terminal branch (empty cluster list) now preserves cursor.last_timestamp so the next scheduled run continues from the last collected timestamp instead of falling back to the 30m default. - disk: disk-list terminal now checks disk_page_num before advancing to the next host. When disk_page_num != 1, more disk pages remain for the current host; the state is returned without disk_next so the next eval re-fetches the next disk page. Only when disk_page_num == 1 (all pages exhausted) is the host cursor advanced.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
packages/mongodb_atlas/data_stream/mongod_database/agent/stream/input.yml.hbs:104
- The terminal branch that runs when
state.next >= size(state.hostlist)dropscursorfrom the returned state. If the process list is empty for a period, this will wipe the persisted cursor and cause the next scheduled run to fall back to the default lookback window again (duplicating data). Preserve the existing cursor in the reset state (and keep it empty when not present).
{
"events": [],
"group_id": state.group_id,
"want_more": false,
"page_num": 1,
"cursor": {
packages/mongodb_atlas/data_stream/mongod_audit/agent/stream/input.yml.hbs:104
- The terminal branch that runs when
state.next >= size(state.hostlist)dropscursorfrom the returned state. If the process list is empty for a period, this wipes the persisted cursor and makes the next scheduled run fall back to the default lookback window again (duplicating data). Preserve the existing cursor in the reset state (or leave it empty when not present).
{
"events": [],
"group_id": state.group_id,
"want_more": false,
"page_num": 1,
"cursor": {
…ing for events
The test-empty-processes-config system tests configure group_id=empty-processes-group,
which causes the mock server to return an empty process list. The CEL program
correctly returns {"events":[],...} with no events. elastic-package system tests
wait up to 10 minutes for events that never arrive, causing CI to fail.
The empty-processes bug fix (CEL no longer returns {} on terminal branch) is
verified by the CEL code itself and by the non200-meas system tests that do
produce events.
The disk-list terminal branch used a nested conditional in the true-branch of
a ternary without parentheses:
A ? B ? C : D : E
CEL's grammar defines the true-branch as a ConditionalOr, not a full ternary,
so this is a syntax error and the program fails to compile at runtime in the
Filebeat CEL input — collecting zero disk events. elastic-package
build/check/lint do not compile the CEL program, so it was not caught
statically. Wrapping the inner ternary in parentheses makes it valid:
A ? (B ? C : D) : E
Verified by compiling all five modified programs with cel-go and by running
the disk program end-to-end against a live Atlas API (happy path and the
non-200 measurement error path).
…hapes Update the mock server so its response bodies are structurally identical to real Atlas API responses, captured by querying a live Atlas cluster. Key changes: - Process list: add groupId, created, lastPing, userAlias, replicaSetName, version per result; use FQDN-style hostnames - Process measurements: add groupId, hostId; granularity "PT10M" (uppercase, no period field); add units field per measurement dataPoint - Disk list: partitionName "xvdf" → "data" (real Atlas uses "data") - Disk measurements: add groupId, hostId, granularity "PT10M"; remove start/end (real Atlas disk measurements omit these); add units - Hardware measurements: add groupId, granularity "PT10M"; add units; no hostId (hardware pipeline maps groupId only) - dataPoint helper: add units parameter to all call sites
stefans-elastic
left a comment
There was a problem hiding this comment.
Just a small comment (and CI failure needs resolving)
handleAlerts and handleEvents both returned empty results arrays, causing the alert, organization, and project data stream system tests to wait the full 10-minute timeout with zero events — the same failure mode as the empty-processes scenario. Return one synthetic record each so the agent collects at least one event per interval and system tests reach the validation step.
elastic-package's validateFields rejects any document containing error.message, so system tests that intentionally produce error events (non-200 responses) can never pass. Same constraint already led to removing empty-processes tests. The CEL fix for state preservation (group_id) is verified by code review and manual testing against a live Atlas API.
stefans-elastic
left a comment
There was a problem hiding this comment.
Code Review
The core state-preservation fixes and the nested-ternary CEL syntax catch are correct and well-reasoned. The disk stream's two-level pagination terminal (advance host vs. stay for more disk pages, gated on disk_page_num) is sound on trace-through, and the CEL grammar analysis (ternary true-branch is a ConditionalOr, so the inner ternary needs parens) is accurate — a real latent bug that no static check surfaces.
A few things to address before merge:
🔴 The fix is incomplete: process-list / disk-list non-200 branches still drop group_id
This PR fixes the measurement (second/third do_request) error branches, but the process/host-list error branch — the first do_request in every stream — still returns bare {events, want_more: false} with no group_id:
process/…/input.yml.hbs(process-list error branch)hardware/…/input.yml.hbs(process-list error branch)disk/…/input.yml.hbs(process-list and disk-list error branches)mongod_database/…/input.yml.hbsandmongod_audit/…/input.yml.hbs(process-list error branch)
This is the identical bug class the PR sets out to fix. By the PR's own state-persistence model, on a non-200 from /processes:
Eval N: /processes → 503 → error branch → {events, want_more:false} ← group_id dropped
outer .as(state,…): !has(state.next) → returns state unchanged
Next interval: {url} re-injected only → build /processes URL with state.group_id
→ ERROR: no such key: group_id
Tellingly, the new mock server explicitly defines non200-processes-group (main.go → handleProcessList) to return HTTP 503 on /processes — but that path lands in the unfixed branch and reproduces the exact crash the PR is about. Recommend preserving group_id/page_num/query (and cursor for mongod_*) in these first-level error branches too, so the fix is complete and consistent.
🟠 Old ~7.9 MB mock binary not removed
The description says "Replace the committed mock-server binary with Go source," but _dev/deploy/docker/mongodb_atlas/test (7,925,733 bytes) is still tracked. The Dockerfile's CMD now points at /mock_server, so this binary is dead weight — and it's still COPY'd into /data in the image. It should be git rm'd. (The two .log fixtures in that dir are still needed by handleLog and should stay.)
🟠 Description & Author's Checklist claim test coverage that no longer exists
Both the body and the checklist assert Bug 2 is "covered by test-non200-meas-config.yml system tests for disk, hardware, and process." No such files exist in the PR, and the latest commit on the branch is remove non200-meas system tests that always fail. Net effect: neither bug has automated CI coverage — Bug 1 (empty list) times out elastic-package, and the Bug 2 tests were removed. The mock server supports the scenarios, but nothing exercises them. Please reconcile the description with reality, and consider whether a pipeline test on the error-event document (or a non-timing-out system assertion) can lock in Bug 2's fix.
Minor
- Mock error shape
{"error": code, "detail": detail}correctly matches the CEL branches readingbody.error/body.detail. 👍 handleLogsynthetic-fallback line is a nice robustness touch.- Digest auth checks only scheme + username substring — appropriate for a mock, and the comment says so.
changelog.yml/manifest.ymlbump to1.3.1(bugfix, patch) is correct.
The mock server is now built from Go source at build time (Dockerfile). The old x86-64 Linux binary committed at _dev/deploy/docker/mongodb_atlas/test is dead weight — it is no longer referenced and was still being COPY'd into the Docker image.
🟠 Binary and description — fixedAgreed on both.
🔴 Process-list / disk-list error branches — pushback
These branches are structured differently from the measurement error branches — they are wrapped in Measurement error branch — result bound directly in the outer Process-list error branch — result is the argument to
The step "group_id dropped" is where the trace diverges. Because the error branch is inside
Ran this path with {
"events": [{"error": {"code": "SERVICE_UNAVAILABLE", "message": "mock: process list unavailable"}}],
"group_id": "non200-processes-group",
"page_num": 1,
"query": "/measurements?granularity=PT1M&period=PT1M",
"want_more": false
}Eval N+1 using that state as input produces identical output — Same result for disk ( Why the measurement fix was necessary but the process-list branches aren't
func withAll(dst, src ref.Val) ref.Val {
new, other, err := with(dst, src) // new = full copy of dst
for k, v := range other {
new[k] = v // src keys overlay; dst-only keys survive
}
return types.NewRefValMap(...)
}The measurement error branch sits outside any {"events": [{"error": {"code": "SERVICE_UNAVAILABLE", "message": "mock: measurement unavailable"}}], "want_more": false}
|
|
✅ All changelog entries have the correct PR link. |
🚀 Benchmarks reportTo see the full report comment with |
💚 Build Succeeded
History
cc @shmsr |
|
Tick the box to add this pull request to the merge queue (same as
|
|
Package mongodb_atlas - 1.3.1 containing this change is available at https://epr.elastic.co/package/mongodb_atlas/1.3.1/ |
Proposed commit message
WHAT: Replace
{}terminal returns with explicit reset states in theprocess,disk,hardware,mongod_database, andmongod_auditCELprograms. Preserve
group_idand pagination keys on all non-200 measurementerror paths. Fix an invalid nested ternary in the
diskterminal (a CELsyntax error that no static check catches). Replace the committed mock-server
binary with Go source. Remove old pre-compiled mock binary.
WHY: Two related bugs caused integration runs to fail with
"unexpected missing events array from evaluation"or"failed eval: ERROR: no such key: group_id". Both share the same root cause:state keys written by one evaluation are lost, so the next evaluation crashes
trying to read them.
How the CEL input loop works
The Filebeat CEL input runs the program in a loop. Each evaluation receives
the previous return value as
state, plus one always-re-injected key:state.url(fromresource.url). Every other key —group_id,page_num,query, pagination cursors — must be present in the program's return valueto survive into the next evaluation. Keys absent from the return are gone.
Bug 1 — empty process list wipes all state
Trigger: Atlas
/processesreturns an empty results array (valid, not anerror — the project has no monitored processes).
Root cause: The terminal branch (
state.next >= size(state.hostlist))returned
{}.Fix: terminal branch returns all required keys:
Bug 2 — non-200 measurement response drops
group_idTrigger: Atlas measurement endpoint returns a non-200 (temporary throttle,
unavailability, auth expiry).
Root cause: The error branch returned only
{events, want_more}.Fix: error branch preserves all required keys:
Additional fixes
diskstream — nested pagination terminal: The disk stream has twopagination levels (host list → disk partitions per host). The disk-partition
terminal (
state.disk_next >= size(state.disk_list)) originally returned{},wiping state. The fix preserves all keys and correctly handles multi-page disk
lists: if
disk_page_num != 1there are more partition pages for the currenthost, so the host cursor must not advance yet.
mongod_database/mongod_audit— terminal branch drops cursor: Whenthe cluster list is empty the terminal branch was missing
cursor.last_timestamp, so the next scheduled run fell back to the 30-minutedefault lookback window instead of continuing from the last timestamp. Fixed by
preserving
cursor.last_timestamp: state.endDate.diskstream — invalid nested ternary (syntax): Thediskterminal useda nested conditional in the true-branch of a ternary without parentheses
(
A ? B ? C : D : E). CEL's grammar defines a ternary's true-branch as aConditionalOr, not a fullExpr, so this is a syntax error — the programfails to compile at runtime in the Filebeat CEL input and collects zero disk
events.
elastic-package build/check/lintdo not compile the CELprogram, so it passed every static check. Fixed by wrapping the inner ternary
in parentheses:
A ? (B ? C : D) : E. All five programs now compile withcel-go (see the live-Atlas section below).
End-to-end data flow for each test scenario
All scenarios use the same mock server
(
_dev/deploy/docker/mock_server/main.go) with Digest auth (useradmin,password
MongoDB@123). Group-ID routing selects the response path.Scenario A — happy path (
test-default-config.yml,group_id: mongodb-group1)Step 1 — CEL fetches process list:
Step 2 — CEL fetches measurements for
hostname-1:27017:Step 3 — CEL
zip(names, values)flattens measurements; returns to Filebeat:{ "events": [{ "processId": "hostname-1:27017", "granularity": "PT1M", "response": { "CONNECTIONS": 38.0, "ASSERT_REGULAR": 0.332, "PROCESS_CPU_USER": 1.07, "PROCESS_CPU_KERNEL": 0.237, "PROCESS_NORMALIZED_CPU_USER": 0.654, "PROCESS_NORMALIZED_CPU_KERNEL": 0.073 } }], "hostlist": [], "next": 0, "want_more": false, "page_num": 1, "group_id": "mongodb-group1", "query": "…" }Step 4 — Ingest pipeline maps fields to
mongodb_atlas.*ECS fields.Pipeline test input/output for the full happy-path field set:
test-process-metrics.jsontest-process-metrics.json-expected.jsonScenario B — non-200 measurement (
test-non200-meas-config.yml,group_id: non200-meas-group)Step 1 — CEL fetches process list: same as Scenario A (mock returns
hostname-1:27017for any group other thanempty-processes-group/non200-processes-group).Step 2 — CEL fetches measurements → mock returns HTTP 503:
Step 3 — CEL error branch returns (after fix):
{ "events": [{"error": {"code": "SERVICE_UNAVAILABLE", "message": "mock: measurements unavailable"}}], "want_more": false, "group_id": "non200-meas-group", "page_num": 1, "query": "/measurements?granularity=PT10m&period=PT10m" }Step 4 — Ingest pipeline receives
{"error": {"code": "…", "message": "…"}}.The
set event.kind: pipeline_error if ctx.error?.message != nullprocessorfires. Indexed document:
{ "@timestamp": "…", "ecs": {"version": "8.11.0"}, "event": {"kind": "pipeline_error", "module": "mongodb_atlas", "category": ["process"], "type": ["info"]}, "error": {"code": "SERVICE_UNAVAILABLE", "message": "mock: measurements unavailable"} }Note: pipeline tests for error events cannot exist — elastic-package treats any
error.messagein a pipeline test output as a test failure by design. Theerror-path document format is validated end-to-end by the system test instead.
error.code(ECSkeyword) anderror.message(ECSmatch_only_text) comefrom the imported ECS v8.11.0 definitions, so the document passes field
validation. This system test passes locally today.
Step 5 — Next eval input still carries
group_id: "non200-meas-group",so the process-list fetch on the following interval succeeds instead of
crashing with
"no such key: group_id"✓.Scenario C — empty process list (Bug 1, local testing only)
group_id: empty-processes-groupis supported by the mock server for localverification. CI system tests cannot cover zero-event scenarios because
elastic-package waits for events and times out. The fix (explicit terminal
return with all keys) is verified by inspection of the CEL code.
CEL terminal branch returns (after fix):
{ "events": [], "group_id": "empty-processes-group", "want_more": false, "page_num": 1, "query": "…" }To test locally: configure a policy with
groupId: empty-processes-groupandobserve that the agent collects zero events per interval without crashing.
Mock server
mongodb-group1(default)non200-meas-group/processesnormal; measurements return HTTP 503empty-processes-group/processesreturnsresults: []non200-processes-group/processesreturns HTTP 503Before this PR the mock server was a pre-compiled x86-64 Linux binary committed
at
_dev/deploy/docker/mongodb_atlas/testwith no source. Routing washard-coded and could not be extended for the new error-path scenarios. The
Dockerfile now builds from Go source (
_dev/deploy/docker/mock_server/main.go)so the routing is readable, testable, and extensible.
Verified against a live Atlas API
Beyond the mock, every modified program was run unmodified against a real
MongoDB Atlas project (a live 3-node replica set) to confirm the fix on real
response shapes. Because
mito(the CEL evaluator used to replicate theFilebeat CEL input loop) has no Digest-auth support, the program's requests
were sent to a tiny local proxy that re-issued them to
cloud.mongodb.comwith Digest auth and forwarded the responses verbatim. The Filebeat loop
semantics were reproduced exactly: publish
result.events, retain the rest asnext state, re-inject
url, repeat whilewant_more; then one extraevaluation to prove the retained state survives the next interval (this is the
eval that used to crash). Error paths (non-200 measurement, empty process list)
were reproduced by toggling fault injection in the proxy while the process list
stayed real.
Result for all five streams (
process,disk,hardware,mongod_database,mongod_audit):processdiskhardwaremongod_databasemongod_auditOn the old code the error paths reproduce the exact runtime failure from the
issue —
failed eval: ERROR: no such key: group_id(andno such key: page_numfor themongod_*streams) on the evaluation that follows the error.The fixed code preserves
group_id/page_num/cursorand the loop continuescleanly.
This live run is also what caught the
disknested-ternary syntax bugdescribed under Additional fixes above — the program failed to compile in
cel-go, which no static check surfaces. All five programs were then confirmed
to compile cleanly with cel-go.
Checklist
changelog.ymlfile.Author's Checklist
elastic-package lintandelastic-package buildpasstest-non200-meas-config.ymlsystem tests were removed (always failed CI){}terminal) verified by CEL code review and local mock-server testing; no CI system test possible (zero-event scenario times out elastic-package)group_id/page_num/cursorand continues cleanlyHow to test
Pipeline tests (no stack required — covers happy-path ingest pipeline):
System tests (requires
elastic-package stack up -d --version 9.x.x):To reproduce Bug 1 locally, run a system test with
groupId: empty-processes-group(or configure the policy in Kibana with that value). The agent should collect
zero events per interval without logging
"no such key: group_id".Related issues
Screenshots
N/A — no UI changes.