feat(go-forwarder): add proxy and custom compression level#1140
feat(go-forwarder): add proxy and custom compression level#1140ndakkoune wants to merge 5 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
ndakkoune
force-pushed
the
nabil.dakkoune/AWSINTS-3697
branch
from
0a34912 to
009bec7
Compare
| cfg *config.Config | ||
| client *http.Client | ||
| storage string | ||
| gzipPool *sync.Pool |
There was a problem hiding this comment.
I did not find any other solution to use a gzip pool with a custom compression level if we use a package-level variable since we have to create the gzipWriter from the configuration. @ViBiOh
There was a problem hiding this comment.
I think it should be fine since we're using it in a single place
| @@ -22,6 +23,9 @@ func main() { | |||
| if err != nil { | |||
There was a problem hiding this comment.
(will change this in the future by joining all the errors in the config, keeping it as is for now)
| @@ -22,6 +23,9 @@ func main() { | |||
| if err != nil { | |||
| panic(err) | |||
There was a problem hiding this comment.
nit: prefer log.fatal instead (recommended to do the same elsewhere)
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.APIURL+"/api/v1/validate", nil) | ||
| if err != nil { | ||
| slog.Warn("failed to build API key validation request", slog.Any("error", err)) | ||
| return nil |
There was a problem hiding this comment.
I think we should return the error here not to silently drop it
| c.SkipServerCertificate = envOrDefaultBool(EnvSkipServerCertificate, false) | ||
| c.UseHTTP = envOrDefaultBool(EnvUseHTTP, false) | ||
|
|
||
| scheme := "https" |
There was a problem hiding this comment.
nit: i think protocol is a straight forward name
|
|
||
| func (f Forwarder) Start(ctx context.Context, in <-chan model.LogEntry) error { | ||
| batches := make(chan []byte, MaxConcurrency) | ||
| batches := make(chan []byte, httpclient.MaxConcurrency) |
There was a problem hiding this comment.
I didn't get why are we limiting the batches slice size to max concurrency ?
There was a problem hiding this comment.
My assumption was that the batcher (CPU bound) would produce more batches than the forwarding goroutines (network bound).
| var Client *http.Client | ||
|
|
||
| func newClient() *http.Client { | ||
| func Init(skipServerCertificate bool) { |
There was a problem hiding this comment.
nit: prefer options pattern, easier to extend in the future if we want to pass other options
2 participants
What does this PR do?
Adds the custom compression level and the proxy support.
Motivation
Testing Guidelines
Additional Notes
DD_USE_COMPRESSIONin favor toDD_COMPRESSION_LEVELHTTP_PROXYandHTTPS_PROXYto configure it ootb; the CloudFormation Template as the responsibility to bindDdHttpProxyURLto the two variables).Types of changes
Check all that apply