-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement audit backend for TimescaleDB. #151
Conversation
e0481fa
to
31ebdac
Compare
31ebdac
to
5c3a30c
Compare
auditing/timescaledb.go
Outdated
if err := addFilter("body", filter.Body, phrase); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("component", filter.Component, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("detail", filter.Detail, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("error", filter.Error, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("forwardedfor", filter.ForwardedFor, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("path", filter.Path, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("phase", filter.Phase, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("remoteaddr", filter.RemoteAddr, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("rqid", filter.RequestId, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("statuscode", filter.StatusCode, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("tenant", filter.Tenant, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("type", filter.Type, equals); err != nil { | ||
return nil, err | ||
} | ||
if err := addFilter("user", filter.User, equals); err != nil { | ||
return nil, err | ||
} | ||
|
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.
this must all match to the Entry json annotations, easy to make mistakes here or miss some of them. There might be a smarter way, but unsure.
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.
Only other approach that comes to my mind is to put the field names into constants and build a generic map[string]any
with them, which will be JSON-serialized instead of using the struct with JSON annotations. Not sure if it's better.
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.
Looks really promising! Please resolve the conflicts and run go mod tidy
So, how about this pull request? I think the implementation here should work? |
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.
One small nit, but if fixed we can merge
Yup, this implementation is fine. I just tend to review the whole bunch of PRs at once. I only had comments on the other two |
Thanks for the reviews, I will adapt the related PRs. |
References: