Skip to content

record_accessor: single-pass keymap resolution #11432

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.

Sign up for GitHub

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

Closed
edsiper wants to merge 1 commit into from
+123 −26
Closed

record_accessor: single-pass keymap resolution #11432

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 123 additions & 26 deletions src/flb_record_accessor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,31 +522,22 @@ static flb_sds_t ra_translate_string(struct flb_ra_parser *rp, flb_sds_t buf)
return tmp;
}

static flb_sds_t ra_translate_keymap(struct flb_ra_parser *rp, flb_sds_t buf,
msgpack_object map, int *found)
/* Append string representation of a keymap value to buf. Does not take ownership of v. */
static flb_sds_t ra_append_keymap_value(struct flb_ra_parser *rp, flb_sds_t buf,
struct flb_ra_value *v, int *found)
{
int i;
int len;
char *js;
char str[32];
const char *p;
flb_sds_t hex = NULL;
struct flb_ra_value *v;

/* Lookup key or subkey value */
if (rp->key == NULL) {
*found = FLB_FALSE;
return buf;
}

v = flb_ra_key_to_value_ext(rp->key->name, map, rp->key->subkeys, FLB_TRUE);
if (!v) {
if (rp->key == NULL || v == NULL) {
*found = FLB_FALSE;
return buf;
}
else {
*found = FLB_TRUE;
}
*found = FLB_TRUE;

/* Based on data type, convert to it string representation */
if (v->type == FLB_RA_BOOL) {
Expand Down Expand Up @@ -614,6 +605,25 @@ static flb_sds_t ra_translate_keymap(struct flb_ra_parser *rp, flb_sds_t buf,
flb_sds_cat_safe(&buf, "null", 4);
}

return buf;
}

static flb_sds_t ra_translate_keymap(struct flb_ra_parser *rp, flb_sds_t buf,
msgpack_object map, int *found)
{
struct flb_ra_value *v;

if (rp->key == NULL) {
*found = FLB_FALSE;
return buf;
}

v = flb_ra_key_to_value_ext(rp->key->name, map, rp->key->subkeys, FLB_TRUE);
if (!v) {
*found = FLB_FALSE;
return buf;
}
buf = ra_append_keymap_value(rp, buf, v, found);
flb_ra_key_value_destroy(v);
return buf;
}
Expand All @@ -633,41 +643,115 @@ flb_sds_t flb_ra_translate(struct flb_record_accessor *ra,
}

/*
* Translate a record accessor buffer, tag and records are optional
* parameters.
*
* For safety, the function returns a newly created string that needs
* to be destroyed by the caller.
*
* Returns NULL if `check` is FLB_TRUE and any key lookup in the record failed
* Optimized version: single pass over map to resolve all KEYMAP lookups,
* then translate using cached values. Same semantics as flb_ra_translate_check.
*/
flb_sds_t flb_ra_translate_check(struct flb_record_accessor *ra,
char *tag, int tag_len,
msgpack_object map, struct flb_regex_search *result,
int check)
{
int i;
int keymap_idx;
int keymap_count;
int map_size;
flb_sds_t tmp = NULL;
flb_sds_t buf;
struct mk_list *head;
struct flb_ra_parser *rp;
int found = FLB_FALSE;
msgpack_object key;
msgpack_object val;
struct flb_ra_value **keymap_cache = NULL;

/* Count KEYMAP entries to decide whether to use single-pass cache */
keymap_count = 0;
mk_list_foreach(head, &ra->list) {
rp = mk_list_entry(head, struct flb_ra_parser, _head);
if (rp->type == FLB_RA_PARSER_KEYMAP) {
keymap_count++;
}
}

/* Single pass over map: resolve all KEYMAP lookups at once */
if (keymap_count > 0 && map.type == MSGPACK_OBJECT_MAP) {
keymap_cache = flb_calloc((size_t) keymap_count, sizeof(struct flb_ra_value *));
if (keymap_cache) {
map_size = map.via.map.size;
for (i = 0; i < map_size; i++) {
key = map.via.map.ptr[i].key;
val = map.via.map.ptr[i].val;
if (key.type != MSGPACK_OBJECT_STR) {
continue;
}
keymap_idx = 0;
mk_list_foreach(head, &ra->list) {
rp = mk_list_entry(head, struct flb_ra_parser, _head);
if (rp->type != FLB_RA_PARSER_KEYMAP) {
continue;
}
if (rp->key == NULL) {
keymap_idx++;
continue;
}
if (flb_sds_cmp(rp->key->name, key.via.str.ptr,
key.via.str.size) != 0) {
keymap_idx++;
continue;
}
if (keymap_cache[keymap_idx]) {
flb_ra_key_value_destroy(keymap_cache[keymap_idx]);
}
keymap_cache[keymap_idx] = flb_ra_value_from_object(&val,
rp->key->subkeys,
FLB_TRUE);
Comment on lines +705 to +707
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Define or include flb_ra_value_from_object

The new code calls flb_ra_value_from_object, but there is no definition or declaration for this symbol anywhere in the repository (repo-wide search only finds this call). That means the build will fail with an implicit-declaration error or undefined symbol at link time for any configuration that compiles flb_record_accessor.c. If this function is intended to be added, it needs to be implemented and declared in the appropriate header (likely alongside other flb_ra_* helpers).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@cosmo0920 cosmo0920 Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edsiper Could you take a look on this?
We didn't have a definition of flb_ra_value_from_object in our code base in master branch.

Copy link
Copy Markdown
Member Author

@edsiper edsiper Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cosmo0920 thanks, I did a partial commit, need to repush today, thanks

Comment on lines +705 to +707
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for function declaration in headers
echo "=== Checking header declaration ==="
fd -e h -e hpp | xargs rg -l 'flb_ra_value_from_object' 2>/dev/null | head -10
echo ""
rg -A3 'flb_ra_value_from_object' --type=c --type=h | head -50

echo ""
echo "=== Checking implementation in source files ==="
fd -e c | xargs grep -l 'flb_ra_value_from_object' 2>/dev/null | head -10

Repository: fluent/fluent-bit

Length of output: 561

🏁 Script executed:

#!/bin/bash
# Search for the actual definition of flb_ra_value_from_object in the file
echo "=== Searching for function definition in src/flb_record_accessor.c ==="
rg -n 'flb_ra_value_from_object' src/flb_record_accessor.c

echo ""
echo "=== Looking for function signature (expanded context) ==="
rg -B5 -A10 'flb_ra_value_from_object' src/flb_record_accessor.c | head -60

echo ""
echo "=== Check if it might be in flb_ra_key header/source ==="
fd -e h -e c | xargs grep -l 'flb_ra_key' | head -10
</parameter>

Repository: fluent/fluent-bit

Length of output: 1342

🏁 Script executed:

#!/bin/bash
# Search for function definition in flb_ra_key.c
echo "=== Searching for function definition in src/flb_ra_key.c ==="
rg -n 'flb_ra_value_from_object' src/flb_ra_key.c

echo ""
echo "=== Looking for flb_ra_key header files ==="
fd -e h | xargs grep -l 'flb_ra_key' | head -10

echo ""
echo "=== Search for the function in all header files ==="
fd -e h | xargs grep -n 'flb_ra_value_from_object' 2>/dev/null

echo ""
echo "=== Check if function is defined in src/flb_ra_key.c ==="
rg -B3 -A15 'flb_ra_value_from_object' src/flb_ra_key.c

Repository: fluent/fluent-bit

Length of output: 466

🏁 Script executed:

#!/bin/bash
# Broader search across entire codebase
echo "=== Broad search for flb_ra_value_from_object ==="
rg 'flb_ra_value_from_object' -t c -t h

echo ""
echo "=== Check for similar function names ==="
rg 'flb_ra.*value.*object' -t c -t h

echo ""
echo "=== Check context around line 705 in detail ==="
sed -n '700,710p' src/flb_record_accessor.c

echo ""
echo "=== Look for possible macro definitions ==="
rg 'define.*flb_ra_value_from_object' -t c -t h

Repository: fluent/fluent-bit

Length of output: 8823

🏁 Script executed:

#!/bin/bash
# Check what's actually in the flb_ra_key.h header
echo "=== Contents of include/fluent-bit/flb_ra_key.h ==="
cat include/fluent-bit/flb_ra_key.h

echo ""
echo "=== Check flb_record_accessor.h for related functions ==="
grep -n 'flb_ra.*value' include/fluent-bit/flb_record_accessor.h | head -20

Repository: fluent/fluent-bit

Length of output: 3379

🏁 Script executed:

#!/bin/bash
# Check git log to see if this is new code
echo "=== Check git log for src/flb_record_accessor.c ==="
git log --oneline -n 10 -- src/flb_record_accessor.c

echo ""
echo "=== Check git log for src/flb_ra_key.c ==="
git log --oneline -n 10 -- src/flb_ra_key.c

echo ""
echo "=== Check if there's been a recent commit adding this function call ==="
git log -p --all -S 'flb_ra_value_from_object' -- src/flb_record_accessor.c | head -100

Repository: fluent/fluent-bit

Length of output: 341

🏁 Script executed:

#!/bin/bash
# Check the recent commit that added this code
echo "=== Check the commit that introduced the function call ==="
git show --stat 3548a164

echo ""
echo "=== Look at the full diff to see if function was added ==="
git show 3548a164 | head -200

Repository: fluent/fluent-bit

Length of output: 50374

🏁 Script executed:

#!/bin/bash
# Final verification - check if there's a typo or similar function name
echo "=== All functions in flb_ra_key.h header ==="
grep -n 'struct flb_ra_value\|^[a-z_]*(' include/fluent-bit/flb_ra_key.h | grep -E '^[0-9]+:.*\(' 

echo ""
echo "=== Functions actually defined in flb_ra_key.c ==="
rg -n '^(struct flb_ra_value|int|void) \w+\(' src/flb_ra_key.c | head -20

Repository: fluent/fluent-bit

Length of output: 820

flb_ra_value_from_object() is not implemented.

The function is called at lines 705–707 but is neither declared in the header nor defined in the source. The commit message promises this function ("Add flb_ra_value_from_object() in flb_ra_key for value-from-object") but the implementation is missing. This will cause a linker error (undefined reference).

A similar function exists—flb_ra_key_to_value_ext()—which takes (ckey, map, subkeys, copy) parameters and returns struct flb_ra_value *. Verify the intended function signature and add the missing implementation in src/flb_ra_key.c, then declare it in include/fluent-bit/flb_ra_key.h.

🤖 Prompt for AI Agents 
In `@src/flb_record_accessor.c` around lines 705 - 707, The call to
flb_ra_value_from_object is unresolved because the function is missing;
implement flb_ra_value_from_object in src/flb_ra_key.c (use the same semantics
as the existing flb_ra_key_to_value_ext — accept the object/map and subkeys and
a copy flag and return struct flb_ra_value *) and have it either perform the
conversion logic or delegate to flb_ra_key_to_value_ext, then add its prototype
to include/fluent-bit/flb_ra_key.h so the symbol is declared for callers (ensure
the implementation signature matches how it is invoked at the call site and
update any include or linkage if needed).

keymap_idx++;
}
}
}
}

buf = flb_sds_create_size(ra->size_hint);
if (!buf) {
flb_error("[record accessor] cannot create outgoing buffer");
if (keymap_cache) {
for (keymap_idx = 0; keymap_idx < keymap_count; keymap_idx++) {
if (keymap_cache[keymap_idx]) {
flb_ra_key_value_destroy(keymap_cache[keymap_idx]);
}
}
flb_free(keymap_cache);
}
return NULL;
}

keymap_idx = 0;
mk_list_foreach(head, &ra->list) {
rp = mk_list_entry(head, struct flb_ra_parser, _head);
if (rp->type == FLB_RA_PARSER_STRING) {
tmp = ra_translate_string(rp, buf);
}
else if (rp->type == FLB_RA_PARSER_KEYMAP) {
tmp = ra_translate_keymap(rp, buf, map, &found);
if (keymap_cache) {
tmp = ra_append_keymap_value(rp, buf,
keymap_cache[keymap_idx],
&found);
}
else {
tmp = ra_translate_keymap(rp, buf, map, &found);
}
keymap_idx++;
if (check == FLB_TRUE && found == FLB_FALSE) {
flb_warn("[record accessor] translation failed, root key=%s", rp->key->name);
flb_sds_destroy(buf);
if (keymap_cache) {
for (i = 0; i < keymap_count; i++) {
if (keymap_cache[i]) {
flb_ra_key_value_destroy(keymap_cache[i]);
}
}
flb_free(keymap_cache);
}
return NULL;
}
Comment on lines 744 to 756
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential NULL pointer dereference when logging warning.

If rp->key is NULL (handled at lines 536-539 and 616-619 which set *found = FLB_FALSE), and check == FLB_TRUE, the code will reach line 745 and dereference rp->key->name, causing a crash.

🐛 Proposed fix to guard against NULL key 
             keymap_idx++;
             if (check == FLB_TRUE && found == FLB_FALSE) {
-                flb_warn("[record accessor] translation failed, root key=%s", rp->key->name);
+                flb_warn("[record accessor] translation failed, root key=%s",
+                         rp->key ? rp->key->name : "(null)");
                 flb_sds_destroy(buf);
                 if (keymap_cache) {
📝 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.

Suggested change
if (check == FLB_TRUE && found == FLB_FALSE) {
flb_warn("[record accessor] translation failed, root key=%s", rp->key->name);
flb_sds_destroy(buf);
if (keymap_cache) {
for (i = 0; i < keymap_count; i++) {
if (keymap_cache[i]) {
flb_ra_key_value_destroy(keymap_cache[i]);
}
}
flb_free(keymap_cache);
}
return NULL;
}
if (check == FLB_TRUE && found == FLB_FALSE) {
flb_warn("[record accessor] translation failed, root key=%s",
rp->key ? rp->key->name : "(null)");
flb_sds_destroy(buf);
if (keymap_cache) {
for (i = 0; i < keymap_count; i++) {
if (keymap_cache[i]) {
flb_ra_key_value_destroy(keymap_cache[i]);
}
}
flb_free(keymap_cache);
}
return NULL;
}
🤖 Prompt for AI Agents 
In `@src/flb_record_accessor.c` around lines 744 - 756, The warning log
dereferences rp->key->name which can be NULL; update the block in
flb_record_accessor (the section checking if check == FLB_TRUE && found ==
FLB_FALSE) to guard against a NULL rp->key before accessing name — e.g., compute
a safe_key_name string from rp->key (or use a literal like "<null>") and use
that in flb_warn, or skip logging the name when rp->key is NULL; ensure the same
NULL-safe approach is applied before any use of rp->key in this failure branch
and preserve the existing cleanup of buf and keymap_cache.

}
Expand All @@ -681,20 +765,33 @@ flb_sds_t flb_ra_translate_check(struct flb_record_accessor *ra,
tmp = ra_translate_tag_part(rp, buf, tag, tag_len);
}

//else if (rp->type == FLB_RA_PARSER_FUNC) {
//tmp = ra_translate_func(rp, buf, tag, tag_len);
//}

if (!tmp) {
flb_error("[record accessor] translation failed");
flb_sds_destroy(buf);
if (keymap_cache) {
for (i = 0; i < keymap_count; i++) {
if (keymap_cache[i]) {
flb_ra_key_value_destroy(keymap_cache[i]);
}
}
flb_free(keymap_cache);
}
return NULL;
}
if (tmp != buf) {
buf = tmp;
}
}

if (keymap_cache) {
for (keymap_idx = 0; keymap_idx < keymap_count; keymap_idx++) {
if (keymap_cache[keymap_idx]) {
flb_ra_key_value_destroy(keymap_cache[keymap_idx]);
}
}
flb_free(keymap_cache);
}

return buf;
}

Expand Down
Loading