[o365] - Report audit input degraded on persistent transport failures - #20875
[o365] - Report audit input degraded on persistent transport failures#20875ShourieG wants to merge 3 commits into
Conversation
The audit data stream's CEL program wraps every do_request in
try("_transport_error") so that a single unfetchable item does not
stall collection. That handler emits a dropped retry placeholder, which
the CEL input treats as a successful evaluation and reports Healthy. A
persistent failure such as an expired client secret failing OAuth 2.0
token acquisition was therefore swallowed: collection stopped while the
integration kept reporting Healthy.
Track consecutive transport failures in state. Below the new
transport_failure_threshold the existing recover-and-continue behaviour
is unchanged, so isolated transient failures still do not degrade the
input. Once the streak reaches the threshold the failure is treated as
persistent and returned as a single-object error, marking the input
degraded until a successful request resets the streak. The threshold is
a hidden variable defaulting to 5.
Regenerate the policy test snapshots for the rendered program and the
new variable.
Updates elastic/sdh-beats#7513
Co-authored-by: Cursor <cursoragent@cursor.com>
Elastic Docs Style Checker (Vale)Summary: 1 warning found
|
| File | Line | Rule | Message |
|---|---|---|---|
| packages/o365/changelog.yml | 8 | Elastic.BritishSpellings | Use American English spelling 'behavior' instead of British English 'behaviour'. |
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 |
|---|---|---|---|---|
audit |
2096.44 | 1626.02 | -470.42 (-22.44%) | 💔 |
To see the full report comment with /test benchmark fullreport
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
| state.with( | ||
| { | ||
| "want_more": false, | ||
| "transport_error_streak": int(state.?transport_error_streak.orValue(0.0)) + 1, |
There was a problem hiding this comment.
| "transport_error_streak": int(state.?transport_error_streak.orValue(0.0)) + 1, | |
| "transport_error_streak": int(state.?transport_error_streak.orValue(0)) + 1, |
Though, if the state is initialised with this field at zero, we don't need any of the optional type handling and this would be
| "transport_error_streak": int(state.?transport_error_streak.orValue(0.0)) + 1, | |
| "transport_error_streak": int(state.transport_error_streak) + 1, |
which I think would be clearer.
| // threshold the failure is persistent, so return a single-object | ||
| // error to mark the input degraded until collection recovers. | ||
| ( | ||
| (int(state.?transport_error_streak.orValue(0.0)) + 1 >= int(state.base.transport_failure_threshold)) ? |
There was a problem hiding this comment.
| (int(state.?transport_error_streak.orValue(0.0)) + 1 >= int(state.base.transport_failure_threshold)) ? | |
| (int(state.?transport_error_streak.orValue(0)) + 1 >= int(state.base.transport_failure_threshold)) ? |
| } | ||
| ), | ||
| "want_more": false, | ||
| "transport_error_streak": int(state.?transport_error_streak.orValue(0.0)) + 1, |
There was a problem hiding this comment.
| "transport_error_streak": int(state.?transport_error_streak.orValue(0.0)) + 1, | |
| "transport_error_streak": int(state.?transport_error_streak.orValue(0)) + 1, |
| "todo_content": tail(state.cursor.todo_content) + [state.cursor.todo_content[0]], | ||
| } | ||
| ), | ||
| "want_more": false, |
There was a problem hiding this comment.
This line is inconsistent with the {"retry":true} object below.
There was a problem hiding this comment.
@efd6, this is an existing behaviour from - #20333. It's the core of the backoff logic which was implemented. Due to how we influence cursor saves atm, this kind of structure was necessary, basically says "save my queue re-ordering, index nothing, then back off until the next poll."
With the current implementation of transport_error_streak we can technically change this to "want_more": true, but this will be a behaviour change to what the related PR introduced. Also now we might end up in states where we unnecessarily loop on "any transport error", until the threshold is reached. This will be self defeating to the backoff logic and result in quick bursts of errors.
There was a problem hiding this comment.
That's fine, but the object below is pointless.
Ah, maybe not. If this is for the side-effect of publishing a cursor, then it makes sense. Is that what it is for?
There was a problem hiding this comment.
@efd6 yes exactly, we need this for a cursor publish, and it results in this pattern unfortunately.
| { | ||
| "todo_content": tail(state.cursor.todo_content) + [state.cursor.todo_content[0]], | ||
| "want_more": false, | ||
| "transport_error_streak": int(state.?transport_error_streak.orValue(0.0)) + 1, |
There was a problem hiding this comment.
| "transport_error_streak": int(state.?transport_error_streak.orValue(0.0)) + 1, | |
| "transport_error_streak": int(state.?transport_error_streak.orValue(0)) + 1, |
Initialize transport_error_streak to zero in the state block so the counter is always present. The input applies the state block on every start and only overlays the persisted cursor, so the field is guaranteed even after an upgrade from a version that never set it. With the field always present the reads no longer need optional handling: int(state.?transport_error_streak.orValue(0.0)) becomes int(state.transport_error_streak) at all nine sites. Regenerate the policy test snapshots for the changed program. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@efd6, I've resolved all comments besides one, which needs further discussion. |
|
✅ All changelog entries have the correct PR link. |
|
🟢 No issues across the latest commits f43916b.
🤖 AI-Generated Review | Vera Review Bot - v0.2.7 | 📚 Knowledge base: integration-skills
|
💚 Build Succeeded
History
cc @ShourieG |
Type of change
Proposed commit message
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots