feat(agent): add keep-proxy-stream-open ingest flag#3067
Conversation
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "gate proxy per-batch stream close behind..." | Re-trigger Greptile |
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Simple, backward-compatible feature flag (defaults false) that gates an existing per-batch stream-close call; no production risk. Bot comments flag test description and sleep-based assertion style, not correctness of the new logic or the test's core invariant.
ee106a9 to
eadf9c8
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
eadf9c8 to
9000935
Compare
There was a problem hiding this comment.
This removes a deliberate production behavior — per-batch stream close on the proxy path — that was explicitly designed to work around a buffering ingress that holds request bodies until the upload closes. With no current human approval (both top-level reviews were dismissed for older commits), and no visible justification that the proxy ingress no longer buffers, this change needs a human reviewer to confirm the proxy side has been updated before merging.
There was a problem hiding this comment.
This removes an intentional per-batch stream-close on the proxy path that was explicitly designed to work around a buffering ingress that holds request bodies until the upload closes. No human has approved the current HEAD (both top-level reviews are dismissed for older commits), and a substantive bot comment (@stamphog) flags that this behavioral change needs a human to confirm the proxy ingress no longer buffers. Without that confirmation, removing this logic could strand events until stop() is called, risking data loss if a process exits abnormally.
9000935 to
46df848
Compare
There was a problem hiding this comment.
Backward-compatible, opt-in flag defaulting to false — no existing production behavior changes without explicitly setting the env var. The bot comments about removing the per-batch close don't apply to the current implementation, which preserves that behavior by default.
46df848 to
8206e88
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Purely additive, backward-compatible opt-in flag defaulting to false — no existing production behavior changes without explicitly setting the env var. The stamphog bot concerns targeted an older version that removed the per-batch close; the current implementation preserves it as the default and gates the new behavior behind the flag. All inline test concerns are resolved.
Problem
On the event-ingest proxy path,
flush()closes the active NDJSON upload after every drained batch. The ingress in front of the agent-proxy buffers the request body and only forwards it once the upload closes, so closing per batch forwards each batch within a round-trip instead of stranding events in a long-lived upload.We want to switch the proxy path back to a single long-lived buffered upload (matching the Django path) to investigate the ingestion buffering behavior, without changing the default for every run. This puts that behavior behind the
tasks-agent-proxy-keep-stream-openfeature flag, off by default.Changes
keepProxyStreamOpenoption toTaskRunEventStreamSender, sourced fromPOSTHOG_TASK_RUN_EVENT_INGEST_KEEP_STREAM_OPENviaAgentServerConfig.eventIngestKeepStreamOpen.closeActiveStream()on the proxy write leg inflush()behind the flag. Off (default) keeps today's behavior: close per drained batch. On holds one long-lived buffered upload and delivers on the normal roll triggers and at stop.tasks-agent-proxy-keep-stream-openflag is enabled (companion PR feat(tasks): add tasks-agent-proxy-keep-stream-open flag posthog#67492).How did you test this?
pnpm --filter @posthog/agent test event-stream-sender-> 18/18 pass.pnpm --filter @posthog/agent typecheck-> clean.pnpm typecheck; CI green on the pushed commit.Automatic notifications