http_client: add response and read idle timeouts #10801
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds response and read-idle timeout support: new timestamp and timeout fields in the public HTTP client struct, two public setter APIs, initialization of timestamps on request, enforcement of response and read-idle timeouts in the response read loop, and plugin config wiring to set per-request timeouts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Plugin as out_http
participant Client as flb_http_client
participant Server as HTTP Server
Plugin->>Client: create client
Plugin->>Client: flb_http_set_response_timeout(timeout)
Plugin->>Client: flb_http_set_read_idle_timeout(timeout)
Plugin->>Client: flb_http_do_request(...)
Client->>Server: send HTTP request
rect rgba(230,240,255,0.5)
note over Client: initialize ts_start, last_read_ts
loop read response
Server-->>Client: data chunk / EOF
alt data received
Client->>Client: update last_read_ts
else no data
Client->>Client: if now - ts_start > response_timeout => error
Client->>Client: if now - last_read_ts > read_idle_timeout => error
end
end
end
Client-->>Plugin: response complete / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
plugins/out_http/http.h (1)
97-99: Field added is fine; document units and disable semanticsConsider clarifying in a comment that the value is seconds and that 0 disables the timeout. If you expect larger ranges, prefer uint32_t or time_t over int.
plugins/out_http/http.c (1)
684-688: Config key added — clarify behaviorGood default. Suggest updating the description to mention units and that 0 disables the timeout, e.g., “Maximum time (in seconds) to wait for a server response; set to 0 to disable.”
include/fluent-bit/flb_http_client.h (1)
249-255: State and timeout fields — consider monotonic timeStoring timestamps as time_t is fine, but wall-clock jumps can skew calculations. If feasible, switch to a monotonic clock source for ts_start/last_read_ts (e.g., CLOCK_MONOTONIC) or note the caveat in comments.
src/flb_http_client.c (3)
47-47: Prefer a monotonic clock for timeout logictime(NULL) is wall clock and can jump. If available, prefer a monotonic source (e.g., clock_gettime(CLOCK_MONOTONIC)) for timeout checks to avoid false positives/negatives.
1503-1515: On timeout, recycle the connection to avoid reusing a bad socketWhen a response/read idle timeout triggers, the connection likely isn’t reusable. Recycle it before returning error so it doesn’t re-enter the keepalive pool.
Apply this diff:
- if (c->response_timeout > 0 && - (now - c->ts_start) > c->response_timeout) { - flb_error("[http_client] response timeout reached"); - return FLB_HTTP_ERROR; - } + if (c->response_timeout > 0 && + (now - c->ts_start) > c->response_timeout) { + flb_error("[http_client] response timeout reached"); + flb_upstream_conn_recycle(c->u_conn, FLB_FALSE); + return FLB_HTTP_ERROR; + } @@ - if (c->read_idle_timeout > 0 && - (now - c->last_read_ts) > c->read_idle_timeout) { - flb_error("[http_client] read idle timeout reached"); - return FLB_HTTP_ERROR; - } + if (c->read_idle_timeout > 0 && + (now - c->last_read_ts) > c->read_idle_timeout) { + flb_error("[http_client] read idle timeout reached"); + flb_upstream_conn_recycle(c->u_conn, FLB_FALSE); + return FLB_HTTP_ERROR; + }
1531-1534: Update last_read_ts with a fresh timestampUse a fresh timestamp after the read completes to avoid skew if the read blocked.
- if (r_bytes > 0) { - c->last_read_ts = now; - } + if (r_bytes > 0) { + c->last_read_ts = time(NULL); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
include/fluent-bit/flb_http_client.h(3 hunks)plugins/out_http/http.c(2 hunks)plugins/out_http/http.h(1 hunks)src/flb_http_client.c(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
include/fluent-bit/flb_http_client.h (1)
src/flb_http_client.c (2)
flb_http_set_read_idle_timeout(1143-1147)flb_http_set_response_timeout(1149-1153)
plugins/out_http/http.c (1)
src/flb_http_client.c (2)
flb_http_set_response_timeout(1149-1153)flb_http_set_read_idle_timeout(1143-1147)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
🔇 Additional comments (5)
plugins/out_http/http.c (1)
196-198: Per-request timeouts initialization — LGTMSetting response_timeout from ctx and read_idle_timeout from net.io_timeout is appropriate and keeps behavior consistent with existing net settings.
include/fluent-bit/flb_http_client.h (2)
30-30: Include time.h — LGTM
384-386: New setter prototypes — LGTMsrc/flb_http_client.c (2)
1143-1153: Setter implementations — LGTM
1433-1436: Initialize timeout tracking — LGTM
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/out_http/http.c (1)
686-688: Fix user-facing typo in option description.“paylod” → “payload”.
Apply this diff:
- "Specify if the response paylod should be logged or not" + "Specify if the response payload should be logged or not"
🧹 Nitpick comments (1)
plugins/out_http/http.c (1)
198-204: Simplify idle-timeout selection and add a helpful debug trace.Minor cleanup to avoid duplication and make observability easier during support.
Apply this diff:
- if (ctx->read_idle_timeout > 0) { - flb_http_set_read_idle_timeout(c, ctx->read_idle_timeout); - } - else { - flb_http_set_read_idle_timeout(c, ctx->ins->net_setup.io_timeout); - } + { + int idle_timeout = (ctx->read_idle_timeout > 0) + ? ctx->read_idle_timeout + : ctx->ins->net_setup.io_timeout; + + flb_http_set_read_idle_timeout(c, idle_timeout); + flb_plg_debug(ctx->ins, + "http timeouts: response=%ds, read_idle=%ds", + ctx->response_timeout, idle_timeout); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
plugins/out_http/http.c(2 hunks)plugins/out_http/http.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/out_http/http.h
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_http/http.c (1)
src/flb_http_client.c (2)
flb_http_set_response_timeout(1149-1153)flb_http_set_read_idle_timeout(1143-1147)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (1)
plugins/out_http/http.c (1)
196-197: Good use of per-request timeouts on the HTTP client.Setting the response timeout right after client creation is correct and maps cleanly to the new API.
Comment on lines
+690
to
+699
| { | ||
| FLB_CONFIG_MAP_TIME, "http.response_timeout", "60s", | ||
| 0, FLB_TRUE, offsetof(struct flb_out_http, response_timeout), | ||
| "Set maximum time to wait for a server response" | ||
| }, | ||
| { | ||
| FLB_CONFIG_MAP_TIME, "http.read_idle_timeout", "0s", | ||
| 0, FLB_TRUE, offsetof(struct flb_out_http, read_idle_timeout), | ||
| "Set maximum allowed time between two consecutive reads" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify option descriptions to spell out behavior and defaults.
Tighten phrasing and document the fallback to net.io_timeout for read idle.
Apply this diff:
- {
- FLB_CONFIG_MAP_TIME, "http.response_timeout", "60s",
- 0, FLB_TRUE, offsetof(struct flb_out_http, response_timeout),
- "Set maximum time to wait for a server response"
- },
+ {
+ FLB_CONFIG_MAP_TIME, "http.response_timeout", "60s",
+ 0, FLB_TRUE, offsetof(struct flb_out_http, response_timeout),
+ "Maximum time to wait for the server response (status/headers). Default: 60s"
+ },
- {
- FLB_CONFIG_MAP_TIME, "http.read_idle_timeout", "0s",
- 0, FLB_TRUE, offsetof(struct flb_out_http, read_idle_timeout),
- "Set maximum allowed time between two consecutive reads"
- },
+ {
+ FLB_CONFIG_MAP_TIME, "http.read_idle_timeout", "0s",
+ 0, FLB_TRUE, offsetof(struct flb_out_http, read_idle_timeout),
+ "Maximum allowed idle time between consecutive socket reads; if 0s, falls back to net.io_timeout; set >0 to override"
+ },To ensure docs/examples are in sync, run:
🏁 Script executed:
#!/bin/bash
# Verify references and docs for the new options
rg -n -C2 -g '!**/build/**' -g '!**/dist/**' $'http\\.response_timeout|http\\.read_idle_timeout'Length of output: 768
Update documentation for http.response_timeout and http.read_idle_timeout
The new descriptions in plugins/out_http/http.c aren’t reflected in any docs or examples—no matches outside the C code. Please update the corresponding documentation (e.g. docs/plugins/out_http.md or your examples) to include the revised option texts and defaults.
🤖 Prompt for AI Agents
In plugins/out_http/http.c around lines 690-699, the config map descriptions for
http.response_timeout and http.read_idle_timeout were changed but the
documentation/examples were not updated; update docs/plugins/out_http.md (and
any example configs) to add these two options, include the exact option names,
their revised descriptions ("Set maximum time to wait for a server response"
with default 60s for http.response_timeout; "Set maximum allowed time between
two consecutive reads" with default 0s for http.read_idle_timeout), and ensure
the config reference table and any example config snippets reflect the new
defaults and wording.
cosmo0920
reviewed
Sep 1, 2025
src/flb_http_client.c
Outdated
| } | ||
| available = flb_http_buffer_available(c) - 1; | ||
| } | ||
| time_t now = time(NULL); |
There was a problem hiding this comment.
Nits: Do we need to define this variable on top of this function?
Just for confirming to follow our coding guideline.
Signed-off-by: Eduardo Silva <[email protected]>
… props Signed-off-by: Eduardo Silva <[email protected]>
edsiper
force-pushed
the
http-client-timeouts
branch
from
369b421 to
b84219b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugins/out_http/http.c (1)
690-699: Clarify option descriptions and document fallback behaviorTighten phrasing and explicitly mention defaults and the read-idle fallback to net.io_timeout.
- { - FLB_CONFIG_MAP_TIME, "http.response_timeout", "60s", - 0, FLB_TRUE, offsetof(struct flb_out_http, response_timeout), - "Set maximum time to wait for a server response" - }, + { + FLB_CONFIG_MAP_TIME, "http.response_timeout", "60s", + 0, FLB_TRUE, offsetof(struct flb_out_http, response_timeout), + "Maximum time to wait for the server response (status/headers). Default: 60s" + }, - { - FLB_CONFIG_MAP_TIME, "http.read_idle_timeout", "0s", - 0, FLB_TRUE, offsetof(struct flb_out_http, read_idle_timeout), - "Set maximum allowed time between two consecutive reads" - }, + { + FLB_CONFIG_MAP_TIME, "http.read_idle_timeout", "0s", + 0, FLB_TRUE, offsetof(struct flb_out_http, read_idle_timeout), + "Maximum allowed idle time between consecutive socket reads; if 0s, falls back to net.io_timeout; set >0 to override" + },Also update user docs/examples to include these options and descriptions (label docs-required). I can draft the docs snippet if you want.
🧹 Nitpick comments (3)
src/flb_http_client.c (2)
47-47: Prefer a monotonic clock for timeout trackingUsing time(NULL) is wall-clock and can jump (NTP, manual changes). For robust timeouts, use CLOCK_MONOTONIC when available, falling back to time(NULL).
Apply within existing spots:
@@ -#include <time.h> +#include <time.h>And at initialization/read sites (see further diffs below).
1143-1147: Clamp negative timeout inputsDefensive check avoids unintended negative values from future callers or mis-parsed config.
int flb_http_set_read_idle_timeout(struct flb_http_client *c, int timeout) { - c->read_idle_timeout = timeout; + if (timeout < 0) { + timeout = 0; + } + c->read_idle_timeout = timeout; return 0; } int flb_http_set_response_timeout(struct flb_http_client *c, int timeout) { - c->response_timeout = timeout; + if (timeout < 0) { + timeout = 0; + } + c->response_timeout = timeout; return 0; }Also applies to: 1149-1153
plugins/out_http/http.c (1)
196-204: Good: per-request timeout wiring with sensible fallbackWires response_timeout directly and read_idle_timeout with a fallback to net.io_timeout. Consider adding a debug line to log the resolved values to aid support.
c->cb_ctx = ctx->ins->callback; flb_http_set_response_timeout(c, ctx->response_timeout); if (ctx->read_idle_timeout > 0) { flb_http_set_read_idle_timeout(c, ctx->read_idle_timeout); } else { flb_http_set_read_idle_timeout(c, ctx->ins->net_setup.io_timeout); } + flb_plg_debug(ctx->ins, "timeouts: response=%ds, read_idle=%ds (resolved)", + ctx->response_timeout, + (ctx->read_idle_timeout > 0) ? ctx->read_idle_timeout + : ctx->ins->net_setup.io_timeout);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
include/fluent-bit/flb_http_client.h(3 hunks)plugins/out_http/http.c(2 hunks)plugins/out_http/http.h(1 hunks)src/flb_http_client.c(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- include/fluent-bit/flb_http_client.h
- plugins/out_http/http.h
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_http/http.c (1)
src/flb_http_client.c (2)
flb_http_set_response_timeout(1149-1153)flb_http_set_read_idle_timeout(1143-1147)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
- GitHub Check: PR - fuzzing test
🔇 Additional comments (1)
src/flb_http_client.c (1)
1433-1436: Time fields and timeout units verified
ts_start and last_read_ts are declared as time_t, and response_timeout/read_idle_timeout are ints (seconds), matching their comparisons; no changes required.
Comment on lines
+1433
to
+1436
| /* Initialize timeout tracking */ | ||
| c->ts_start = time(NULL); | ||
| c->last_read_ts = c->ts_start; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use CLOCK_MONOTONIC for timeout measurements
Prevents false positives/negatives if the system clock changes. Keeps your compare logic intact.
@@ int flb_http_do_request(struct flb_http_client *c, size_t *bytes)
- /* Initialize timeout tracking */
- c->ts_start = time(NULL);
- c->last_read_ts = c->ts_start;
+ /* Initialize timeout tracking */
+#if defined(CLOCK_MONOTONIC)
+ {
+ struct timespec ts;
+ clock_gettime(CLOCK_MONOTONIC, &ts);
+ c->ts_start = (time_t) ts.tv_sec;
+ c->last_read_ts = c->ts_start;
+ }
+#else
+ c->ts_start = time(NULL);
+ c->last_read_ts = c->ts_start;
+#endif
@@ int flb_http_get_response_data(struct flb_http_client *c, size_t bytes_consumed)
- time_t now;
+ time_t now;
+ struct timespec ts;
@@
- now = time(NULL);
+ /* monotonic time for timeouts */
+#if defined(CLOCK_MONOTONIC)
+ clock_gettime(CLOCK_MONOTONIC, &ts);
+ now = (time_t) ts.tv_sec;
+#else
+ now = time(NULL);
+#endif
@@
- if (r_bytes > 0) {
- c->last_read_ts = now;
- }
+ if (r_bytes > 0) {
+ c->last_read_ts = now;
+ }Also applies to: 1504-1504, 1460-1461, 1532-1535
🤖 Prompt for AI Agents
In src/flb_http_client.c around lines 1433-1436 (and also apply to lines
~1460-1461, 1504, and 1532-1535), the code initializes timeout timestamps using
time(NULL); replace these uses of the system clock with CLOCK_MONOTONIC-based
time measurements: use clock_gettime(CLOCK_MONOTONIC, &ts) to obtain a monotonic
timespec, convert to a consistent integer/float representation (e.g., seconds or
milliseconds) and assign to c->ts_start and c->last_read_ts (and any other
timeout timestamp variables), and update any comparisons to use the same
monotonic representation so timeout checks are immune to system clock changes.
Signed-off-by: Eduardo Silva <[email protected]>
This was referenced Sep 1, 2025
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements timeouts for the full http response and for read idle timeouts.
It also extends HTTP output plugin with the new property
http.response_timeoutto set the maximum duration of a response (for slow servers), the default is60s.Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit