-
Notifications
You must be signed in to change notification settings - Fork 166
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
fix: Error handling in AWS Security Lake #390
Conversation
Welcome @embano1! It looks like this is your first PR to falcosecurity/falcosidekick 🎉 |
@embano1 Can you take a look at my answers to your comments please |
@Issif sorry for my delayed response! Will hopefully get back to this until the EOW. Any deadline you have in mind/blocking you? |
No deadline, take your time, I just wanted to be sure you still follow. Thanks |
Closes: falcosecurity#389 Signed-off-by: Michael Gasch <15986659+embano1@users.noreply.github.com>
@Issif made a couple more changes to catch edge cases. Please take a critical look :) Still not quite happy with the code as we ignore context errors, no proper shutdown logic and coarse error handling. But that's out of scope for this PR. |
The lack on contexts is something I know, for now, it's doesn't create any issue as far I know, but I'll change that in future v3. Like you said, it's way over the scope of this PR. Thanks for your help. |
LGTM label has been added. Git tree hash: 6fd404421199917b1f1159882038ccaa94e7b8e7
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: embano1, Issif The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Closes: #389
Signed-off-by: Michael Gasch 15986659+embano1@users.noreply.github.com
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area outputs
What this PR does / why we need it:
Improves error handling in AWS Security Lake output.
Which issue(s) this PR fixes:
Fixes #389
Special notes for your reviewer:
I could not test the PR as I am not very familiar with the project (I'm the maintainer of
memlog
which is used here). Would be great to get some discussion on the PR as my understanding of the Falco agent behavior is limited.cc/ @Issif