Skip to content
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 AWS Security Lake with memlog usage #389

Closed
embano1 opened this issue Jan 13, 2023 · 1 comment · Fixed by #390
Closed

Fix error handling AWS Security Lake with memlog usage #389

embano1 opened this issue Jan 13, 2023 · 1 comment · Fixed by #390

Comments

@embano1
Copy link
Contributor

embano1 commented Jan 13, 2023

As the maintainer of memlog I am happy you are using my package! Actually found out incidentally while checking importers of the package. Went through the code and IMHO there are some minor corrections or clarifications needed for the error handling logic in:

func (c *Client) EnqueueSecurityLake(falcopayload types.FalcoPayload) {
offset, err := c.Config.AWS.SecurityLake.Memlog.Write(c.Config.AWS.SecurityLake.Ctx, []byte(falcopayload.String()))
if err != nil {
if err.Error() != "future offset" {
go c.CountMetric(Outputs, 1, []string{"output:awssecuritylake.", "status:error"})
c.Stats.AWSSecurityLake.Add(Error, 1)
c.PromStats.Outputs.With(map[string]string{"destination": "awssecuritylake.", "status": Error}).Inc()
log.Printf("[ERROR] : %v SecurityLake. - %v\n", c.OutputType, err)
return
}
}
log.Printf("[INFO] : %v SecurityLake. - Event queued (%v)\n", c.OutputType, falcopayload.UUID)
*c.Config.AWS.SecurityLake.WriteOffset = offset
}
func (c *Client) StartSecurityLakeWorker() {
for {
time.Sleep(time.Duration(c.Config.AWS.SecurityLake.Interval) * time.Minute)
// time.Sleep(5 * time.Second)
batch := make([]memlog.Record, c.Config.AWS.SecurityLake.BatchSize)
count, err := c.Config.AWS.SecurityLake.Memlog.ReadBatch(c.Config.AWS.SecurityLake.Ctx, *c.Config.AWS.SecurityLake.ReadOffset+1, batch)
if err != nil {
if err.Error() != "future offset" {
go c.CountMetric(Outputs, 1, []string{"output:awssecuritylake.", "status:error"})
c.Stats.AWSSecurityLake.Add(Error, 1)
c.PromStats.Outputs.With(map[string]string{"destination": "awssecuritylake.", "status": Error}).Inc()
log.Printf("[ERROR] : %v SecurityLake. - %v\n", c.OutputType, err)
continue
}
}
if count > 0 {
var err error

For example, this line feels redundant as Write() never throws "future offset" - can be simplified: if err.Error() != "future offset"

Also, I recommend using errors.Is(err, memlog.Err<Type>) instead of string assertions.

if count > 0 ignores error cases where count == 0.

I am happy to provide a PR, just wanted to better understand the error handling code from the author @Issif

@Issif
Copy link
Member

Issif commented Jan 13, 2023

Thanks for your comments. I'll be happy to review a PR from you to fix my silly mistakes 👍

embano1 added a commit to embano1/falcosidekick that referenced this issue Jan 15, 2023
embano1 added a commit to embano1/falcosidekick that referenced this issue Feb 2, 2023
embano1 added a commit to embano1/falcosidekick that referenced this issue Feb 2, 2023
poiana pushed a commit that referenced this issue Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants