-
Notifications
You must be signed in to change notification settings - Fork 1.8k
out_es: custom http headers #9416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
out_es: custom http headers #9416
Conversation
7f83e4e to
8bd6ab8
Compare
|
@PettitWesley @edsiper any news on this ? |
…mple from docs as custom headers are not supported by elasticsearch output till fluent/fluent-bit#9416 is merged and released
…m examples (#7093) removed FluentBit Elasticsearch example from docs as custom headers are not supported by elasticsearch output till fluent/fluent-bit#9416 is merged and released fixes https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6985 ### Describe Your Changes Please provide a brief description of the changes you made. Be as specific as possible to help others understand the purpose and impact of your modifications. ### Checklist The following checks are **mandatory**: - [ ] My change adheres [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/contributing/).
…m examples (#7093) removed FluentBit Elasticsearch example from docs as custom headers are not supported by elasticsearch output till fluent/fluent-bit#9416 is merged and released fixes https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6985 ### Describe Your Changes Please provide a brief description of the changes you made. Be as specific as possible to help others understand the purpose and impact of your modifications. ### Checklist The following checks are **mandatory**: - [ ] My change adheres [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/contributing/). (cherry picked from commit f934f71)
Signed-off-by: Andrii Chubatiuk <[email protected]>
8bd6ab8 to
216a70b
Compare
|
hey @edsiper |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
Is there anything we can do to get this included in a future release? |
…m examples (VictoriaMetrics#7093) removed FluentBit Elasticsearch example from docs as custom headers are not supported by elasticsearch output till fluent/fluent-bit#9416 is merged and released fixes https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6985 ### Describe Your Changes Please provide a brief description of the changes you made. Be as specific as possible to help others understand the purpose and impact of your modifications. ### Checklist The following checks are **mandatory**: - [ ] My change adheres [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/contributing/).
…m examples (VictoriaMetrics#7093) removed FluentBit Elasticsearch example from docs as custom headers are not supported by elasticsearch output till fluent/fluent-bit#9416 is merged and released fixes https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6985 ### Describe Your Changes Please provide a brief description of the changes you made. Be as specific as possible to help others understand the purpose and impact of your modifications. ### Checklist The following checks are **mandatory**: - [ ] My change adheres [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/contributing/).
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Signed-off-by: Andrii Chubatiuk <[email protected]>
WalkthroughAdds support for user-defined HTTP headers in the Elasticsearch output plugin. Introduces a headers field in the context, config map entry for multiple header pairs, and iterates configured headers to inject them into HTTP requests during flush and initial request setup. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FB as Fluent Bit
participant ES as ES Output Plugin
participant HC as HTTP Client
participant ELS as Elasticsearch
FB->>ES: flush(cb_es_flush)
Note over ES: Load config (ctx), incl. headers list
ES->>HC: build request (URI, payload)
loop For each configured header
ES->>HC: addHeader(key, value)
end
HC->>ELS: HTTP request with headers
ELS-->>HC: HTTP response
HC-->>ES: deliver response
ES-->>FB: flush result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/out_es/es.c (1)
890-898: Validate header entries and handle flb_http_add_header failures
- Check the return value of flb_http_add_header and log or abort on -1 to avoid silent header‐addition errors.
- Before calling flb_http_add_header, verify mv->val.list contains exactly two entries (key + value); reject or warn on single‐token configs to prevent “X: X” headers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/out_es/es.c(3 hunks)plugins/out_es/es.h(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_es/es.c (1)
src/flb_http_client.c (1)
flb_http_add_header(963-995)
⏰ 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 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: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- 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_COVERAGE=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=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_SMALL=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_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 (-DSANITIZE_UNDEFINED=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 (-DFLB_SMALL=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, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=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 (-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=Off, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, 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-24.04, clang-14)
- 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-compile-centos-7
🔇 Additional comments (3)
plugins/out_es/es.h (1)
54-56: LGTM! Headers field added correctly.The
headersfield is properly declared as a pointer tostruct mk_listand is appropriately positioned with other HTTP-related configuration fields. This aligns with the config map entry in es.c that usesFLB_CONFIG_MAP_SLIST_1.plugins/out_es/es.c (2)
828-832: LGTM! Variable declarations are appropriate.The new variables (
head,mv,key,val) are correctly typed for iterating over the headers configuration usingflb_config_map_foreach.
1143-1148: LGTM! Configuration entry properly defined.The config map entry correctly uses
FLB_CONFIG_MAP_SLIST_1for key/value pairs withFLB_CONFIG_MAP_MULTto allow multiple headers. The description clearly communicates the functionality.
|
@patrick-stephens can you review this PR? |
|
Looks fine to me although I was expecting us to just use the HTTP plugin configuration for this, plus we probably want to replicate it to Opensearch if not there already. |
Added ability to set custom HTTP headers for Elasticsearch output
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
fluent/fluent-bit-docs#1390
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