out_es: add support for dynamic index using record accessor patterns#11942
out_es: add support for dynamic index using record accessor patterns#11942SuRyaMane389 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds record-accessor-based dynamic index selection to the Elasticsearch output: config compiles an index accessor when the index contains ChangesDynamic index selection via record accessors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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: 958eee31b6
ℹ️ 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".
| if (!ra_index) { | ||
| flb_plg_warn(ctx->ins, | ||
| "invalid index translation from record accessor pattern, default to static index"); |
There was a problem hiding this comment.
Fall back when the index accessor resolves empty
When a record does not contain the configured accessor key, flb_ra_translate() is called with check disabled and returns an allocated empty SDS rather than NULL; this if (!ra_index) check is therefore skipped and the bulk metadata is emitted with _index":"" instead of using a safe fallback. Any mixed stream where one record lacks myindex will produce an Elasticsearch bulk item that the server rejects, so this should either use flb_ra_translate_check() or treat zero-length translations as missing.
Useful? React with 👍 / 👎.
| ctx->index, &tm); | ||
| es_index = index_formatted; | ||
| } | ||
| else if (ctx->ra_index) { |
There was a problem hiding this comment.
Resolve record-accessor indexes before current-time formatting
With both Index $myindex and Current_Time_Index On, this new ra_index branch is never reached because the preceding current_time_index branch formats the literal ctx->index string. That means otherwise valid dynamic-index configurations that already enable current_time_index send _index":"$myindex" for every record instead of the per-record value.
Useful? React with 👍 / 👎.
| TEST_CHECK((c >= 'A' && c <= 'F') || | ||
| (c == '-') || | ||
| (c >= '0' && c <= '9')); |
There was a problem hiding this comment.
Accept lowercase generated IDs in the new runtime test
The formatter generates IDs with %04x, which emits lowercase hexadecimal a-f, but this new check only permits uppercase A-F, digits, and dashes. As soon as the hash contains any lowercase hex letter, index_record_accessor_generate_id fails even though the generated _id is valid, breaking the added runtime coverage.
Useful? React with 👍 / 👎.
…tor index check logic Signed-off-by: Author Name <surya..mane@thive.io>
This PR adds dynamic Elasticsearch index resolution in out_es using record accessor patterns on the index field (example: $myindex), enabling per-record index routing without duplicating output sections.
What changed:
Definitions:
Architectural impact:
Example behavior:
Example configuration file:
Issue:
[N/A]
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.
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
Tests