in_opentelemetry: Configurable logs_body_key by prashantv · Pull Request #10335 · fluent/fluent-bit

prashantv · 2025-05-14T21:45:17Z

Currently, the logs body is added to the record in multiple ways

If it's a MAP, then the key-value pairs are added to the record at the top-level.
For other types, it's added under the key "message" for OTLP Proto and "log" for OTLP JSON.

Support a way to always nest the logs body under a specific key for consistency, and to avoid possible clashes with other top-level keys in the record (e.g., __internal__).

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:

Example configuration file for the change
Debug log output from testing the change

[N/A] Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

[N/A] Run local packaging test showing all targets (including any new ones) build.
[N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

Documentation required for this feature

Backporting

Backport to latest stable release.

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.

niedbalski · 2025-05-15T09:19:26Z

    json = get_log_body(chunk, size);
    TEST_CHECK(json != NULL);

+    printf("cb_check_log_body: %s\n", json);


niedbalski

LGTM

cosmo0920 · 2025-05-15T14:29:02Z

I kicked CI runners. So, we'll wait for the result of CI tasks.

edsiper · 2025-05-15T15:15:40Z

running workflows...

leonardo-albertovich

Most of these requests are coding style "nitpicks" so they all depend on my assessment of the conditional in line 1503, if I'm wrong about that you can feel free to take those comments as guidelines for future contributions.

leonardo-albertovich · 2025-05-16T09:08:41Z

                        ret = FLB_EVENT_ENCODER_ERROR_SERIALIZATION_FAILURE;
                    }
                    else {
-                        if (log_records[log_record_index]->body->value_case ==


I think this conditio modification changes the logic in an unexpected way according to a quick truth table I put together :

logs_body_key value_case branch_taken NULL NULL else NULL PTR main PTR NULL else PTR PTR else

Whereas the pervious logic results in :

logs_body_key value_case branch_taken NULL NULL else NULL PTR main PTR NULL else PTR PTR main

With the result being that if logs_body_key the value of value_case will be ignored regardless of its state.

I could be wrong so please don't hesitate to correct me if that's the case.

Ah, I overlooked this conditional change.
The added test case is not affected for just eliminating ctx->logs_body_key == NULL in the first if branch.
So, do we only need to handle logs_body_key setting if OPENTELEMETRY__PROTO__COMMON__V1__ANY_VALUE__VALUE_KVLIST_VALUE typed records case?

I'm a dummy, I focused on the wrong thing, now that I look at it again I can see that it's correct.

leonardo-albertovich · 2025-05-16T09:12:02Z

    msgpack_object_map *log_records_entry;
    msgpack_object     *timestamp_object;
    uint64_t            timestamp_uint64;
+    char *logs_body_key;


If the comment from line 1503 is correct and you do make changes to the code please adapt this line so it follows the predominant style in the function.

You don't need to follow this style in your own code but when you do modify pre-existing code please try to retain the style to keep things tidy and readable, not because one is better than the other but because it's easier to read homogenous code.

Ah thanks, I missed that this block of variables was indented (originally had the variable declared elsewhere and moved it up).

leonardo-albertovich · 2025-05-16T09:14:14Z

 }

+static void flb_test_log_body() {
+    flb_test_logs_body_impl(NULL /* logs_body_key */, "{\"log\":\"{\\\"message\\\":\\\"test\\\"}\"}");


These comments are superfluous and don't make the code easier to understand, please remove them and omit them in the future.

Comments are valuable and should be included but only when they clearly make it easier for whoever comes next to understand the context and reasoning behind a specific piece of code.

Removed

(this is a Go habit of mine to help the reader know what argument a value like 1/true/NULL is used for)

leonardo-albertovich · 2025-05-16T09:15:35Z

        else {
+            logs_body_key = ctx->logs_body_key;
+            if (ctx->logs_body_key == NULL) {
+                logs_body_key = "log";


Even though it's technically not correct (or rather wise) in order to ensue that the compiler does not emit a warning over the loss of the const qualifier please add the appropriate cast here.

(char *) "log"

Thanks, added the (unsafe) cast.

Strangely, I didn't see a warning on this line, so tried to reproduce it in a separate simple program, and I see warnings when I do:

char *p; const char a[] = "hello world"; p = a; // warning that assignment discards `const` qualifier

however, if I use a fixed string, there's no warning,

char *p; p = "hello world";

(even though it's definitely unsafe, if I then later do p[0] = 'a', it seg faults)

leonardo-albertovich · 2025-05-16T09:55:21Z

                        ret = FLB_EVENT_ENCODER_ERROR_SERIALIZATION_FAILURE;
                    }
                    else {
-                        if (log_records[log_record_index]->body->value_case ==


I'm a dummy, I focused on the wrong thing, now that I look at it again I can see that it's correct.

prashantv

Appreciate the detailed reviews!

prashantv · 2025-05-16T13:22:31Z

    msgpack_object_map *log_records_entry;
    msgpack_object     *timestamp_object;
    uint64_t            timestamp_uint64;
+    char *logs_body_key;


Ah thanks, I missed that this block of variables was indented (originally had the variable declared elsewhere and moved it up).

prashantv · 2025-05-16T13:31:34Z

        else {
+            logs_body_key = ctx->logs_body_key;
+            if (ctx->logs_body_key == NULL) {
+                logs_body_key = "log";


Thanks, added the (unsafe) cast.

Strangely, I didn't see a warning on this line, so tried to reproduce it in a separate simple program, and I see warnings when I do:

char *p; const char a[] = "hello world"; p = a; // warning that assignment discards `const` qualifier

however, if I use a fixed string, there's no warning,

char *p; p = "hello world";

(even though it's definitely unsafe, if I then later do p[0] = 'a', it seg faults)

prashantv · 2025-05-16T13:38:19Z

 }

+static void flb_test_log_body() {
+    flb_test_logs_body_impl(NULL /* logs_body_key */, "{\"log\":\"{\\\"message\\\":\\\"test\\\"}\"}");


Removed

(this is a Go habit of mine to help the reader know what argument a value like 1/true/NULL is used for)

edsiper · 2025-05-19T16:46:07Z

@prashantv last request: would you please make 2 commits ? one for the primary change in in_opentelemetry and the other for the test ?, e.g:

in_opentelemetry: ....
tests: runtime: in_opentelemetry: ...

Currently, the logs body is added to the record in multiple ways * If it's a MAP, then the key-value pairs are added to the record at the top-level. * For other types, it's added under the key "message" for OTLP Proto and "log" for OTLP JSON. Support a way to always nest the logs body under a specific key for consistency, and to avoid possible clashes with other top-level keys in the record (e.g., `__internal__`). Signed-off-by: Prashant Varanasi <prashant@chronosphere.io>

Signed-off-by: Prashant Varanasi <prashant@chronosphere.io>

github-actions Bot added the docs-required label May 14, 2025

prashantv force-pushed the prashant/input-otel-body branch from dfc56db to 6874c7c Compare May 14, 2025 23:44

niedbalski reviewed May 15, 2025

View reviewed changes

niedbalski approved these changes May 15, 2025

View reviewed changes

prashantv temporarily deployed to pr May 15, 2025 14:28 — with GitHub Actions Inactive

prashantv force-pushed the prashant/input-otel-body branch from bd19b2e to 08e06b4 Compare May 15, 2025 14:57

prashantv marked this pull request as ready for review May 15, 2025 14:58

prashantv requested review from edsiper, fujimotos, koleini and leonardo-albertovich as code owners May 15, 2025 14:58

prashantv had a problem deploying to pr May 15, 2025 15:15 — with GitHub Actions Failure

prashantv had a problem deploying to pr May 15, 2025 18:03 — with GitHub Actions Error

prashantv temporarily deployed to pr May 15, 2025 18:08 — with GitHub Actions Inactive

prashantv temporarily deployed to pr May 15, 2025 18:31 — with GitHub Actions Inactive

cosmo0920 approved these changes May 16, 2025

View reviewed changes

cosmo0920 added this to the Fluent Bit v4.0.2 milestone May 16, 2025

leonardo-albertovich suggested changes May 16, 2025

View reviewed changes

leonardo-albertovich approved these changes May 16, 2025

View reviewed changes

prashantv force-pushed the prashant/input-otel-body branch from 08e06b4 to 1c8cdf0 Compare May 16, 2025 13:45

prashantv commented May 16, 2025

View reviewed changes

leonardo-albertovich approved these changes May 16, 2025

View reviewed changes

prashantv temporarily deployed to pr May 19, 2025 14:32 — with GitHub Actions Inactive

prashantv temporarily deployed to pr May 19, 2025 14:56 — with GitHub Actions Inactive

edsiper modified the milestones: Fluent Bit v4.0.2, Fluent bit v4.0.3 May 19, 2025

prashantv force-pushed the prashant/input-otel-body branch from 1c8cdf0 to ff89352 Compare May 19, 2025 18:38

tests: runtime: in_opentelemetry: Test logs_body_key

d327e2a

Signed-off-by: Prashant Varanasi <prashant@chronosphere.io>

prashantv force-pushed the prashant/input-otel-body branch from ff89352 to d327e2a Compare May 19, 2025 18:40

prashantv temporarily deployed to pr May 19, 2025 18:42 — with GitHub Actions Inactive

prashantv temporarily deployed to pr May 19, 2025 19:04 — with GitHub Actions Inactive

edsiper merged commit 8177065 into fluent:master May 20, 2025

BrewTestBot mentioned this pull request May 30, 2025

fluent-bit 4.0.3 Homebrew/homebrew-core#225287

Merged

Uh oh!

Conversation

prashantv commented May 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niedbalski left a comment

Choose a reason for hiding this comment

Uh oh!

cosmo0920 commented May 15, 2025

Uh oh!

edsiper commented May 15, 2025

Uh oh!

leonardo-albertovich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prashantv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edsiper commented May 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants