opentelemetry: traces: unify JSON to ctrace encoder #11042
Conversation
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughCentralizes OTLP JSON→CTraces conversion into a new public converter, adds a TRACES-specific error taxonomy, delegates plugin JSON handling to that converter, strengthens trace/span ID validation in logs path, and adds trace-focused tests and build integration. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as in_opentelemetry<br/>(plugin)
participant API as flb_opentelemetry<br/>json_traces_to_ctrace
participant Conv as JSON → MsgPack
participant Parser as MsgPack Parser
participant CTr as CTraces Builder
Plugin->>API: flb_opentelemetry_json_traces_to_ctrace(body,len,&error_status)
API->>Conv: convert JSON → MessagePack
Conv-->>API: msgpack payload
API->>Parser: parse root → resourceSpans → scopeSpans → spans
Parser->>CTr: create resources, scopes, spans, attrs, events, links, status
CTr-->>API: assembled ctrace (pointer) or NULL
API-->>Plugin: ctrace | NULL + error_status
Plugin->>Plugin: handle ctrace or log failure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (obj->type != MSGPACK_OBJECT_MAP) { | ||
| flb_error("unexpected root object type for traces"); | ||
| return NULL; | ||
| } | ||
|
|
||
| ret = flb_otel_utils_find_map_entry_by_key(&obj->via.map, "resourceSpans", 0, FLB_TRUE); | ||
| if (ret < 0) { | ||
| flb_error("resourceSpans entry is missing"); | ||
| return NULL; |
There was a problem hiding this comment.
The new centralized JSON→ctrace conversion only looks for the resourceSpans key and immediately errors when it isn’t present. The previous in-plugin implementation also checked resource_spans as a fallback, allowing payloads that used the older snake_case form to be ingested. With this change any such payloads now trigger resourceSpans entry is missing and the trace is dropped. If backward compatibility is still required, consider retaining the alternate key or documenting the breaking change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
good heads up, but snake_case name lookup is not required.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
plugins/in_opentelemetry/opentelemetry_traces.c (1)
62-77: error_status is always 0 with current impl; improve propagationThe converter doesn’t currently set error_status, so this logs “status: 0” on failure. After wiring the library to set error_status (see separate comment), also consider logging the symbolic name via flb_opentelemetry_error_to_string for clarity.
Example:
- flb_plg_error(ctx->ins, "invalid JSON trace: conversion error (status: %d)", error_status); + flb_plg_error(ctx->ins, + "invalid JSON trace: conversion error (status: %d%s%s)", + error_status, + error_status ? ", name: " : "", + error_status ? flb_opentelemetry_error_to_string(error_status) : "");src/opentelemetry/flb_opentelemetry_traces.c (3)
701-706: Remove duplicate/contradictory flags blockThere are two “flags” blocks; this one says “unsupported” but later you actually set flags (Lines 733-737). Remove this earlier block to avoid confusion.
- /* flags */ - ret = flb_otel_utils_find_map_entry_by_key(&span.via.map, "flags", 0, FLB_TRUE); - if (ret >= 0 && span.via.map.ptr[ret].val.type == MSGPACK_OBJECT_POSITIVE_INTEGER) { - /* unsupported in CTraces */ - }
335-338: Don’t abort all events on a single malformed eventReturning -1 on missing event name stops processing the entire events array. Prefer skipping just that event with a warning.
- if (!name_str) { - flb_warn("span event name is missing"); - return -1; - } + if (!name_str) { + flb_warn("span event name is missing; skipping event"); + continue; + }
1051-1058: Best‑effort parsing: consider skipping bad resourceSpans instead of abortingAborting and destroying the whole ctrace on the first bad resourceSpan drops otherwise valid data. Consider logging and continuing.
Pseudo‑change:
for (...) { if (process_resource_span(ctr, &resource_spans->via.array.ptr[i]) == -1) { flb_warn("failed to process resource span %d; skipping", i); continue; } } return ctr;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
include/fluent-bit/flb_opentelemetry.h(1 hunks)plugins/in_opentelemetry/opentelemetry_traces.c(2 hunks)src/CMakeLists.txt(1 hunks)src/opentelemetry/flb_opentelemetry_traces.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
include/fluent-bit/flb_opentelemetry.h (1)
src/opentelemetry/flb_opentelemetry_traces.c (1)
flb_opentelemetry_json_traces_to_ctrace(1063-1102)
src/opentelemetry/flb_opentelemetry_traces.c (8)
src/flb_sds.c (2)
flb_sds_create_len(58-76)flb_sds_destroy(389-399)src/opentelemetry/flb_opentelemetry_utils.c (4)
flb_otel_utils_find_map_entry_by_key(28-73)flb_otel_utils_json_payload_get_wrapped_value(75-192)flb_otel_utils_hex_to_id(573-599)flb_otel_utils_convert_string_number_to_u64(601-622)lib/cfl/src/cfl_sds.c (2)
cfl_sds_create_len(93-111)cfl_sds_destroy(127-137)lib/ctraces/src/ctr_scope.c (4)
ctr_instrumentation_scope_create(89-112)ctr_scope_span_set_instrumentation_scope(78-87)ctr_scope_span_create(22-36)ctr_scope_span_set_schema_url(64-76)lib/ctraces/src/ctr_span.c (16)
ctr_span_event_add_ts(413-444)ctr_span_event_set_attributes(484-496)ctr_span_event_set_dropped_attributes_count(498-501)ctr_span_set_status(281-299)ctr_span_create(26-88)ctr_span_set_trace_id(91-109)ctr_span_set_span_id(120-134)ctr_span_set_parent_span_id(145-161)ctr_span_start_ts(259-266)ctr_span_end_ts(276-279)ctr_span_kind_set(171-179)ctr_span_set_attributes(204-216)ctr_span_set_dropped_attributes_count(345-348)ctr_span_set_dropped_events_count(335-338)ctr_span_set_dropped_links_count(340-343)ctr_span_set_schema_url(321-328)lib/ctraces/src/ctr_link.c (5)
ctr_link_create(22-55)ctr_link_set_trace_state(79-91)ctr_link_set_attributes(93-101)ctr_link_set_dropped_attr_count(103-106)ctr_link_set_flags(108-111)src/flb_pack.c (1)
flb_pack_json(530-535)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
plugins/in_opentelemetry/opentelemetry_traces.c (2)
src/opentelemetry/flb_opentelemetry_traces.c (1)
flb_opentelemetry_json_traces_to_ctrace(1063-1102)src/flb_input_trace.c (1)
flb_input_trace_append(123-131)
⏰ 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). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- 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: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 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 (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- 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_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 (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- 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-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: PR - fuzzing test
- 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, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (1)
src/CMakeLists.txt (1)
117-117: Build glue looks goodNew source added to OpenTelemetry Helpers is correct. Nothing else needed here.
| struct ctrace *flb_opentelemetry_json_traces_to_ctrace(const char *body, size_t len, | ||
| int *error_status); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix incomplete type and missing header; define error_status semantics
- Unknown type in some translation units: struct ctrace is not declared here. Either forward-declare it or include <ctraces/ctraces.h>.
- strcmp is used but <string.h> isn’t included in this header.
- error_status is exposed but not documented/guaranteed to be set by the implementation.
Apply:
@@
-#include <stdint.h>
+#include <stdint.h>
+#include <string.h>
+
+/* Forward declaration to allow users to include this header without
+ * pulling in <ctraces/ctraces.h> everywhere. */
+struct ctrace;
@@
-struct ctrace *flb_opentelemetry_json_traces_to_ctrace(const char *body, size_t len,
- int *error_status);
+/* Converts an OTLP JSON Traces payload to a CTraces context.
+ * On failure returns NULL and, if error_status != NULL, sets it to a non‑zero code. */
+struct ctrace *flb_opentelemetry_json_traces_to_ctrace(const char *body, size_t len,
+ int *error_status);Optionally, introduce trace‑specific error codes in this header in a follow‑up, instead of reusing LOGS codes.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In include/fluent-bit/flb_opentelemetry.h around lines 127-129, three issues
must be fixed: forward-declare or include the definition for struct ctrace
(e.g., add a forward declaration "struct ctrace;" or include the appropriate
header <ctraces/ctraces.h>) so translation units know the type; add #include
<string.h> to this header (or ensure the header that uses strcmp includes it) so
string functions are declared; and document and guarantee error_status semantics
by changing the function comment/signature notes to state when error_status is
set and what values mean (e.g., 0 on success, non-zero on error) — ensure the
implementation sets *error_status in all return paths; optionally leave room to
add trace-specific error codes later.
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/opentelemetry/flb_opentelemetry_traces.c (1)
122-129: Array attributes still silently succeed without storing data.Despite past review comments indicating this was addressed, the MSGPACK_OBJECT_ARRAY case still returns
ret = 0(success) without setting any attribute. This silently drops array-valued attributes from the trace data, potentially causing data loss that's difficult to debug.Consider one of these approaches:
Option 1 (minimal): Return an error so the caller's warning is triggered:
case MSGPACK_OBJECT_ARRAY: /* * The less fun part (OTel JSON encoding), the value can be an array and * only allows values such as string, bool, int64, double. I am glad this * don't support nested arrays or maps. */ - ret = 0; + flb_warn("array-valued attributes are not supported in CTraces, skipping"); + ret = -1; break;Option 2 (better): Serialize simple arrays to preserve information (iterate array, validate element types, concatenate to comma-separated string or JSON-encode).
🧹 Nitpick comments (1)
tests/internal/README.md (1)
39-55: Specify language for fenced code block.The fenced code block should include a language identifier for proper syntax highlighting and markdown compliance.
Apply this diff:
-``` +```json { "test_name": { "input": { ... }, # OTLP/JSON traces payloadBased on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/opentelemetry/flb_opentelemetry_logs.c(1 hunks)src/opentelemetry/flb_opentelemetry_traces.c(1 hunks)tests/internal/CMakeLists.txt(1 hunks)tests/internal/README.md(2 hunks)tests/internal/data/opentelemetry/logs.json(1 hunks)tests/internal/data/opentelemetry/traces.json(1 hunks)tests/internal/opentelemetry.c(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/internal/data/opentelemetry/traces.json
🧰 Additional context used
🧬 Code graph analysis (2)
src/opentelemetry/flb_opentelemetry_traces.c (6)
src/flb_sds.c (2)
flb_sds_create_len(58-76)flb_sds_destroy(389-399)src/opentelemetry/flb_opentelemetry_utils.c (4)
flb_otel_utils_find_map_entry_by_key(28-73)flb_otel_utils_json_payload_get_wrapped_value(75-192)flb_otel_utils_hex_to_id(573-599)flb_otel_utils_convert_string_number_to_u64(601-622)lib/cfl/src/cfl_sds.c (2)
cfl_sds_create_len(93-111)cfl_sds_destroy(127-137)lib/ctraces/src/ctr_scope.c (4)
ctr_instrumentation_scope_create(89-112)ctr_scope_span_set_instrumentation_scope(78-87)ctr_scope_span_create(22-36)ctr_scope_span_set_schema_url(64-76)lib/ctraces/src/ctr_span.c (13)
ctr_span_event_add_ts(413-444)ctr_span_event_set_attributes(484-496)ctr_span_event_set_dropped_attributes_count(498-501)ctr_span_create(26-88)ctr_span_set_trace_id(91-109)ctr_span_set_span_id(120-134)ctr_span_set_parent_span_id(145-161)ctr_span_set_flags(315-319)ctr_span_start_ts(259-266)ctr_span_end_ts(276-279)ctr_span_kind_set(171-179)ctr_span_set_attributes(204-216)ctr_span_set_schema_url(321-328)lib/ctraces/src/ctr_link.c (5)
ctr_link_create(22-55)ctr_link_set_trace_state(79-91)ctr_link_set_attributes(93-101)ctr_link_set_dropped_attr_count(103-106)ctr_link_set_flags(108-111)
tests/internal/opentelemetry.c (5)
src/opentelemetry/flb_opentelemetry_utils.c (2)
flb_otel_utils_hex_to_id(573-599)flb_otel_utils_find_map_entry_by_key(28-73)src/flb_pack.c (2)
flb_pack_json(530-535)flb_msgpack_to_json_str(1459-1500)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)include/fluent-bit/flb_opentelemetry.h (1)
flb_opentelemetry_error_code(159-169)src/opentelemetry/flb_opentelemetry_traces.c (1)
flb_opentelemetry_json_traces_to_ctrace(1211-1263)
🪛 markdownlint-cli2 (0.18.1)
tests/internal/README.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- 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: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 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_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 (-DSANITIZE_ADDRESS=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_SANITIZE_THREAD=On, 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_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=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 (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- 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, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=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_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- 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-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-centos-7
- GitHub Check: PR - fuzzing test
🔇 Additional comments (9)
src/opentelemetry/flb_opentelemetry_traces.c (2)
468-474: LGTM: Hex decode validation is now properly implemented.All calls to
flb_otel_utils_hex_to_id()now check the return value and handle invalid hex appropriately by setting error_status and returning early. This addresses the previous review concern about invalid hex not being caught.Also applies to: 505-511, 721-728, 751-758, 799-806
1222-1224: LGTM: Error status properly initialized and set throughout.The public API correctly initializes error_status to 0 on entry and sets appropriate error codes on all failure paths. This addresses the previous review concern about error_status not being populated.
Also applies to: 1229-1234, 1243-1250, 1258-1260
tests/internal/CMakeLists.txt (1)
174-174: LGTM: Test data path updated to support logs/traces split.The change from
test_cases.jsontologs.jsonaligns with the PR's reorganization of test data into separate files for logs and traces, improving test organization.src/opentelemetry/flb_opentelemetry_logs.c (1)
305-310: LGTM: Hex validation properly added for trace/span IDs.The validation now checks the return value of
flb_otel_utils_hex_to_id()for both trace_id and span_id, setting appropriate error codes (FLB_OTEL_LOGS_ERR_INVALID_TRACE_ID / FLB_OTEL_LOGS_ERR_INVALID_SPAN_ID) when invalid hex is detected. This prevents malformed IDs from propagating through the system.Also applies to: 317-322
tests/internal/data/opentelemetry/logs.json (1)
595-696: LGTM: Comprehensive hex validation test coverage.The new test cases thoroughly exercise the hex validation logic with:
- Invalid hex characters (G, Z)
- Odd-length hex strings (31, 15 chars)
- Mixed valid/invalid characters
Each case properly specifies the expected error code, ensuring robust validation of the hex decoding implementation.
tests/internal/opentelemetry.c (4)
30-30: LGTM: CTRACES integration enabled.Adding the ctraces/ctraces.h include integrates the CTraces library for trace-specific testing, aligning with the PR's objective to add centralized JSON-to-CTraces conversion.
411-447: LGTM: Comprehensive hex validation edge-case tests.The test function covers important edge cases:
- Zero-length strings (should succeed with empty output)
- Odd-length strings (should fail)
- Invalid hex characters (G, Z)
- Uppercase and lowercase hex (both should succeed)
This provides thorough validation of the
flb_otel_utils_hex_to_idimplementation.
540-541: LGTM: Test data paths separated for logs and traces.The separate paths (
logs.jsonandtraces.json) support the PR's objective to organize test data by signal type, improving test maintainability and clarity.
801-926: LGTM: Traces test implementation mirrors logs testing approach.The new
test_opentelemetry_traces_cases()function:
- Loads test cases from traces.json
- Calls the centralized
flb_opentelemetry_json_traces_to_ctrace()API- Validates both success and error cases with proper error code checking
- Properly cleans up ctrace contexts
The implementation is consistent with the existing logs testing pattern and properly integrated into the TEST_LIST.
Also applies to: 1024-1024
1 participant
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