Conversation
|
Seems like it works. |
auditing/auditing.go
Outdated
| // Async indexes audit traces asynchronously if set to true. if this functionality is implemented depends on the audit backend implementation. | ||
| // If this is set to true it can occur that audit traces get lost in case the backend is not available for receiving the trace. | ||
| Async bool | ||
| // AsyncRetry defines the amount of attempts to retry sending an audit trace to a backend in case it failed. | ||
| AsyncRetry int | ||
| // AsyncBackoff defines the backoff after a failed attempt to index an audit trace to a backend. | ||
| AsyncBackoff time.Duration | ||
| // AsyncTimeout sets a timeout for indexing a trace for the backend. | ||
| AsyncTimeout time.Duration |
There was a problem hiding this comment.
These configs are declared for all auditing providers, but are only implemented for splunk.
Either these should be moved to the SplunkConfig or a different solution should be considered.
What if we'd create an asyncAuditing which would then only implement the async mechanism while forwarding the index requests to the actual/wrapped backend?
// would be used like this:
splunk, err := auditing.Splunk(...)
if err != nil {
return nil, err
}
asyncAuditing := auditing.Async(auditing.AsyncConfig{Retry: 1, /*...*/}, splunk)
return asyncAuditing, nilThere was a problem hiding this comment.
Yes, sounds good. I wanted to keep it small as first attempt and don't generalize it, but it certainly much cleaner to do it like this. I'll give that a try.
auditing/splunk.go
Outdated
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), a.asyncTimeout) | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, a.endpoint+"/services/collector", bytes.NewBuffer(e)) |
There was a problem hiding this comment.
Is there any actual difference between the core of this for loop and the non-async variant?
This could be extracted into one function which could receive a context. In case of extracting the async mechanism this is of course obsolete.
Co-authored-by: Valentin Knabel <valentin.knabel@x-cellent.com>
2 participants
Description
I tested this locally against the official splunk container image: https://hub.docker.com/r/splunk/splunk.