-
Notifications
You must be signed in to change notification settings - Fork 515
[o365] Simplification of data fetching logic #16024
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we lean on repeated input loops more now, I think it may be worth doing the dropping of the retry events in the agent with a beat processor, rather than sending the event to be dropped by the ingest pipeline. An example of this is here corresponding to this alternative way of saying {"retry": true}.
… means it won't be a duplicate during ingest anyway).
3645b09 to
d0e4387
Compare
🚀 Benchmarks reportTo see the full report comment with |
💚 Build Succeeded
History
|
Proposed commit message
Notes for the reviewer
I started this before the last 4 PRs, and I checked that there are no conflicts with those changes:
We no longer parse the next page URLs, since we have the
endTimefrom the initial request.o365: fix error propagation within cel program #15445
This error handling logic isn't relevant in the new flattened structure.
The new code still limits subscription requests, but to a lesser degree, which I think is better overall.
The new CEL code passes with the original system tests.
The system tests have been updated to use a mock o365 server rather than the
streamtool. The mock server has configurability, assertions and logging beyond what is used in the system test, which may be useful for future debugging. The amount of code is not insignificant, and the quality is just okay, so if this is a maintenance concern, it can be moved to a separate PR or removed entirely. Let me know what you think.Checklist
changelog.ymlfile.How to test this PR locally
You can manually run the mock server like this:
In another terminal, run the CEL code in
mitolike this:Stop the mock server with Ctrl+C to trigger it's shutdown report.
You can remove the
mitoauth configuration if you disableCheckAccessTokenin the mock server's inline configuration.This version of
mitowill run faster than the following one, which rate limits to 1 rps:Related issues