Conversation
Signed-off-by: BOSLET, CORY <cb645j+ATT@att.com>
📝 WalkthroughWalkthroughAdds configuration and runtime logic to promote configured message-body keys into OpenTelemetry OTLP resource attributes, with init-time validation and per-record extraction during log flushes. Changes
Sequence Diagram(s)sequenceDiagram
participant FluentBit
participant ConfigLoader as Config (init)
participant OTLPPlugin as Plugin
participant OTLPExporter as Collector
FluentBit->>ConfigLoader: load `logs_resource_attributes_message_key`
ConfigLoader->>OTLPPlugin: populate ra_resource_attributes_message (validated)
FluentBit->>OTLPPlugin: emit log batch (MSGPACK)
OTLPPlugin->>OTLPPlugin: parse message body, lookup configured keys
OTLPPlugin->>OTLPPlugin: convert values -> OTLP KeyValue, append to resource.attributes
OTLPPlugin->>OTLPExporter: send OTLP resource logs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
This is related to issue #11491 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07e1ed1c54
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/out_opentelemetry/opentelemetry_conf.c (1)
186-187: Consider renamingconfig_log_bodyto match actual responsibility.This function currently handles
add_labelparsing and label KV setup, so the name is misleading.💡 Suggested rename for clarity
-static int config_log_body(struct flb_output_instance *ins, - struct opentelemetry_context *ctx) +static int config_add_labels(struct flb_output_instance *ins, + struct opentelemetry_context *ctx) @@ - ret = config_log_body(ins, ctx); + ret = config_add_labels(ins, ctx);Also applies to: 332-333
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/out_opentelemetry/opentelemetry_conf.c` around lines 186 - 187, The function name config_log_body is misleading because it parses add_label and sets up label key/value pairs; rename the function to something descriptive like config_parse_labels (or config_setup_labels) and update its declaration/definition and all call sites that reference config_log_body, including the second occurrence referenced at lines 332-333; ensure the opentelemetry_context parameter and any uses of add_label, label KVs, and related symbols are preserved, update any forward declarations or header prototypes, and adjust comments/documentation to match the new name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/out_opentelemetry/opentelemetry_logs.c`:
- Around line 1218-1219: The call to
set_resource_attributes_from_message_body(ctx, event.body,
resource_log->resource) is being run only during resource initialization so
later records in the same resource never get their configured keys promoted;
move this promotion into the per-record processing path where each event.body is
handled (instead of only in the resource init code), invoking
set_resource_attributes_from_message_body with the current ctx and event.body
against resource_log->resource for every record, and make the promotion
idempotent (do not overwrite existing resource attributes unless intended) so
repeated promotions are safe.
- Around line 793-862: The code currently appends promoted keys into
resource->attributes unconditionally, causing duplicates; after creating attr
via msgpack_kv_to_otlp_any_value and before calling flb_realloc, scan the
existing resource->attributes (resource->n_attributes entries) and compare their
key strings to attr->key (e.g., strcmp(resource->attributes[i]->key,
attr->key)); if a match is found, call otlp_kvpair_destroy(attr) and skip the
append (break/continue out of promotion loop) to avoid reallocating and adding a
duplicate; if no match, proceed with the existing flb_realloc / assignment
logic.
---
Nitpick comments:
In `@plugins/out_opentelemetry/opentelemetry_conf.c`:
- Around line 186-187: The function name config_log_body is misleading because
it parses add_label and sets up label key/value pairs; rename the function to
something descriptive like config_parse_labels (or config_setup_labels) and
update its declaration/definition and all call sites that reference
config_log_body, including the second occurrence referenced at lines 332-333;
ensure the opentelemetry_context parameter and any uses of add_label, label KVs,
and related symbols are preserved, update any forward declarations or header
prototypes, and adjust comments/documentation to match the new name.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
plugins/out_opentelemetry/opentelemetry.cplugins/out_opentelemetry/opentelemetry.hplugins/out_opentelemetry/opentelemetry_conf.cplugins/out_opentelemetry/opentelemetry_logs.c
| int i; | ||
| size_t key_len; | ||
| struct mk_list *head; | ||
| struct flb_config_map_val *mv; | ||
| struct flb_slist_entry *entry; | ||
| msgpack_object_kv *kv; | ||
| Opentelemetry__Proto__Common__V1__KeyValue *attr; | ||
| Opentelemetry__Proto__Common__V1__KeyValue **tmp_attrs; | ||
|
|
||
| /* | ||
| * Use ctx->ra_resource_attributes_message directly — this is the pointer | ||
| * managed by the Fluent Bit config-map framework and is reliably available | ||
| * in every worker thread context, unlike an embedded mk_list copy. | ||
| */ | ||
| if (!ctx->ra_resource_attributes_message || | ||
| mk_list_size(ctx->ra_resource_attributes_message) == 0) { | ||
| return; | ||
| } | ||
|
|
||
| if (body == NULL || body->type != MSGPACK_OBJECT_MAP) { | ||
| return; | ||
| } | ||
|
|
||
| /* Iterate directly over the config-map-managed list */ | ||
| flb_config_map_foreach(head, mv, ctx->ra_resource_attributes_message) { | ||
| if (mk_list_size(mv->val.list) != 1) { | ||
| continue; | ||
| } | ||
|
|
||
| entry = mk_list_entry_first(mv->val.list, struct flb_slist_entry, _head); | ||
| key_len = flb_sds_len(entry->str); | ||
|
|
||
| for (i = 0; i < body->via.map.size; i++) { | ||
| kv = &body->via.map.ptr[i]; | ||
|
|
||
| if (kv->key.type != MSGPACK_OBJECT_STR) { | ||
| continue; | ||
| } | ||
|
|
||
| if (kv->key.via.str.size != key_len) { | ||
| continue; | ||
| } | ||
|
|
||
| if (strncmp(kv->key.via.str.ptr, entry->str, key_len) != 0) { | ||
| continue; | ||
| } | ||
|
|
||
| /* Found the key — convert to OTLP KeyValue */ | ||
| attr = msgpack_kv_to_otlp_any_value(kv); | ||
| if (!attr) { | ||
| flb_plg_warn(ctx->ins, "resource attributes: failed to convert key '%s' to OTLP KeyValue", | ||
| entry->str); | ||
| break; | ||
| } | ||
|
|
||
| /* Grow the resource attributes array by one slot */ | ||
| tmp_attrs = flb_realloc(resource->attributes, | ||
| (resource->n_attributes + 1) * | ||
| sizeof(Opentelemetry__Proto__Common__V1__KeyValue *)); | ||
| if (!tmp_attrs) { | ||
| flb_plg_error(ctx->ins, "resource attributes: memory allocation failed for key '%s'", | ||
| entry->str); | ||
| otlp_kvpair_destroy(attr); | ||
| break; | ||
| } | ||
|
|
||
| resource->attributes = tmp_attrs; | ||
| resource->attributes[resource->n_attributes] = attr; | ||
| resource->n_attributes++; | ||
| break; |
There was a problem hiding this comment.
Prevent duplicate resource-attribute keys during promotion.
The helper appends promoted keys unconditionally. If a key already exists in resource->attributes (e.g., from $resource['attributes']), you can emit duplicate keys for the same resource.
💡 Suggested fix (skip already-present keys before append)
static void set_resource_attributes_from_message_body(
struct opentelemetry_context *ctx,
msgpack_object *body,
Opentelemetry__Proto__Resource__V1__Resource *resource)
{
int i;
+ size_t j;
+ int duplicate;
size_t key_len;
@@
if (strncmp(kv->key.via.str.ptr, entry->str, key_len) != 0) {
continue;
}
+ duplicate = FLB_FALSE;
+ for (j = 0; j < resource->n_attributes; j++) {
+ if (resource->attributes[j] != NULL &&
+ resource->attributes[j]->key != NULL &&
+ strcmp(resource->attributes[j]->key, entry->str) == 0) {
+ duplicate = FLB_TRUE;
+ break;
+ }
+ }
+
+ if (duplicate == FLB_TRUE) {
+ break;
+ }
+
/* Found the key — convert to OTLP KeyValue */
attr = msgpack_kv_to_otlp_any_value(kv);
if (!attr) {
flb_plg_warn(ctx->ins, "resource attributes: failed to convert key '%s' to OTLP KeyValue",
entry->str);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/out_opentelemetry/opentelemetry_logs.c` around lines 793 - 862, The
code currently appends promoted keys into resource->attributes unconditionally,
causing duplicates; after creating attr via msgpack_kv_to_otlp_any_value and
before calling flb_realloc, scan the existing resource->attributes
(resource->n_attributes entries) and compare their key strings to attr->key
(e.g., strcmp(resource->attributes[i]->key, attr->key)); if a match is found,
call otlp_kvpair_destroy(attr) and skip the append (break/continue out of
promotion loop) to avoid reallocating and adding a duplicate; if no match,
proceed with the existing flb_realloc / assignment logic.
There was a problem hiding this comment.
@cb645j, sounds good! The suggested diff should cover it — scan resource->attributes[0..n_attributes-1] and compare each .key against attr->key (via strcmp) before calling flb_realloc. If a match is found, call otlp_kvpair_destroy(attr) and break to skip the append. Let me know if you run into anything while implementing it.
|
@edsiper can you please check this |
|
@patrick-stephens pls review |
|
link to docs |
|
@cosmo0920 can you please review |
We need to handle coderabbitai's reviews at first. |
Signed-off-by: BOSLET, CORY <cb645j+ATT@att.com>
i have made updates based off coderabbitai. Can you please check now. How do we get the PR checks to run? |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/out_opentelemetry/opentelemetry_conf.c (1)
187-188: Function nameconfig_log_bodyis misleading.This function processes
add_labelconfiguration and populatesctx->kv_labels, but its name suggests it handles log body configuration. The comment on line 332 (/* Parse 'add_label' */) also conflicts with the function name.Consider renaming to something more descriptive like
config_add_labelsorconfig_kv_labelsto match its actual purpose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/out_opentelemetry/opentelemetry_conf.c` around lines 187 - 188, Rename the misleading static function config_log_body to a descriptive name like config_add_labels (or config_kv_labels) because it parses the 'add_label' configuration and populates ctx->kv_labels; update the function definition, any prototype/declaration, and every call site to use the new name while keeping the static linkage, and also update the nearby comment ("/* Parse 'add_label' */") to match the new function name so identifiers (config_log_body, ctx->kv_labels) are no longer inconsistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/out_opentelemetry/opentelemetry_conf.c`:
- Around line 187-188: Rename the misleading static function config_log_body to
a descriptive name like config_add_labels (or config_kv_labels) because it
parses the 'add_label' configuration and populates ctx->kv_labels; update the
function definition, any prototype/declaration, and every call site to use the
new name while keeping the static linkage, and also update the nearby comment
("/* Parse 'add_label' */") to match the new function name so identifiers
(config_log_body, ctx->kv_labels) are no longer inconsistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 125eefb4-5c22-4a49-ad33-0bc7e465a6f8
📒 Files selected for processing (2)
plugins/out_opentelemetry/opentelemetry_conf.cplugins/out_opentelemetry/opentelemetry_logs.c
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/out_opentelemetry/opentelemetry_logs.c
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
Example Configuration
Debug log ouput
[2026/03/02 15:30:38.171218000] [debug] [output:stdout:stdout.0] task_id=0 assigned to thread #0
{"date":1772483437.169789,"time":"2026-03-02T15:35:24.315873000Z","severity":"debug","severity_number":"7","msg":"Dev pr-11510 test2","subsys":"identity-cache-cell","deployment.environment.name":"dev","service.name":"my-app-log-svc5","application":"TESTAPP-19999"}
Documentation
Otel Log Data Model fields now supported:
Valgrind
coming soon