Skip to content

opentelemetry: logs: fix packaging length for trace_id and span_id#10767

Merged
edsiper merged 2 commits intomasterfrom
opentelemetry-logs-ids
Aug 21, 2025
Merged

opentelemetry: logs: fix packaging length for trace_id and span_id#10767
edsiper merged 2 commits intomasterfrom
opentelemetry-logs-ids

Conversation

@edsiper
Copy link
Member

@edsiper edsiper commented Aug 21, 2025

Fixes #10750


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

  • Bug Fixes

    • Corrected binary encoding of trace_id (16 bytes) and span_id (8 bytes) and emit them only when valid.
    • Prevented errors from malformed or non-string IDs by safely ignoring them.
    • Improved control flow to avoid crashes when IDs are missing or invalid.
  • Performance/Improvements

    • Reduced payload size by emitting correctly sized binary IDs.
    • More robust handling of OpenTelemetry log records.
  • Tests

    • Added a test validating binary sizes for trace_id and span_id via record accessors.

Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
@edsiper edsiper added this to the Fluent Bit v4.1 milestone Aug 21, 2025
@coderabbitai
Copy link

coderabbitai bot commented Aug 21, 2025

Walkthrough

Gates emission of trace_id/span_id to exact hex string lengths (32/16), converts to 16/8-byte binary respectively, and ignores invalid/non-string IDs without error. Updates tests to assert binary sizes via record accessors. Test list updated; note duplicate test definition/registration added.

Changes

