http: default HTTP User-Agent header with version information#11434
http: default HTTP User-Agent header with version information#11434endersonmaia wants to merge 34 commits intofluent:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds centralized User-Agent header macros and includes version info in the public HTTP client header; replaces ad-hoc "User-Agent"/"Fluent-Bit" literals across core HTTP client, AWS utils, many plugins, and tests. Some plugins now add an explicit User-Agent header where previously absent. (47 words) Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4466f5114
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
e4466f5 to
e14def4
Compare
e14def4 to
d2ca797
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/out_websocket/websocket.c (1)
70-84:⚠️ Potential issue | 🟠 MajorUser‑configured
User-Agentgets overridden.Because
flb_http_add_headerde‑duplicates by key, adding the default header after config headers will replace any user‑providedUser-Agent. This breaks override expectations and is inconsistent with other plugins.♻️ Suggested ordering to allow user overrides
- /* Append additional headers from configuration */ - flb_config_map_foreach(head, mv, ctx->headers) { - key = mk_list_entry_first(mv->val.list, struct flb_slist_entry, _head); - val = mk_list_entry_last(mv->val.list, struct flb_slist_entry, _head); - - flb_http_add_header(c, - key->str, flb_sds_len(key->str), - val->str, flb_sds_len(val->str)); - } - - /* User-Agent */ - flb_http_add_header(c, FLB_HTTP_HEADER_USER_AGENT, - sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1, - FLB_HTTP_HEADER_USER_AGENT_DEFAULT, - sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1); + /* User-Agent (default; can be overridden by configured headers) */ + flb_http_add_header(c, FLB_HTTP_HEADER_USER_AGENT, + sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1, + FLB_HTTP_HEADER_USER_AGENT_DEFAULT, + sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1); + + /* Append additional headers from configuration */ + flb_config_map_foreach(head, mv, ctx->headers) { + key = mk_list_entry_first(mv->val.list, struct flb_slist_entry, _head); + val = mk_list_entry_last(mv->val.list, struct flb_slist_entry, _head); + + flb_http_add_header(c, + key->str, flb_sds_len(key->str), + val->str, flb_sds_len(val->str)); + }
🤖 Fix all issues with AI agents
In `@plugins/in_nginx_exporter_metrics/nginx.c`:
- Around line 136-141: The call to flb_http_add_header(…,
FLB_HTTP_HEADER_USER_AGENT, …) must have its return value checked and handled;
if it fails (e.g., allocation error) abort building/sending the request instead
of continuing without the header. Update the code around the flb_http_add_header
invocation to test the return value, log an error, free/cleanup any
request/client resources as done on other failures, and return an
error/early-exit from the enclosing function (or propagate an error code) so the
request is not sent without the User-Agent header.
In `@plugins/out_http/http.c`:
- Around line 270-274: The default User-Agent header is being added after
append_headers which causes per-record User-Agent values to be replaced or
duplicated; move the flb_http_add_header call that adds
FLB_HTTP_HEADER_USER_AGENT_DEFAULT to occur before append_headers is invoked so
that any user-supplied User-Agent in append_headers can override it (ensure this
change respects existing allow_duplicated_headers logic and update any
surrounding comments accordingly).
In `@plugins/out_td/td.c`:
- Around line 180-184: Check the return value of flb_http_add_header when adding
FLB_HTTP_HEADER_USER_AGENT/FLB_HTTP_HEADER_USER_AGENT_DEFAULT; if it fails, log
an error mentioning the header insertion failed, perform necessary cleanup of
the HTTP request/context (free any allocated request object and related
resources), and short‑circuit by returning an error code from the surrounding
function instead of proceeding with the request.
🧹 Nitpick comments (1)
plugins/out_es/es.c (1)
65-69: Avoid magic length for the AWS UA value.
Consider usingsizeof("aws-fluent-bit-plugin") - 1to keep the length in sync with the literal.Suggested tweak
flb_http_add_header(c, FLB_HTTP_HEADER_USER_AGENT, sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1, - "aws-fluent-bit-plugin", 21); + "aws-fluent-bit-plugin", + sizeof("aws-fluent-bit-plugin") - 1);
| /* User-Agent */ | ||
| flb_http_add_header(client, | ||
| FLB_HTTP_HEADER_USER_AGENT, | ||
| sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1, | ||
| FLB_HTTP_HEADER_USER_AGENT_DEFAULT, | ||
| sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1); |
There was a problem hiding this comment.
Check the return value to avoid silently skipping the User-Agent.
If flb_http_add_header fails (e.g., allocation failure), the request proceeds without the header, which can reintroduce the 418 behavior this PR aims to fix. Consider aborting the request on failure.
Suggested fix
- flb_http_add_header(client,
- FLB_HTTP_HEADER_USER_AGENT,
- sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1,
- FLB_HTTP_HEADER_USER_AGENT_DEFAULT,
- sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1);
+ if (flb_http_add_header(client,
+ FLB_HTTP_HEADER_USER_AGENT,
+ sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1,
+ FLB_HTTP_HEADER_USER_AGENT_DEFAULT,
+ sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1) < 0) {
+ flb_plg_error(ins, "failed to add User-Agent header");
+ goto http_error;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* User-Agent */ | |
| flb_http_add_header(client, | |
| FLB_HTTP_HEADER_USER_AGENT, | |
| sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1, | |
| FLB_HTTP_HEADER_USER_AGENT_DEFAULT, | |
| sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1); | |
| /* User-Agent */ | |
| if (flb_http_add_header(client, | |
| FLB_HTTP_HEADER_USER_AGENT, | |
| sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1, | |
| FLB_HTTP_HEADER_USER_AGENT_DEFAULT, | |
| sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1) < 0) { | |
| flb_plg_error(ins, "failed to add User-Agent header"); | |
| goto http_error; | |
| } |
🧰 Tools
🪛 Cppcheck (2.19.0)
[error] 140-140: There is an unknown macro here somewhere. Configuration is required. If FLB_VERSION_STR is a macro then please configure it.
(unknownMacro)
🤖 Prompt for AI Agents
In `@plugins/in_nginx_exporter_metrics/nginx.c` around lines 136 - 141, The call
to flb_http_add_header(…, FLB_HTTP_HEADER_USER_AGENT, …) must have its return
value checked and handled; if it fails (e.g., allocation error) abort
building/sending the request instead of continuing without the header. Update
the code around the flb_http_add_header invocation to test the return value, log
an error, free/cleanup any request/client resources as done on other failures,
and return an error/early-exit from the enclosing function (or propagate an
error code) so the request is not sent without the User-Agent header.
| /* User-Agent */ | ||
| flb_http_add_header(c, FLB_HTTP_HEADER_USER_AGENT, | ||
| sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1, | ||
| FLB_HTTP_HEADER_USER_AGENT_DEFAULT, | ||
| sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1); |
There was a problem hiding this comment.
Handle failure from flb_http_add_header instead of ignoring it.
If header insertion fails (e.g., OOM), the request proceeds without a User‑Agent and defeats the goal. Consider logging and short‑circuiting with cleanup.
✅ Suggested fix (error handling + cleanup)
/* User-Agent */
- flb_http_add_header(c, FLB_HTTP_HEADER_USER_AGENT,
- sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1,
- FLB_HTTP_HEADER_USER_AGENT_DEFAULT,
- sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1);
+ ret = flb_http_add_header(c, FLB_HTTP_HEADER_USER_AGENT,
+ sizeof(FLB_HTTP_HEADER_USER_AGENT) - 1,
+ FLB_HTTP_HEADER_USER_AGENT_DEFAULT,
+ sizeof(FLB_HTTP_HEADER_USER_AGENT_DEFAULT) - 1);
+ if (ret != 0) {
+ flb_plg_error(ctx->ins, "failed to add User-Agent header");
+ flb_free(pack);
+ flb_free(body);
+ flb_http_client_destroy(c);
+ flb_upstream_conn_release(u_conn);
+ FLB_OUTPUT_RETURN(FLB_RETRY);
+ }🧰 Tools
🪛 Cppcheck (2.19.0)
[error] 183-183: There is an unknown macro here somewhere. Configuration is required. If FLB_VERSION_STR is a macro then please configure it.
(unknownMacro)
🤖 Prompt for AI Agents
In `@plugins/out_td/td.c` around lines 180 - 184, Check the return value of
flb_http_add_header when adding
FLB_HTTP_HEADER_USER_AGENT/FLB_HTTP_HEADER_USER_AGENT_DEFAULT; if it fails, log
an error mentioning the header insertion failed, perform necessary cleanup of
the HTTP request/context (free any allocated request object and related
resources), and short‑circuit by returning an error code from the surrounding
function instead of proceeding with the request.
d2ca797 to
249922d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/out_websocket/websocket.c`:
- Around line 80-85: The User-Agent default header is being added after the loop
that adds user-supplied headers (via the existing headers iteration in
websocket.c), which prevents users from overriding it when header deduplication
(allow_dup_headers) is disabled; move the flb_http_add_header(...) call that
adds FLB_HTTP_HEADER_USER_AGENT_DEFAULT so it executes before the
user-configured headers loop (i.e., place the flb_http_add_header(User-Agent...)
immediately before the code that iterates and adds user headers) to match the
ordering used in
loki.c/influxdb.c/opentelemetry.c/prometheus_remote_write.c/http.c/datadog.c and
allow user headers to override the default.
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
Signed-off-by: Enderson Maia <endersonmaia@gmail.com>
cb0844b to
e90dcce
Compare
braydonk
left a comment
There was a problem hiding this comment.
out_stackdriver changes LGTM
This will implement a standard User-Agent header for all http requests.
For ex.:
Ideally, I'd like to have something like this with os/arch and plugin information:
Tell me if I should work on something like that for this PR or another.
Fixes #8880
The issue was closed beacause it was stale. Please open it please.
Enter
[N/A]in the box, if an item is not applicable to your change.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