Added PUT support #10882
Added PUT support #10882
Conversation
WalkthroughAdds a configurable HTTP method to the out_http plugin: new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User Config
participant Conf as out_http Conf
participant Ctx as flb_out_http (ctx)
participant Up as Upstream
participant Net as HTTP Client
User->>Conf: set http_method = "POST"/"PUT"
Conf->>Ctx: parse & set ctx.http_method
alt invalid method
Conf-->>User: log error, abort init
else valid method
Conf->>Up: initialize upstream
Note over Ctx,Net: On flush/send, use ctx.http_method
Ctx->>Net: create/send HTTP request (method = ctx.http_method)
Net-->>Ctx: response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
|
Added a small Documentation update here: fluent/fluent-bit-docs#2064 |
nicknezis
requested review from
edsiper,
fujimotos and
koleini
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
plugins/out_http/http.c (2)
176-180: Null-check the HTTP client to prevent a crash and leak.
flb_http_client(...)can return NULL;c->proxy.hostis dereferenced later andcleanup:unconditionally destroysc. Guard the creation path and the cleanup.Apply this diff:
- /* Create HTTP client context */ - c = flb_http_client(u_conn, ctx->http_method, ctx->uri, + /* Create HTTP client context */ + c = flb_http_client(u_conn, ctx->http_method, ctx->uri, payload_buf, payload_size, ctx->host, ctx->port, ctx->proxy, 0); + if (c == NULL) { + flb_plg_error(ctx->ins, "failed to create HTTP client for %s:%i", + ctx->host, ctx->port); + if (payload_buf != body) { + flb_free(payload_buf); + } + flb_upstream_conn_release(u_conn); + return FLB_RETRY; + }And in the cleanup section:
- /* Destroy HTTP client context */ - flb_http_client_destroy(c); + /* Destroy HTTP client context */ + if (c != NULL) { + flb_http_client_destroy(c); + }
503-513: Bug: wrong NULL checks in header extraction (latent leak/skip).The code checks
headers[i]instead ofheaders[i*2]/headers[i*2+1]. On allocation failure it won’t trip the guard, possibly leaking a half-pair and skipping headers.Apply this fix:
- headers[i * 2] = strndup(k.ptr, k.size); - - if (!headers[i]) { + headers[i * 2] = strndup(k.ptr, k.size); + if (!headers[i * 2]) { goto err; } - headers[i * 2 + 1] = strndup(v.ptr, v.size); - - if (!headers[i]) { + headers[i * 2 + 1] = strndup(v.ptr, v.size); + if (!headers[i * 2 + 1]) { goto err; }
🧹 Nitpick comments (2)
plugins/out_http/http.c (2)
635-637: Make error message method-neutral.Minor text nit; avoid “post” now that PUT is supported.
Apply:
- "failed to post requests body key \"%s\"", ctx->body_key); + "failed to send requests with body key \"%s\"", ctx->body_key);
530-535: Optional: rename function to match semantics.
post_all_requestsis misleading now; suggestsend_all_requests.Example:
-static int post_all_requests(struct flb_out_http *ctx, +static int send_all_requests(struct flb_out_http *ctx,And update its single call site in
cb_http_flush.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/out_http/http.c(5 hunks)plugins/out_http/http.h(1 hunks)plugins/out_http/http_conf.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
plugins/out_http/http_conf.c (2)
src/flb_output.c (1)
flb_output_get_property(1096-1099)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
plugins/out_http/http.c (1)
src/flb_http_client.c (1)
flb_http_client(814-859)
🔇 Additional comments (5)
plugins/out_http/http.h (1)
70-72: Field addition looks good; preserves defaulting via conf.Placing
http_methodnear connection params is fine and keeps the struct cohesive. No ABI concerns within the tree since all offsets are recompiled.plugins/out_http/http.c (4)
112-116: Rename to method-agnostic helper is sane.Signature and intent match the new config; good extraction point.
599-602: Trace message now method-neutral — good.Keeps logs accurate for POST/PUT.
650-659: Method-agnostic calls look correct.
http_request(...)used consistently for both JSON/msgpack paths.
760-764: Config map entry for http_method: approve (docs/UI); verify for POST leftoversConfig map entry is fine — manual conf parsing makes offsetof=0 acceptable and the description is clear. Approving changes.
Sandbox search failed ("No files were searched"). Run locally to confirm there are no remaining POST-specific code paths or log strings:
rg --debug -n --hidden -uu -S -g '!**/vendor/**' \ -e 'http_post\(' \ -e 'posting record' \ -e 'failed to post' \ -e '\bPOST\b' -i
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugins/out_http/http.c (3)
530-535: Return an aggregated status for multi-record sends.send_all_requests() overwrites ret per record; a late success can mask earlier failures. Track an overall_ret (prefer FLB_RETRY over FLB_OK, FLB_ERROR over both) and return it.
Example patch (core idea):
- int ret = 0; + int ret = 0; + int overall_ret = FLB_OK; @@ - ret = http_request(ctx, body, body_size, event_chunk->tag, - flb_sds_len(event_chunk->tag), headers); + ret = http_request(ctx, body, body_size, event_chunk->tag, + flb_sds_len(event_chunk->tag), headers); + if (ret == FLB_ERROR) { + overall_ret = FLB_ERROR; + } + else if (ret == FLB_RETRY && overall_ret == FLB_OK) { + overall_ret = FLB_RETRY; + } @@ - return ret; + return overall_ret;
598-602: Enrich trace with HTTP method.Including the verb helps debug mixed POST/PUT flows.
Apply this diff:
- flb_plg_trace(ctx->ins, "sending record %zu", record_count++); + flb_plg_trace(ctx->ins, "sending record %zu via %s", + record_count++, + ctx->http_method == FLB_HTTP_POST ? "POST" : "PUT");
631-637: Tweak error message grammar.“failed to send requests body key” reads oddly.
Apply this diff:
- "failed to send requests body key \"%s\"", ctx->body_key); + "failed to send requests using body key \"%s\"", ctx->body_key);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_http/http.c(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_http/http.c (2)
src/flb_http_client.c (1)
flb_http_client(814-859)src/flb_sds.c (1)
flb_sds_destroy(389-399)
🔇 Additional comments (2)
plugins/out_http/http.c (2)
650-659: LGTM: method-agnostic requests in flush path.Replacing http_post() with http_request() for both JSON/GELF and msgpack paths looks correct; tag/length handling is intact.
760-764: Config surface LGTM — parsing & PUT support verifiedplugins/out_http/http_conf.c parses http_method case‑insensitively (defaults to FLB_HTTP_POST; "PUT" → FLB_HTTP_PUT), FLB_HTTP_PUT is defined in include/fluent-bit/flb_http_client.h and handled by src/flb_http_client.c; callers use FLB_HTTP_PUT.
|
Could you add Signed-off line in your commit? We need to follow DCO rules and that workflow to be green. |
I rebased onto master and signed the original commit. Hopefully it looks better now? |
Signed-off-by: Nicholas Nezis <[email protected]>
Signed-off-by: Nicholas Nezis <[email protected]>
Signed-off-by: Nicholas Nezis <[email protected]>
Signed-off-by: Nicholas Nezis <[email protected]>
Usage
Users can now configure the HTTP method in their Fluent Bit configuration:
[OUTPUT]
Name http
Match *
Host example.com
Port 443
URI /api/logs
http_method PUT
tls on
The plugin will validate the method and default to POST if not specified, maintaining backward compatibility.
Ticket: #10522
Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
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
New Features
Bug Fixes
Documentation