m365_defender: add agentless worst-case memory harness and ORR coverage - #20405
Conversation
Add a self-contained Docker + cgroup harness under _dev/scripts/memcap-agent/ to measure per-page peak memory for the three agentless data streams (alert, incident, vulnerability) as part of the agentless Operational Readiness Review. Unlike the o365 harness (a single CEL content blob), the m365_defender memory driver is a single decoded API page, and its shape differs per stream: alert is records times per-alert size (server caps $top at 1000), incident is incidents times alerts-per-incident via the nested body.alerts split with keep_parent, and vulnerability is a fixed pageSize=10000 page re-encoded by CEL. Each stream serves one worst-case page from an elastic/stream mock to a real elastic-agent under a Docker memory cap and reports the cgroup memory.peak. run.sh takes STREAM=alert|incident|vulnerability; sweep.sh fits memory.peak = base + k*page and derives the largest page that fits at 1Gi and 512Mi. The cel program is spliced from the shipping cel.yml.hbs at run time so it never drifts; the httpjson configs are hand-rendered copies of their .hbs templates.
Elastic Docs Style Checker (Vale)Summary: 3 suggestions found 💡 Suggestions (3): Optional style improvements. Apply when helpful.
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. |
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
incident |
1137.66 | 856.53 | -281.13 (-24.71%) | 💔 |
vulnerability |
10989.01 | 6734.01 | -4255 (-38.72%) | 💔 |
To see the full report comment with /test benchmark fullreport
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
| memory: 4Gi # Due to the large volume of data being processed in memory, a 4 GB allocation is required for agentless deployment-anything less may lead to out-of-memory (OOM) issue. | ||
| # Peak memory is set by the largest single decoded API page, not by dataset size. | ||
| # Measured with _dev/scripts/memcap-agent (all three agentless streams in one agent, | ||
| # polling back-to-back): 1,516 MB with 400 alerts per incident, which reproduces the |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: high path: packages/m365_defender/manifest.yml:34
The agentless sizing comment compares the harness memory.peak (1,516 MB) against a production working-set peak (1,507 MB) - two different quantities. Quote the harness working set (1,132 MB) instead, or say explicitly that the numbers being compared are peak vs working set.
Details
1,516 MB is the peak_bytes column of the 400-alerts/incident row in _dev/scripts/memcap-agent/results/sustained.csv (1,589,641,216 B = 1,516 MiB). The workingset_bytes column of the same row is 1,186,988,032 B = 1,132 MiB. The harness itself documents that these are not interchangeable: bin/lib.sh says "Production ORR telemetry uses the working-set metric, so comparing it against a raw memory.peak from this harness would compare two different quantities", and the harness README (line 255) says of working set "This is the column to publish; memory.peak additionally includes page cache". So comparing 1,516 MB against a 1,507 MB production working-set peak is exactly the comparison the harness warns against; the like-for-like harness figure is 1,132 MB, which is ~25% below the production peak rather than within 0.6% of it.
This matters because the comment is the recorded justification for the agentless memory request and for the proposed 4Gi -> 2Gi reduction, and the same comparison is made in the harness README at line 363 ("The second row lands within 0.6% of the worst working set ever observed in production"), where it supports the stronger claim that "the production peaks are fully explained ... No unaccounted mechanism is needed". If the working-set columns are the correct basis, that conclusion does not follow from the data as committed. memory: 4Gi is unchanged, so nothing regresses today - but the next PR that acts on this comment would be sizing from a mismatched pair.
Recommendation:
State which metric each number is, and cite the working-set figure alongside the peak so the peak-vs-working-set gap is visible rather than implied. For example:
resources:
requests:
# Peak memory is set by the largest single decoded API page, not by dataset size.
# Measured with _dev/scripts/memcap-agent (all three agentless streams in one agent,
# polling back-to-back) at 400 alerts per incident: working set 1,132 MB, cgroup
# memory.peak 1,516 MB. Production's worst observed working set is 1,507 MB, so the
# harness does not yet reproduce it on the like-for-like (working-set) metric.
# The request stays at 4Gi until the agentless ORR signs off a reduction:
# alerts-per-incident is unbounded (~1.2 MB of pod memory per alert with
# include_alerts=true) and the 2Gi OOM boundary sits near 590 alerts per incident.
memory: 4GiApply the same correction to the sentence at _dev/scripts/memcap-agent/README.md:363.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| # Wait for exactly 3 alerts. The mock returns the same 3 alerts on every poll, | ||
| # so -confirm proves the fingerprint `_id` de-duplicates repeated deliveries | ||
| # rather than inflating the count. | ||
| get_docs -profile ${CONFIG_PROFILES}/${PROFILE} -want 3 -confirm 15s -timeout 5m ${DATA_STREAM_NAME} |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/m365_defender/data_stream/alert/_dev/test/scripts/agent_replacement.txt:55
In the alert agent-replacement test the 15s confirm window is shorter than the 30s poll interval, so the de-duplication behaviour the comment says it proves is not actually exercised. Widen the confirm window or shorten the interval.
Details
The comment above this line states that -confirm proves the fingerprint _id de-duplicates repeated deliveries. That only holds if at least one further poll lands inside the confirm window. test_config.yaml in this same file sets interval: 30s (line 109), but the confirm window is 15s, so once the first poll has produced the 3 documents the window can close before the second poll ever runs, and the assertion degrades to "the count did not change while nothing happened".
The incident test added in this same PR gets this right and says so explicitly: it uses interval: 10s with -confirm 25s and notes that this "spans at least two further polls". The alert test looks like it inherited the 15s window from the o365 precedent, where the test only claims no-loss and makes no de-duplication claim. The same mismatch applies to the phase 2 check on line 81, though the de-dup claim is only made for phase 1.
Recommendation:
Either match the incident test's timing (preferred - it keeps the test short) or widen the window so it spans at least two polls at the current interval:
# option A - mirror the incident test, in test_config.yaml:
data_stream:
vars:
interval: 10s
# and keep the assertion at:
# get_docs ... -want 3 -confirm 25s -timeout 5m ${DATA_STREAM_NAME}# option B - keep interval: 30s and widen the window instead:
get_docs -profile ${CONFIG_PROFILES}/${PROFILE} -want 3 -confirm 65s -timeout 5m ${DATA_STREAM_NAME}
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
|
|
||
| # The fit and the published tables come from one implementation, in the agentless-orr | ||
| # skill, so a number in a document and a number on this terminal cannot disagree. | ||
| RENDERER="${ORR_RENDERER:-$HOME/.cursor/skills/agentless-orr/scripts/harness_autofill.sh}" |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/m365_defender/_dev/scripts/memcap-agent/bin/sweep.sh:160
The sweep's fit/render step defaults to a per-user Cursor path for a script that is not in this repo, so the harness README's staleness guard cannot be run by another maintainer. Document how to obtain the renderer, or point ORR_RENDERER at something checked in.
Details
The default resolves to $HOME/.cursor/skills/agentless-orr/scripts/harness_autofill.sh, which is a personal, editor-specific install path. harness_autofill.sh is not present anywhere in this repository (the only checked-in skills are under .agents/skills/, and none is agentless-orr), so on any other machine this branch silently falls through to the "run the renderer yourself" message.
That undercuts a guarantee the committed README makes about itself: it says the AUTOFILL tables "are rendered from results/*.csv", that "a re-run cannot leave a document behind, and two documents cannot disagree", and it documents harness_autofill.sh --check README.md (README.md:219) as the way to detect a stale block. As committed, no reviewer or future maintainer of this package can run that check or re-render the tables after promoting a new CSV - the numbers become hand-maintained in practice, which is the exact failure mode the mechanism was added to prevent.
Recommendation:
Say in the harness README where harness_autofill.sh comes from and how to install it, and make the default path editor-neutral so ORR_RENDERER is the documented knob rather than a fallback nobody hits:
# The renderer ships with the agentless-orr skill; see README.md, "Publishing a number",
# for where to obtain it. Override with ORR_RENDERER=/path/to/harness_autofill.sh.
RENDERER="${ORR_RENDERER:-}"
if [ -n "$RENDERER" ] && [ -x "$RENDERER" ]; then
echo
"$RENDERER" --fit "$CSV" || true
else
echo
echo "Fit: set ORR_RENDERER to the agentless-orr renderer and re-run, or run it manually:"
echo " harness_autofill.sh --fit $CSV"
fi🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| echo " SWEEP SUMMARY" | ||
| echo "==================================================================" | ||
| printf ' %-10s %-11s %-10s %-10s %-6s %s\n' point raw_MB peak_MB ws_MB oom label | ||
| awk -F, 'NR>1{printf " %-10s %-11.1f %-10.1f %-10.1f %-6s %s\n", $2,$10/1048576,$11/1048576,$12/1048576,$14,$3}' "$CSV" |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: high path: packages/m365_defender/_dev/scripts/memcap-agent/bin/sweep.sh:156
The SWEEP SUMMARY table prints anon_bytes under its oom heading because the awk field index is off by one; use $15 (oom) instead of $14 (anon_bytes).
Details
HARNESS_CSV_HEADER in bin/lib.sh defines 17 columns: axis(1), point(2), label(3), streams(4), params(5), mode(6), mem_limit_bytes(7), gomemlimit(8), hold_s(9), total_raw_bytes(10), peak_bytes(11), workingset_bytes(12), workingset_peak_bytes(13), anon_bytes(14), oom(15), agent_image(16), run_log(17). The header printf on line 155 labels the fifth output column oom, but the awk on line 156 reads $14, which is anon_bytes. Every summary row therefore shows a byte count (or 0, as in the committed results/knobs.csv and results/sustained.csv rows) where the OOM flag should be, and a run that actually OOM'd is indistinguishable from one that did not. That matters here: README.md states that an OOM run has memory.peak pinned at the cap and must be excluded from the fit and re-run at a larger SWEEP_CAP, and the summary table is the console signal an operator uses to notice it. A pinned-at-cap peak read as a real peak would be promoted into results/ and published.
Recommendation:
Read the oom column ($15) rather than anon_bytes ($14) so the summary's fifth column matches its header.
| awk -F, 'NR>1{printf " %-10s %-11.1f %-10.1f %-10.1f %-6s %s\n", $2,$10/1048576,$11/1048576,$12/1048576,$14,$3}' "$CSV" | |
| awk -F, 'NR>1{printf " %-10s %-11.1f %-10.1f %-10.1f %-6s %s\n", $2,$10/1048576,$11/1048576,$12/1048576,$15,$3}' "$CSV" |
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
efd6
left a comment
There was a problem hiding this comment.
The commit message refers to an autofill.sh, but this does not exist. Can you clarify that?
There was a problem hiding this comment.
Pull request overview
This PR prepares the m365_defender integration’s agentless deployment for Operational Readiness Review (ORR) by adding an internal worst-case memory measurement harness, adding checkpoint/resume (“agent replacement”) script tests for key streams, and expanding operator troubleshooting documentation. It also bumps the package version to 5.16.0 and records the change in changelog.yml.
Changes:
- Add
_dev/scripts/memcap-agentharness to measure worst-case memory usage under constrained cgroups and persist sweep results as CSVs. - Add
agent_replacementscript tests foralert,incident, andvulnerabilitystreams to validate resume behavior and de-duplication expectations. - Expand README troubleshooting guidance and clarify that only the Vulnerabilities data stream ships a 7-day ILM policy.
Reviewed changes
Copilot reviewed 29 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/m365_defender/manifest.yml | Bump package version and expand agentless memory request rationale/comments. |
| packages/m365_defender/docs/README.md | Clarify ILM scope and add troubleshooting scenarios (tracing, auth, recovery, throttling, etc.). |
| packages/m365_defender/_dev/build/docs/README.md | Regenerated built docs matching docs/README.md updates. |
| packages/m365_defender/data_stream/alert/_dev/test/scripts/agent_replacement.txt | New script test validating alert stream idempotency and replacement behavior. |
| packages/m365_defender/data_stream/incident/_dev/test/scripts/agent_replacement.txt | New script test validating incident+alerts fan-out stability and replacement behavior. |
| packages/m365_defender/data_stream/vulnerability/_dev/test/scripts/agent_replacement.txt | New script test validating nextLink paging completion and replacement no-loss expectations. |
| packages/m365_defender/changelog.yml | Add 5.16.0 changelog entry referencing troubleshooting doc expansion. |
| packages/m365_defender/_dev/scripts/memcap-agent/README.md | Document the memory harness design, usage, and interpretation for ORR. |
| packages/m365_defender/_dev/scripts/memcap-agent/.gitignore | Ignore harness scratch outputs (logs/, work/, etc.). |
| packages/m365_defender/_dev/scripts/memcap-agent/bin/run.sh | Runner for a single capped-memory harness measurement (cold/sustained). |
| packages/m365_defender/_dev/scripts/memcap-agent/bin/sweep.sh | Sweep automation to generate curve/knob measurements across axes. |
| packages/m365_defender/_dev/scripts/memcap-agent/bin/lib.sh | Shared corpus generation, config splicing, memory accounting, CSV emission. |
| packages/m365_defender/_dev/scripts/memcap-agent/bin/fake-es.py | Minimal bulk-accepting sink used to drain output in sustained runs. |
| packages/m365_defender/_dev/scripts/memcap-agent/results/scale.csv | Committed sweep results for page-size scaling axis. |
| packages/m365_defender/_dev/scripts/memcap-agent/results/alerts-per-incident.csv | Committed sweep results for alerts-per-incident axis. |
| packages/m365_defender/_dev/scripts/memcap-agent/results/sustained.csv | Committed sustained-mode reference measurements (production-peak reproduction). |
| packages/m365_defender/_dev/scripts/memcap-agent/results/knobs.csv | Committed “knob ladder” results exploring mitigation configurations. |
| packages/m365_defender/_dev/scripts/memcap-agent/results/per-stream.csv | Committed per-stream attribution sweeps (not additive sizing). |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/alert/mock-config.yml | elastic/stream mock config serving alert corpus for harness. |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/alert/elastic-agent.yml.tmpl | Standalone agent template for alert harness (ship-config splice placeholder). |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/alert/corpus/template.ndjson | Alert synthetic record template calibrated for worst-case memory. |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/alert/corpus/fields.yml | Alert corpus field schema for generator. |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/alert/corpus/config.yml | Alert corpus generation tuning (cardinality/enums/ranges). |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/incident/mock-config.yml | elastic/stream mock config serving incident corpus for harness. |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/incident/elastic-agent.yml.tmpl | Standalone agent template for incident harness (ship-config splice placeholder). |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/incident/corpus/template.ndjson | Incident-with-expanded-alerts synthetic record template (alerts-per-incident parameter). |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/incident/corpus/fields.yml | Incident corpus field schema (including embedded alert fields). |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/incident/corpus/config.yml | Incident corpus generation tuning. |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/vulnerability/mock-config.yml | elastic/stream mock config serving vulnerability corpus for harness. |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/vulnerability/elastic-agent.yml.tmpl | Standalone agent template for vulnerability harness (CEL program splice placeholder). |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/vulnerability/corpus/template.ndjson | Vulnerability synthetic record template for page-size memory worst case. |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/vulnerability/corpus/fields.yml | Vulnerability corpus field schema. |
| packages/m365_defender/_dev/scripts/memcap-agent/streams/vulnerability/corpus/config.yml | Vulnerability corpus generation tuning. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 33 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/m365_defender/docs/README.md:186
- The troubleshooting section instructs operators to “enable request tracing”, but the actual integration setting name/path isn’t stated. In this package the request tracer toggle is
enable_request_tracerand it is hidden from the UI (show_user: falsein the policy template / stream vars), so readers may not know how to turn it on. Consider explicitly naming the setting and how to enable it (e.g., via advanced YAML/policy editing for agent-based deployments), and keep the existing note that it’s not user-configurable for agentless.
packages/m365_defender/_dev/build/docs/README.md:186 - Same as docs/README.md: this section tells operators to “enable request tracing” but doesn’t name the integration setting (
enable_request_tracer) or explain how to enable it given it is hidden from the UI (show_user: false). Please clarify the exact setting name and activation method (advanced YAML/policy edit), and keep the agentless limitation note.
### Enabling request tracing
When debugging a permissions issue or unexpected API responses on the Alerts, Incidents, or Vulnerabilities data streams, enable request tracing and inspect the request trace logs to see the interaction with the server. (The Events data stream is collected over Azure Event Hub and does not offer request tracing.) OAuth2 token values can be decoded using [https://jwt.ms/](https://jwt.ms/) and should include a `roles` section listing the configured permissions.
**Security warning:** request trace files are not redacted. They contain the `Authorization` header and, during OAuth2 token exchange, the client secret in clear text. Only enable request tracing in a controlled debugging session, restrict access to the trace files, disable it as soon as you are finished, and rotate the client secret if a trace file that may contain it was exposed. On agentless deployments this setting is not user-configurable.
|
✅ All changelog entries have the correct PR link. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 33 changed files in this pull request and generated no new comments.
Suppressed comments (5)
Previously missed (5) — in code that hasn't changed since the last review.
packages/m365_defender/data_stream/alert/_dev/test/scripts/agent_replacement.txt:68
- Phase 1 installs a package policy but it is never removed before uninstalling the agent. Because add_package_policy in phase 2 overwrites DATA_STREAM_NAME, there’s no way to clean up the phase-1 policy later, which can leave stale policies in the stack and potentially interfere with subsequent script tests. Capture the phase-1 DATA_STREAM_NAME (e.g., PHASE1_DATA_STREAM_NAME) and remove it during final cleanup (along with phase 2).
# --- Simulate agent replacement (node rotation, pod eviction) ---
# Destroy the agent entirely. The new agent will have no cursor state and will
# re-collect from initial_interval.
uninstall_agent -profile ${CONFIG_PROFILES}/${PROFILE} -timeout 1m
docker_down m365d-mock
packages/m365_defender/data_stream/incident/_dev/test/scripts/agent_replacement.txt:86
- Phase 1 creates a package policy (add_package_policy) but then the agent is uninstalled without removing that policy. Since phase 2 reuses DATA_STREAM_NAME, the phase-1 data stream name is lost and can’t be cleaned up at the end, leaving stale policies in the stack. Store the phase-1 DATA_STREAM_NAME in a separate variable and remove it during final cleanup (in addition to phase 2).
# --- Simulate agent replacement (node rotation, pod eviction) ---
# Destroy the agent entirely. The new agent will have no cursor state and will
# re-collect from initial_interval.
uninstall_agent -profile ${CONFIG_PROFILES}/${PROFILE} -timeout 1m
docker_down m365d-mock
packages/m365_defender/data_stream/vulnerability/_dev/test/scripts/agent_replacement.txt:79
- Phase 1 installs a package policy but doesn’t remove it before uninstalling the agent. Because phase 2 calls add_package_policy again and overwrites DATA_STREAM_NAME, the phase-1 policy name is lost and cannot be cleaned up later, leaving stale policies in the stack. Save the phase-1 DATA_STREAM_NAME (e.g., PHASE1_DATA_STREAM_NAME) and remove it during final cleanup (along with phase 2).
packages/m365_defender/manifest.yml:50 - The comment mixes binary units (Gi) with decimal labeling (“2Gi (2,048 MB)”). 2Gi corresponds to 2,048 MiB (≈2,147 MB). To avoid confusion when readers compare these figures, consider using MiB consistently (or state explicitly that “MB” here is MiB).
packages/m365_defender/_dev/scripts/memcap-agent/README.md:436 - This section uses “MB” in formulas and comparisons alongside “Gi” caps (e.g., “2Gi (2,048 MB)”). 2Gi is 2,048 MiB (≈2,147 MB), so the unit labels are ambiguous. Consider switching these to MiB (or explicitly state that MB values are MiB) to keep the sizing math unambiguous.
At 400 alerts per incident the measured worst case is **2,135 MB**, which is *above* 2Gi
(2,048 MB) — a pod at that request would OOM, so 2Gi is off the table for this worst case.
3Gi leaves ~30% headroom and 4Gi ~48%. Getting under 2Gi with headroom to spare means
|
🟢 Reviewed the latest commits e630722 — nothing new beyond already posted comments. Review summaryIssues found across earlier commits fe10410 — 1 medium
Issues found across earlier commits 0db7b44 — 2 medium, 1 low
🤖 AI-Generated Review | Vera Review Bot - v0.2.6 | 📚 Knowledge base: integration-skills
|
💚 Build Succeeded
History
cc @kcreddy |
|
Tick the box to add this pull request to the merge queue (same as
|
|
Package m365_defender - 5.17.0 containing this change is available at https://epr.elastic.co/package/m365_defender/5.17.0/ |
Proposed commit message
Checklist
changelog.ymlfile.How to test this PR locally