[Filebeat] httpjson - possible memory leak

[andrewkroh]

While using the ti_recordedfuture integration that collects CSV data with the Filebeat httpjson input I am observing continued memory growth from Filebeat. Attached is a heap profile (not from the same time as this graph).

For confirmed bugs, please report:

• Version: 8.7.0, 8.7.1-SNAPSHOT (bda4053)
• Operating System: linux/amd64
• Steps to Reproduce:

1. Create agent policy.
2. Add four instance of ti_recordedfuture (one for each data type... domain, url, ip, hash).
3. Let it collect over several iterations.

Memory profile from 8.7.1-SNAPSHOT: mem2.pprof.gz

[elasticmachine]

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

[andrewkroh]

Delve shows that the transformContext is holding on the last response which consist of everything returned (100,000 CSV rows). Any way to prevent this?

[ShourieG]

@andrewkroh right now no way ... by default..the last response is always held in transform context . Maybe we could introduce a flag to discard the last response in scenarios in which we don't need it. This will ofc prevent any further operations using .last_response.

[andrewkroh]

We should migrate this to the CEL input. At least with CEL it would not hold onto excessive amounts of data between iterations. It would still hold the data in memory during the request, but after that it will get released.

[elasticmachine]

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

[efd6]

Whether we can do anything about this hinges on whether we guarantee/expect the last response to survive across periodic evaluations. I don't think we do/should since this would make a periodic evaluation within a single filebeat behave differently to periodic evaluations across restarts. The documentation says

All of the mentioned objects are only stored at runtime, except cursor, which has values that are persisted between restarts.

Which doesn't clarify this case, at least not why the last response in this run is considered more important than the last response ever.

If we can tighten this, then it is possible to nil out the last response body at the end of the periodic evaluation. I think this is a reasonable thing to do.

diff --git a/x-pack/filebeat/input/httpjson/input.go b/x-pack/filebeat/input/httpjson/input.go
index 50a4f7f20a..e0f8be3f31 100644
--- a/x-pack/filebeat/input/httpjson/input.go
+++ b/x-pack/filebeat/input/httpjson/input.go
@@ -163,6 +163,11 @@ func run(ctx v2.Context, cfg config, pub inputcursor.Publisher, crsr *inputcurso
        trCtx.cursor.load(crsr)
 
        doFunc := func() error {
+               defer func() {
+                       // Clear response bodies between evaluations.
+                       trCtx.firstResponse.body = nil
+                       trCtx.lastResponse.body = nil
+               }()
+
                log.Info("Process another repeated request.")
 
                startTime := time.Now()

Looking at a subset of the integrations that use HTTPJSON I do not see any use of .last_response.body that depends on it persisting across periodic evals. Arguably, any configuation that is doing this is being clever and will likely break in interesting and difficult to debug ways when restarts are part of the mix.

[andrewkroh]

I don't think we do/should since this would make a periodic evaluation within a single filebeat behave differently to periodic evaluations across restarts.

Great point. If we didn't find any usages within our own integrations, then I would be comfortable with clarifying the docs around this and making that change. 😀