Cohort / File(s) Summary
OTel ID conversion and emission
src/opentelemetry/flb_opentelemetry_logs.c
Add strict type/length checks before converting hex IDs; convert 32-char trace_id to 16-byte binary and 16-char span_id to 8-byte binary; relax error handling for invalid IDs; adjust binary value lengths accordingly.
Internal tests for ID sizes
tests/internal/opentelemetry.c
Add test_trace_span_binary_sizes validating 16/8-byte outputs via record accessors; include necessary headers; register test. Note: the test function and its registration are duplicated.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client as HTTP/JSON Client
  participant Ingest as HTTP Input
  participant Parser as OTLP JSON Parser
  participant IDProc as ID Validator/Converter
  participant Builder as Record Builder
  participant Export as gRPC OTLP Exporter

  Client->>Ingest: Send JSON (trace_id, span_id as hex strings)
  Ingest->>Parser: Pass JSON payload
  Parser->>IDProc: Provide trace_id/span_id values
  alt Valid strings with exact lengths (32/16)
    IDProc->>IDProc: Hex→binary (16/8 bytes)
    IDProc->>Builder: Set binary IDs (16/8 bytes)
  else Missing/invalid/non-string/length mismatch
    IDProc->>Builder: Skip IDs (no error)
  end
  Builder->>Export: Emit record with IDs (if present)
  Export-->>Client: Forward via gRPC
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective Addressed Explanation
Correct binary sizes for trace_id (16 bytes) and span_id (8 bytes) [#10750]
Ensure gRPC payload uses those exact lengths [#10750] Changes are in logs processing; no explicit gRPC payload verification present.
Add regression tests for HTTP/JSON → gRPC round-trip [#10750] Only unit test via record accessors; no gRPC round-trip test added.
Maintain compatibility with prior behavior (v3.2.10) [#10750] No comparative or compatibility checks provided.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
Duplicate test function and registration (tests/internal/opentelemetry.c) Duplication appears accidental; while related to the objective, the duplicate definition/registration is not required to meet any stated objective.

Suggested labels

backport to v4.0.x

Suggested reviewers

  • koleini
  • fujimotos
  • leonardo-albertovich

Poem

A nibble of hex, a byte of delight,
I twitch my ears at IDs made right—
Sixteen for traces, eight for the span,
No extra fluff in the binary plan.
I thump my paws, tests hop in line,
Bugs burrow out—everything’s fine! 🐇✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch opentelemetry-logs-ids

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/opentelemetry/flb_opentelemetry_logs.c (1)

213-221: Wrong return code on span_id hex validation failure.

On invalid span_id hex characters you set error_status = FLB_OTEL_LOGS_ERR_INVALID_SPAN_ID, but return -FLB_OTEL_LOGS_ERR_UNEXPECTED_TIMESTAMP_TYPE. This mismatch confuses callers and error reporting.

Apply this diff:

-                if (error_status) {
-                    *error_status = FLB_OTEL_LOGS_ERR_INVALID_SPAN_ID;
-                }
-                return -FLB_OTEL_LOGS_ERR_UNEXPECTED_TIMESTAMP_TYPE;
+                if (error_status) {
+                    *error_status = FLB_OTEL_LOGS_ERR_INVALID_SPAN_ID;
+                }
+                return -FLB_OTEL_LOGS_ERR_INVALID_SPAN_ID;
🧹 Nitpick comments (5)
tests/internal/opentelemetry.c (2)

765-767: Assert encoder error_status to catch silent validation failures.

We should verify error_status stays 0 after flb_opentelemetry_logs_json_to_msgpack to avoid passing when internal validation fails.

Apply this diff:

 ret = flb_opentelemetry_logs_json_to_msgpack(&enc, input_json, strlen(input_json), NULL, &error_status);
 TEST_CHECK(ret == 0);
+TEST_CHECK(error_status == 0);

757-757: Remove unused variable.

Variable ‘len’ is declared but never used.

Apply this diff:

-    size_t len;
src/opentelemetry/flb_opentelemetry_logs.c (3)

296-301: Correct trace_id packing: now 16 bytes (not 32).

This addresses the core bug where traceId was doubled in size. One small nit: check the conversion return for completeness, even though we validate the hex beforehand.

Apply this diff:

-    flb_otel_utils_hex_to_id(trace_id->via.str.ptr, trace_id->via.str.size, tmp_id, 16);
-    flb_log_event_encoder_append_metadata_values(encoder,
-                                                    FLB_LOG_EVENT_STRING_VALUE("trace_id", 8),
-                                                    FLB_LOG_EVENT_BINARY_VALUE(tmp_id, 16));
+    if (flb_otel_utils_hex_to_id(trace_id->via.str.ptr, trace_id->via.str.size, tmp_id, 16) == 0) {
+        flb_log_event_encoder_append_metadata_values(encoder,
+                                                     FLB_LOG_EVENT_STRING_VALUE("trace_id", 8),
+                                                     FLB_LOG_EVENT_BINARY_VALUE(tmp_id, 16));
+    }

303-308: Correct span_id packing: now 8 bytes (not 16).

Same nit as above: check the converter’s return for completeness.

Apply this diff:

-    flb_otel_utils_hex_to_id(span_id->via.str.ptr, span_id->via.str.size, tmp_id, 8);
-    flb_log_event_encoder_append_metadata_values(encoder,
-                                                    FLB_LOG_EVENT_STRING_VALUE("span_id", 7),
-                                                    FLB_LOG_EVENT_BINARY_VALUE(tmp_id, 8));
+    if (flb_otel_utils_hex_to_id(span_id->via.str.ptr, span_id->via.str.size, tmp_id, 8) == 0) {
+        flb_log_event_encoder_append_metadata_values(encoder,
+                                                     FLB_LOG_EVENT_STRING_VALUE("span_id", 7),
+                                                     FLB_LOG_EVENT_BINARY_VALUE(tmp_id, 8));
+    }

181-189: ctype usage: cast inputs to unsigned char for defined behavior.

isxdigit() requires an unsigned char value (or EOF). To avoid UB on non-ASCII inputs, cast arguments.

Apply this diff:

-    for (i = 0; i < 32; i++) {
-        if (!isxdigit(trace_id->via.str.ptr[i])) {
+    for (i = 0; i < 32; i++) {
+        if (!isxdigit((unsigned char) trace_id->via.str.ptr[i])) {
             ...
         }
     }
@@
-    for (i = 0; i < 16; i++) {
-        if (!isxdigit(span_id->via.str.ptr[i])) {
+    for (i = 0; i < 16; i++) {
+        if (!isxdigit((unsigned char) span_id->via.str.ptr[i])) {
             ...
         }
     }

Also applies to: 213-221

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d725599 and 665a6e9.

📒 Files selected for processing (2)
  • src/opentelemetry/flb_opentelemetry_logs.c (1 hunks)
  • tests/internal/opentelemetry.c (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/internal/opentelemetry.c (5)
src/flb_log_event_encoder.c (2)
  • flb_log_event_encoder_init (42-74)
  • flb_log_event_encoder_destroy (99-116)
src/opentelemetry/flb_opentelemetry_logs.c (1)
  • flb_opentelemetry_logs_json_to_msgpack (794-871)
src/flb_record_accessor.c (3)
  • flb_ra_create (271-358)
  • flb_ra_get_value_object (803-814)
  • flb_ra_destroy (232-248)
src/flb_log_event_decoder.c (5)
  • flb_log_event_decoder_init (99-116)
  • flb_log_event_decoder_read_groups (85-97)
  • flb_log_event_decoder_next (310-406)
  • flb_log_event_decoder_get_record_type (408-428)
  • flb_log_event_decoder_destroy (147-179)
src/flb_ra_key.c (1)
  • flb_ra_key_value_destroy (842-851)
⏰ 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). (28)
  • 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: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=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, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
  • 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, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=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 (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=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=Off, 3.31.6, clang, clang++)
  • 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 - fuzzing test
  • 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, gcc, g++, ubuntu-22.04, clang-12)
  • 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-22.04, clang-12)
  • GitHub Check: pr-compile-centos-7
🔇 Additional comments (2)
tests/internal/opentelemetry.c (2)

25-26: Record accessor headers are correctly added.

Needed for flb_ra_create()/flb_ra_get_value_object() usage in the new test. No issues.


836-836: Test is properly registered.

Name is descriptive and specific to the regression being fixed. Good.

Comment on lines +740 to +827
void test_trace_span_binary_sizes()
{
int ret;
struct flb_log_event_encoder enc;
struct flb_log_event_decoder dec;
struct flb_log_event event;
int32_t record_type;
char *input_json;
int error_status = 0;
int found_trace_id = 0;
int found_span_id = 0;
size_t trace_id_size = 0;
size_t span_id_size = 0;
struct flb_record_accessor *ra_trace_id;
struct flb_record_accessor *ra_span_id;
struct flb_ra_value *val_trace_id;
struct flb_ra_value *val_span_id;
size_t len;

/* Test input with trace_id and span_id */
input_json = "{\"resourceLogs\":[{\"scopeLogs\":[{\"logRecords\":[{\"timeUnixNano\":\"1640995200000000000\",\"traceId\":\"5B8EFFF798038103D269B633813FC60C\",\"spanId\":\"EEE19B7EC3C1B174\",\"body\":{\"stringValue\":\"test\"}}]}]}]}";

ret = flb_log_event_encoder_init(&enc, FLB_LOG_EVENT_FORMAT_FLUENT_BIT_V2);
TEST_CHECK(ret == FLB_EVENT_ENCODER_SUCCESS);

ret = flb_opentelemetry_logs_json_to_msgpack(&enc, input_json, strlen(input_json), NULL, &error_status);
TEST_CHECK(ret == 0);

/* Create record accessors for trace_id and span_id */
ra_trace_id = flb_ra_create("$otlp['trace_id']", FLB_FALSE);
TEST_CHECK(ra_trace_id != NULL);

ra_span_id = flb_ra_create("$otlp['span_id']", FLB_FALSE);
TEST_CHECK(ra_span_id != NULL);

/* Decode the output to check binary sizes */
ret = flb_log_event_decoder_init(&dec, enc.output_buffer, enc.output_length);
TEST_CHECK(ret == FLB_EVENT_DECODER_SUCCESS);

flb_log_event_decoder_read_groups(&dec, FLB_TRUE);

while ((ret = flb_log_event_decoder_next(&dec, &event)) == FLB_EVENT_DECODER_SUCCESS) {
ret = flb_log_event_decoder_get_record_type(&event, &record_type);
TEST_CHECK(ret == 0);

if (record_type == FLB_LOG_EVENT_NORMAL) {
/* Use record accessor to get trace_id */
val_trace_id = flb_ra_get_value_object(ra_trace_id, *event.metadata);
if (val_trace_id != NULL) {
found_trace_id = 1;
if (val_trace_id->type == FLB_RA_BINARY) {
trace_id_size = flb_sds_len(val_trace_id->val.binary);
printf("Found trace_id with binary size: %zu\n", trace_id_size);
/* trace_id should be 16 bytes (32 hex chars = 16 bytes) */
TEST_CHECK_(trace_id_size == 16, "trace_id binary size should be 16, got %zu", trace_id_size);
}
else if (val_trace_id->type == FLB_RA_STRING) {
printf("Found trace_id as string: %s\n", val_trace_id->val.string);
}
flb_ra_key_value_destroy(val_trace_id);
}

/* Use record accessor to get span_id */
val_span_id = flb_ra_get_value_object(ra_span_id, *event.metadata);
if (val_span_id != NULL) {
found_span_id = 1;
if (val_span_id->type == FLB_RA_BINARY) {
span_id_size = flb_sds_len(val_span_id->val.binary);
printf("Found span_id with binary size: %zu\n", span_id_size);
/* span_id should be 8 bytes (16 hex chars = 8 bytes) */
TEST_CHECK_(span_id_size == 8, "span_id binary size should be 8, got %zu", span_id_size);
}
else if (val_span_id->type == FLB_RA_STRING) {
printf("Found span_id as string: %s\n", val_span_id->val.string);
}
flb_ra_key_value_destroy(val_span_id);
}
}
}

flb_log_event_decoder_destroy(&dec);
flb_log_event_encoder_destroy(&enc);
flb_ra_destroy(ra_trace_id);
flb_ra_destroy(ra_span_id);

TEST_CHECK(found_trace_id == 1);
TEST_CHECK(found_span_id == 1);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Strengthen the test: enforce binary type and verify the exact bytes, not only the lengths.

Today the test passes if IDs show up as strings. Given the PR fixes packaging to 16/8-byte binaries, assert type == FLB_RA_BINARY and bytes match the hex source. Also prefer const for the JSON literal.

Apply this diff:

 void test_trace_span_binary_sizes()
 {
     int ret;
     struct flb_log_event_encoder enc;
     struct flb_log_event_decoder dec;
     struct flb_log_event event;
     int32_t record_type;
-    char *input_json;
+    const char *input_json;
     int error_status = 0;
     int found_trace_id = 0;
     int found_span_id = 0;
     size_t trace_id_size = 0;
     size_t span_id_size = 0;
     struct flb_record_accessor *ra_trace_id;
     struct flb_record_accessor *ra_span_id;
     struct flb_ra_value *val_trace_id;
     struct flb_ra_value *val_span_id;
-    size_t len;
+    unsigned char expected_trace_id[16];
+    unsigned char expected_span_id[8];
+    const char *trace_hex = "5B8EFFF798038103D269B633813FC60C";
+    const char *span_hex  = "EEE19B7EC3C1B174";
 
     /* Test input with trace_id and span_id */
     input_json = "{\"resourceLogs\":[{\"scopeLogs\":[{\"logRecords\":[{\"timeUnixNano\":\"1640995200000000000\",\"traceId\":\"5B8EFFF798038103D269B633813FC60C\",\"spanId\":\"EEE19B7EC3C1B174\",\"body\":{\"stringValue\":\"test\"}}]}]}]}";
 
     ret = flb_log_event_encoder_init(&enc, FLB_LOG_EVENT_FORMAT_FLUENT_BIT_V2);
     TEST_CHECK(ret == FLB_EVENT_ENCODER_SUCCESS);
 
     ret = flb_opentelemetry_logs_json_to_msgpack(&enc, input_json, strlen(input_json), NULL, &error_status);
     TEST_CHECK(ret == 0);
+    TEST_CHECK(error_status == 0);
+
+    /* Prepare expected binary bytes from hex */
+    ret = flb_otel_utils_hex_to_id(trace_hex, 32, expected_trace_id, sizeof(expected_trace_id));
+    TEST_CHECK(ret == 0);
+    ret = flb_otel_utils_hex_to_id(span_hex, 16, expected_span_id, sizeof(expected_span_id));
+    TEST_CHECK(ret == 0);
 
     /* Create record accessors for trace_id and span_id */
     ra_trace_id = flb_ra_create("$otlp['trace_id']", FLB_FALSE);
     TEST_CHECK(ra_trace_id != NULL);
 
     ra_span_id = flb_ra_create("$otlp['span_id']", FLB_FALSE);
     TEST_CHECK(ra_span_id != NULL);
@@
             /* Use record accessor to get trace_id */
             val_trace_id = flb_ra_get_value_object(ra_trace_id, *event.metadata);
             if (val_trace_id != NULL) {
                 found_trace_id = 1;
-                if (val_trace_id->type == FLB_RA_BINARY) {
+                TEST_CHECK_(val_trace_id->type == FLB_RA_BINARY, "trace_id must be emitted as binary");
+                if (val_trace_id->type == FLB_RA_BINARY) {
                     trace_id_size = flb_sds_len(val_trace_id->val.binary);
                     printf("Found trace_id with binary size: %zu\n", trace_id_size);
                     /* trace_id should be 16 bytes (32 hex chars = 16 bytes) */
                     TEST_CHECK_(trace_id_size == 16, "trace_id binary size should be 16, got %zu", trace_id_size);
+                    TEST_CHECK_(memcmp(expected_trace_id, val_trace_id->val.binary, 16) == 0,
+                                "trace_id binary content mismatch");
                 }
-                else if (val_trace_id->type == FLB_RA_STRING) {
-                    printf("Found trace_id as string: %s\n", val_trace_id->val.string);
-                }
                 flb_ra_key_value_destroy(val_trace_id);
             }
 
             /* Use record accessor to get span_id */
             val_span_id = flb_ra_get_value_object(ra_span_id, *event.metadata);
             if (val_span_id != NULL) {
                 found_span_id = 1;
-                if (val_span_id->type == FLB_RA_BINARY) {
+                TEST_CHECK_(val_span_id->type == FLB_RA_BINARY, "span_id must be emitted as binary");
+                if (val_span_id->type == FLB_RA_BINARY) {
                     span_id_size = flb_sds_len(val_span_id->val.binary);
                     printf("Found span_id with binary size: %zu\n", span_id_size);
                     /* span_id should be 8 bytes (16 hex chars = 8 bytes) */
                     TEST_CHECK_(span_id_size == 8, "span_id binary size should be 8, got %zu", span_id_size);
+                    TEST_CHECK_(memcmp(expected_span_id, val_span_id->val.binary, 8) == 0,
+                                "span_id binary content mismatch");
                 }
-                else if (val_span_id->type == FLB_RA_STRING) {
-                    printf("Found span_id as string: %s\n", val_span_id->val.string);
-                }
                 flb_ra_key_value_destroy(val_span_id);
             }
         }
     }
📝 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
void test_trace_span_binary_sizes()
{
int ret;
struct flb_log_event_encoder enc;
struct flb_log_event_decoder dec;
struct flb_log_event event;
int32_t record_type;
char *input_json;
int error_status = 0;
int found_trace_id = 0;
int found_span_id = 0;
size_t trace_id_size = 0;
size_t span_id_size = 0;
struct flb_record_accessor *ra_trace_id;
struct flb_record_accessor *ra_span_id;
struct flb_ra_value *val_trace_id;
struct flb_ra_value *val_span_id;
size_t len;
/* Test input with trace_id and span_id */
input_json = "{\"resourceLogs\":[{\"scopeLogs\":[{\"logRecords\":[{\"timeUnixNano\":\"1640995200000000000\",\"traceId\":\"5B8EFFF798038103D269B633813FC60C\",\"spanId\":\"EEE19B7EC3C1B174\",\"body\":{\"stringValue\":\"test\"}}]}]}]}";
ret = flb_log_event_encoder_init(&enc, FLB_LOG_EVENT_FORMAT_FLUENT_BIT_V2);
TEST_CHECK(ret == FLB_EVENT_ENCODER_SUCCESS);
ret = flb_opentelemetry_logs_json_to_msgpack(&enc, input_json, strlen(input_json), NULL, &error_status);
TEST_CHECK(ret == 0);
/* Create record accessors for trace_id and span_id */
ra_trace_id = flb_ra_create("$otlp['trace_id']", FLB_FALSE);
TEST_CHECK(ra_trace_id != NULL);
ra_span_id = flb_ra_create("$otlp['span_id']", FLB_FALSE);
TEST_CHECK(ra_span_id != NULL);
/* Decode the output to check binary sizes */
ret = flb_log_event_decoder_init(&dec, enc.output_buffer, enc.output_length);
TEST_CHECK(ret == FLB_EVENT_DECODER_SUCCESS);
flb_log_event_decoder_read_groups(&dec, FLB_TRUE);
while ((ret = flb_log_event_decoder_next(&dec, &event)) == FLB_EVENT_DECODER_SUCCESS) {
ret = flb_log_event_decoder_get_record_type(&event, &record_type);
TEST_CHECK(ret == 0);
if (record_type == FLB_LOG_EVENT_NORMAL) {
/* Use record accessor to get trace_id */
val_trace_id = flb_ra_get_value_object(ra_trace_id, *event.metadata);
if (val_trace_id != NULL) {
found_trace_id = 1;
if (val_trace_id->type == FLB_RA_BINARY) {
trace_id_size = flb_sds_len(val_trace_id->val.binary);
printf("Found trace_id with binary size: %zu\n", trace_id_size);
/* trace_id should be 16 bytes (32 hex chars = 16 bytes) */
TEST_CHECK_(trace_id_size == 16, "trace_id binary size should be 16, got %zu", trace_id_size);
}
else if (val_trace_id->type == FLB_RA_STRING) {
printf("Found trace_id as string: %s\n", val_trace_id->val.string);
}
flb_ra_key_value_destroy(val_trace_id);
}
/* Use record accessor to get span_id */
val_span_id = flb_ra_get_value_object(ra_span_id, *event.metadata);
if (val_span_id != NULL) {
found_span_id = 1;
if (val_span_id->type == FLB_RA_BINARY) {
span_id_size = flb_sds_len(val_span_id->val.binary);
printf("Found span_id with binary size: %zu\n", span_id_size);
/* span_id should be 8 bytes (16 hex chars = 8 bytes) */
TEST_CHECK_(span_id_size == 8, "span_id binary size should be 8, got %zu", span_id_size);
}
else if (val_span_id->type == FLB_RA_STRING) {
printf("Found span_id as string: %s\n", val_span_id->val.string);
}
flb_ra_key_value_destroy(val_span_id);
}
}
}
flb_log_event_decoder_destroy(&dec);
flb_log_event_encoder_destroy(&enc);
flb_ra_destroy(ra_trace_id);
flb_ra_destroy(ra_span_id);
TEST_CHECK(found_trace_id == 1);
TEST_CHECK(found_span_id == 1);
}
void test_trace_span_binary_sizes()
{
int ret;
struct flb_log_event_encoder enc;
struct flb_log_event_decoder dec;
struct flb_log_event event;
int32_t record_type;
const char *input_json;
int error_status = 0;
int found_trace_id = 0;
int found_span_id = 0;
size_t trace_id_size = 0;
size_t span_id_size = 0;
struct flb_record_accessor *ra_trace_id;
struct flb_record_accessor *ra_span_id;
struct flb_ra_value *val_trace_id;
struct flb_ra_value *val_span_id;
unsigned char expected_trace_id[16];
unsigned char expected_span_id[8];
const char *trace_hex = "5B8EFFF798038103D269B633813FC60C";
const char *span_hex = "EEE19B7EC3C1B174";
/* Test input with trace_id and span_id */
input_json = "{\"resourceLogs\":[{\"scopeLogs\":[{\"logRecords\":[{\"timeUnixNano\":\"1640995200000000000\",\"traceId\":\"5B8EFFF798038103D269B633813FC60C\",\"spanId\":\"EEE19B7EC3C1B174\",\"body\":{\"stringValue\":\"test\"}}]}]}]}";
ret = flb_log_event_encoder_init(&enc, FLB_LOG_EVENT_FORMAT_FLUENT_BIT_V2);
TEST_CHECK(ret == FLB_EVENT_ENCODER_SUCCESS);
ret = flb_opentelemetry_logs_json_to_msgpack(&enc, input_json, strlen(input_json), NULL, &error_status);
TEST_CHECK(ret == 0);
TEST_CHECK(error_status == 0);
/* Prepare expected binary bytes from hex */
ret = flb_otel_utils_hex_to_id(trace_hex, 32, expected_trace_id, sizeof(expected_trace_id));
TEST_CHECK(ret == 0);
ret = flb_otel_utils_hex_to_id(span_hex, 16, expected_span_id, sizeof(expected_span_id));
TEST_CHECK(ret == 0);
/* Create record accessors for trace_id and span_id */
ra_trace_id = flb_ra_create("$otlp['trace_id']", FLB_FALSE);
TEST_CHECK(ra_trace_id != NULL);
ra_span_id = flb_ra_create("$otlp['span_id']", FLB_FALSE);
TEST_CHECK(ra_span_id != NULL);
/* Decode the output to check binary sizes */
ret = flb_log_event_decoder_init(&dec, enc.output_buffer, enc.output_length);
TEST_CHECK(ret == FLB_EVENT_DECODER_SUCCESS);
flb_log_event_decoder_read_groups(&dec, FLB_TRUE);
while ((ret = flb_log_event_decoder_next(&dec, &event)) == FLB_EVENT_DECODER_SUCCESS) {
ret = flb_log_event_decoder_get_record_type(&event, &record_type);
TEST_CHECK(ret == 0);
if (record_type == FLB_LOG_EVENT_NORMAL) {
/* Use record accessor to get trace_id */
val_trace_id = flb_ra_get_value_object(ra_trace_id, *event.metadata);
if (val_trace_id != NULL) {
found_trace_id = 1;
TEST_CHECK_(val_trace_id->type == FLB_RA_BINARY, "trace_id must be emitted as binary");
if (val_trace_id->type == FLB_RA_BINARY) {
trace_id_size = flb_sds_len(val_trace_id->val.binary);
printf("Found trace_id with binary size: %zu\n", trace_id_size);
/* trace_id should be 16 bytes (32 hex chars = 16 bytes) */
TEST_CHECK_(trace_id_size == 16, "trace_id binary size should be 16, got %zu", trace_id_size);
TEST_CHECK_(memcmp(expected_trace_id, val_trace_id->val.binary, 16) == 0,
"trace_id binary content mismatch");
}
flb_ra_key_value_destroy(val_trace_id);
}
/* Use record accessor to get span_id */
val_span_id = flb_ra_get_value_object(ra_span_id, *event.metadata);
if (val_span_id != NULL) {
found_span_id = 1;
TEST_CHECK_(val_span_id->type == FLB_RA_BINARY, "span_id must be emitted as binary");
if (val_span_id->type == FLB_RA_BINARY) {
span_id_size = flb_sds_len(val_span_id->val.binary);
printf("Found span_id with binary size: %zu\n", span_id_size);
/* span_id should be 8 bytes (16 hex chars = 8 bytes) */
TEST_CHECK_(span_id_size == 8, "span_id binary size should be 8, got %zu", span_id_size);
TEST_CHECK_(memcmp(expected_span_id, val_span_id->val.binary, 8) == 0,
"span_id binary content mismatch");
}
flb_ra_key_value_destroy(val_span_id);
}
}
}
flb_log_event_decoder_destroy(&dec);
flb_log_event_encoder_destroy(&enc);
flb_ra_destroy(ra_trace_id);
flb_ra_destroy(ra_span_id);
TEST_CHECK(found_trace_id == 1);
TEST_CHECK(found_span_id == 1);
}

@edsiper edsiper merged commit 54acd76 into master Aug 21, 2025
64 checks passed
@edsiper edsiper deleted the opentelemetry-logs-ids branch August 21, 2025 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OTEL traceId and spanId sent via HTTP/JSON are doubled in size and malformed

1 participant

Comments