fix(o365): resume collection from a persisted checkpoint and follow content-list pagination - #2550
Merged
osmontero merged 5 commits intoSep 8, 2026
Conversation
The Management Activity API truncates content-list responses and signals continuation with a NextPageUri response header. The collector read only the first response, so every content blob beyond the first page was silently dropped for tenants with enough audit volume to paginate. The go-sdk utils.DoReq helper cannot surface response headers, so this adds a local request helper that returns them, and follows the header URL verbatim because its nextPage parameter is an opaque server-side id. NextPageUri is validated against the configured management endpoint's scheme and host before being followed, since the request carries a bearer token and the URL comes from the response. Closes #2436
The collector kept one collection window in memory for every group and advanced it by wall clock after each tick, whether or not collection had succeeded. Restarting the plugin, the event processor or the host re-seeded the window to five minutes ago, and a transient auth failure, throttle or 5xx skipped that time range permanently. GetLogs also swallowed content list and content detail errors, so nothing upstream could tell a complete interval from a partial one. Each group now keeps its own window position, persisted to disk and reloaded on startup, and it only advances past a window that was actually collected. Collection errors propagate through GetLogs and pull instead of being logged and dropped, and GetContentDetails requires HTTP 200 like GetContentList already did. A single window end timestamp per group is enough because the API filters on when a content blob became available rather than when the event happened, so a high-water mark cannot miss late arriving events. Windows are bounded at twelve hours. The API's documented maximum is twenty four, and beyond it results are partial rather than rejected, so a window that stops advancing on failure must never be allowed to grow into that range. Only one window is collected per tick: pull rebuilds the whole session on every call, so collecting a whole backlog in one tick multiplied the token requests and subscription starts and blocked the tick long enough to stall configuration updates. Advancing up to twelve hours per tick drains the largest permitted backlog in about an hour. Positions are clamped a day short of the API's seven day retention. The request is issued seconds after the tick captures the current time, so clamping to exactly seven days produced a start time the API rejects, and a rejected window never advances, which stalled collection permanently for a tenant that fell that far behind. Positions are never pruned while the configured group list is empty, since that is indistinguishable from configuration not having loaded yet and pruning would recreate the gap this closes. The state file lives in the pipeline directory, which is already a host mount for the event processor container, so no installer or deployment change is needed. Writes are atomic, a missing file is a normal first run, and an unreadable one degrades to the previous seeding behaviour instead of taking collection down. Closes #2435
…enabled one StartSubscriptions returned nil from inside the retry loop when Microsoft reported a subscription as already enabled, which exited the whole function and abandoned the remaining content types. In steady state the first subscription is always already enabled, so the other four were never attempted. That is a bootstrap trap: if a subscription failed to start on the first run, every later run short circuited on the first content type and the failed one was never started again. Its content stayed unavailable permanently, because the API refuses to list or retrieve content for a subscription that is not enabled. The already enabled response now clears the error and moves to the next subscription, so a disabled or never started content type is recovered on the next tick. StartSubscriptions also uses the shared retry delay instead of its own hardcoded copy; the default is unchanged.
This was
linked to
issues
Sep 7, 2026
❌ Go dependencies check failedThere are outdated Go dependencies, or modules that could not be inspected. Script output |
✅ AI review — ApprovedNo issues detected in this diff. ✅
|
doReqWithHeaders returned the decoded body, headers and status without treating a non success status as an error, unlike utils.DoReq alongside it. Callers checked the status, so no failed response was ever accepted, but the error they logged came from unmarshalling an error document into the expected type. An AF20030 rejection therefore reached the operator as "json: cannot unmarshal object into Go value of type []main.ContentList" instead of the code and message the API actually returned. The helper now reports non success statuses with the same error shape as utils.DoReq, so the two behave alike in the same file.
osmontero
approved these changes
Sep 8, 2026
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
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.
Closes #2435
Closes #2436
What was broken
#2436 — The API truncates content-list responses and signals continuation with a
NextPageUriresponse header. The collector read only the first response, so everycontent blob past the first page was dropped. No error surfaced, because the first
request succeeded.
#2435 — The collection window lived in memory and advanced by wall clock every
tick, whether collection succeeded or not. Any restart re-seeded it to five minutes
ago; any transient failure skipped that range forever.
Fixed in that order on purpose: persisting a checkpoint over a truncated enumeration
would record "interval complete" while pages were missing, which is worse than the
original bug.
What was built
GetContentListfollows theNextPageUrichain to the end, using a local HTTPhelper because
utils.DoReqcannot return response headers. The URL is validatedagainst the configured management endpoint's scheme and host before being followed —
the request carries a bearer token — and then used verbatim, since its
nextPageparameter is an opaque server id.
Each group owns a collection window position, persisted to disk. Per tick:
load position (or seed at now-5m on first run)
clamp forward if older than 6 days
window = position, min(position + 12h, now)
collect it
on error → return; position untouched, next tick retries the same range
on success → position = window end, persisted
For that to work, errors had to stop being swallowed.
GetLogsnow returns theevents it gathered together with an error when any subscription or blob failed,
and
pullpropagates it.GetContentDetailsalso requires HTTP 200 now, matchingGetContentList.State file
{WORK_DIR}/pipeline/o365_state.json— one timestamp per group, written atomically(temp file + rename) under the same lock that guards the in-memory map:
{"groups":{"3":{"groupName":"test","windowEnd":"2026-09-07T19:19:43.488057286Z"}}}Missing file is a normal first run. A corrupt one is logged and treated as empty,
falling back to the old seeding behaviour rather than stopping collection. A failed
write never aborts a tick.
One timestamp per group is enough because the API filters on when a blob became
available, not when the event happened, so a high-water mark cannot miss
late-arriving events.
Third fix
StartSubscriptions returned nil from inside the retry loop on "already
enabled", which exited the whole function and abandoned the other four content
types. If one failed to start on the very first run, it was never attempted again
and its content stayed unavailable permanently.