-
Notifications
You must be signed in to change notification settings - Fork 1.8k
out_parseable: Add Parseable output plugin #11318
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.
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
base: master
Are you sure you want to change the base?
Conversation
|
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. 📝 WalkthroughWalkthroughAdds a new Fluent Bit output plugin "parseable" with a CMake option, plugin registration/build target, a public header for configuration and metrics, and an implementation that formats and sends logs, metrics, and traces to Parseable over HTTP with optional enrichment and retry logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Core as Fluent Bit Core
participant Plugin as out_parseable
participant K8s as Kubernetes (optional)
participant Parseable as Parseable HTTP API
Note over Core,Plugin: Flush event (batch + tag)
Core->>Plugin: cb_flush(batch, tag)
alt dynamic stream from tag
Plugin->>Plugin: extract stream from tag
else dynamic stream via k8s
Plugin->>K8s: request pod metadata
K8s-->>Plugin: return metadata
Plugin->>Plugin: enrich records with k8s fields
end
Plugin->>Plugin: select formatter (JSON / OTLP JSON / OTLP protobuf)
Plugin->>Plugin: format payload, maybe gzip, set headers (X-P-Stream, Auth, user)
Plugin->>Parseable: HTTP POST (TLS if configured)
Parseable-->>Plugin: response (2xx / 4xx / 5xx)
alt 2xx
Plugin->>Plugin: update success metrics
else retryable error
Plugin->>Plugin: apply retry policy, update error metrics
else non-retryable 4xx
Plugin->>Plugin: log and drop, update metrics
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
513d286 to
ad53871
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (2)
plugins/out_parseable/parseable.c (2)
41-43: Remove redundant forward declarations.These
externdeclarations are unnecessary sincecmt_encode_opentelemetry.his already included at line 39, which should provide these declarations.🔎 Suggested fix
-/* Forward declarations for cmetrics opentelemetry encoding */ -extern cfl_sds_t cmt_encode_opentelemetry_create(struct cmt *cmt); -extern void cmt_encode_opentelemetry_destroy(cfl_sds_t text); - #include <ctraces/ctraces.h>
792-792: Misleading void cast.The comment states
destis "Used via flb_sds_cat/flb_sds_printf" butdestis directly used and modified throughout the function. This(void) dest;statement is unnecessary and misleading.🔎 Suggested fix
- (void) dest; /* Used via flb_sds_cat/flb_sds_printf */ - if (obj->type == MSGPACK_OBJECT_MAP) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmake/plugins_options.cmakeplugins/CMakeLists.txtplugins/out_parseable/CMakeLists.txtplugins/out_parseable/parseable.cplugins/out_parseable/parseable.h
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
cmake/plugins_options.cmake
🔇 Additional comments (9)
cmake/plugins_options.cmake (1)
138-138: LGTM!The plugin option follows the established pattern and is correctly placed in alphabetical order among the output plugins.
plugins/out_parseable/parseable.h (1)
40-82: LGTM!The plugin context structure is well-organized with clear sections for configuration, compression, batch controls, routing, metrics, and connection management. All fields align with the implementation in parseable.c.
plugins/CMakeLists.txt (1)
382-382: LGTM!The plugin registration follows the established pattern. The
REGISTER_OUT_PLUGINmacro will check theFLB_OUT_PARSEABLEoption defined in plugins_options.cmake before including the plugin.plugins/out_parseable/CMakeLists.txt (1)
1-5: LGTM!The CMake configuration follows the standard pattern for Fluent Bit plugins.
plugins/out_parseable/parseable.c (5)
1731-1938: HTTP POST logic is well-structured.The function handles compression, headers, error responses, and cleanup correctly. The retry logic appropriately distinguishes between client errors (no retry) and server/network errors (retry).
Note: Ensure the metrics NULL checks are added as mentioned in the earlier comment for
cb_parseable_init, as this function uses those metrics.
1940-2083: LGTM!The flush callback correctly handles different event types (logs, metrics, traces) with appropriate formatting. The dynamic stream routing logic is well-implemented with proper cleanup of allocated resources.
2105-2117: LGTM!The plugin descriptor is properly configured with support for logs, metrics, and traces, network and TLS capabilities, and 2 workers for parallel processing.
548-617: LGTM!The configuration map is well-documented with sensible defaults. The plugin provides good flexibility with options for compression, custom headers, dynamic stream routing, and Kubernetes enrichment.
1449-1463: Null byte removal is defensive programming for input data, not indicative of a bug in string operations.The
flb_sds_printfimplementation is sound and does not produce embedded null bytes—it usesvsnprintf, which cannot insert nulls in the middle of output, and correctly handles truncation by reallocating and retrying. Similarly,escape_json_stringactively skips control characters (including null bytes) withcontinue.If null bytes appear in the output, they likely originate from input data (msgpack string values) rather than from string formatting bugs. Removing them is reasonable defensive programming. However, the warning logs these removals, so consider whether input validation upstream could prevent them entirely or whether the absence of such bytes should be enforced earlier in the pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmake/plugins_options.cmakeplugins/CMakeLists.txtplugins/out_parseable/CMakeLists.txtplugins/out_parseable/parseable.cplugins/out_parseable/parseable.h
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/out_parseable/CMakeLists.txt
- cmake/plugins_options.cmake
- plugins/CMakeLists.txt
🔇 Additional comments (3)
plugins/out_parseable/parseable.h (1)
40-82: LGTM!The context structure is well-organized with clear separation of concerns: configuration (uri, data_type, stream, etc.), output formatting (json_date_format, date_key), compression settings, routing (dynamic_stream), enrichment (enrich_kubernetes), and observability (metrics counters/gauges).
plugins/out_parseable/parseable.c (2)
1476-1678: LGTM!These protobuf formatting functions properly check allocation failures and use
flb_sds_cat_safewhich handles errors more gracefully than the OTEL JSON formatting code. Good error handling pattern with immediate checks and cleanup.
2106-2117: LGTM!The plugin descriptor is correctly configured with support for logs, metrics, and traces, network I/O with TLS, and 2 workers for improved throughput.
29dd784 to
eaaec51
Compare
|
Example Configuration Debug Log Output |
31111d2 to
cc92b38
Compare
cc92b38 to
6688b85
Compare
There was a problem hiding this 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
♻️ Duplicate comments (1)
plugins/out_parseable/parseable.c (1)
1079-1137: Missing NULL checks for multiple flb_sds_cat operations.While some parts of the function use the
SDS_CAT_OR_GOTOmacro for safe concatenation, manyflb_sds_catcalls in this section (lines 1079, 1083, 1084, 1089, 1090, 1096, 1114, 1119, 1121, 1123, 1126, 1128, 1137) don't check for NULL returns. If any allocation fails, subsequent operations will crash.Consider using the
SDS_CAT_OR_GOTOmacro consistently throughout the function or adding NULL checks after each concatenation.🔎 Example refactor pattern
Replace bare
flb_sds_catcalls with the macro:- resource_attrs = flb_sds_cat(resource_attrs, ",", 1); + SDS_CAT_OR_GOTO(resource_attrs, ",", 1, otel_alloc_error);Apply this pattern to all
flb_sds_catcalls in the function.
🧹 Nitpick comments (1)
plugins/out_parseable/parseable.c (1)
1184-1530: Consider consistent use of SDS_CAT_OR_GOTO macro throughout record processing.While the function defines the
SDS_CAT_OR_GOTOmacro for safe concatenation, mostflb_sds_catandflb_sds_printfcalls in the record processing loop don't use it. This creates potential crash points if memory allocation fails during JSON construction.For improved robustness, consider applying the macro pattern consistently throughout the loop or wrapping these sections in a helper function with centralized error handling.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmake/plugins_options.cmakeplugins/CMakeLists.txtplugins/out_parseable/CMakeLists.txtplugins/out_parseable/parseable.cplugins/out_parseable/parseable.h
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/out_parseable/CMakeLists.txt
- plugins/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
cmake/plugins_options.cmake
🔇 Additional comments (9)
cmake/plugins_options.cmake (1)
138-138: LGTM!The option declaration follows the established pattern for output plugins and is correctly alphabetized.
plugins/out_parseable/parseable.h (1)
20-84: LGTM!The header file structure is well-organized with:
- Proper include guards
- Appropriate includes for the types used
- Reasonable default values
- Well-documented struct members aligned with the plugin's functionality
plugins/out_parseable/parseable.c (7)
60-68: Good addition to address NULL check issues.The SDS_CAT_OR_GOTO macro provides a clean pattern for safe string concatenation with automatic cleanup on failure. This addresses the systemic NULL check issues identified in previous reviews.
74-94: LGTM!The function correctly uses
uint32_tfor the loop counter to matchmap->via.map.size, addressing the signed/unsigned mismatch issue from previous reviews.
819-845: LGTM!The function now includes NULL checks after each
flb_sds_catoperation (lines 840-842), properly addressing the allocation failure handling identified in previous reviews.
644-817: LGTM!The initialization function properly handles all allocation failures with appropriate cleanup, addressing previous concerns about missing NULL checks for URI and metric allocations.
2118-2163: LGTM!The custom cleanup function
parseable_config_destroyproperly addresses the memory leak concern raised in previous reviews by explicitly freeing all SDS string fields and the upstream connection. The comment correctly notes thatheadersis managed by the config_map framework.
1008-1018: Recursion issue appears resolved.The previous review flagged a potential infinite recursion when OTLP data is detected. The current implementation correctly bypasses recursion by calling
flb_pack_msgpack_to_json_formatdirectly (line 1009) instead of calling back toparseable_format_json, breaking the potential loop.
1752-2116: LGTM!The HTTP POST and flush callback functions demonstrate proper resource management:
- Upstream connection handling with NULL checks
- Compression with fallback on failure
- HTTP client creation with error handling
- Consistent cleanup of dynamically allocated buffers (out_buf, dynamic_stream, compressed payload)
- Appropriate use of return macros
Add a new output plugin for Parseable, a cloud-native log analytics platform. The plugin supports sending logs, metrics, and traces to Parseable with gzip compression, custom HTTP headers, dynamic stream routing from Kubernetes metadata, Kubernetes metadata enrichment, built-in metrics for observability, and configurable retry limits and batch sizes. Signed-off-by: Debanitrkl <[email protected]>
6688b85 to
434b7ec
Compare
There was a problem hiding this 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
♻️ Duplicate comments (2)
plugins/out_parseable/parseable.c (2)
1818-1825: Missing NULL check for metric counter.Line 1822 calls
cmt_counter_inconctx->cmt_errors_totalwithout checking if it's NULL. Since metric creation can fail (per lines 778-783 with logged warnings), this will crash if the metric failed to initialize. Add a NULL check before use, consistent with the pattern at lines 1962-1964.🔎 Suggested fix
if (!u_conn) { flb_plg_error(ctx->ins, "no upstream connections available to %s:%i", u->tcp_host, u->tcp_port); - cmt_counter_inc(ctx->cmt_errors_total, ts = cfl_time_now(), - 1, (char *[]) {"connection"}); + if (ctx->cmt_errors_total) { + cmt_counter_inc(ctx->cmt_errors_total, ts = cfl_time_now(), + 1, (char *[]) {"connection"}); + } return FLB_RETRY; }
1863-1872: Missing NULL check for metric counter.Line 1869 calls
cmt_counter_inconctx->cmt_errors_totalwithout checking if it's NULL. Add a NULL check consistent with other metric operations in this function.🔎 Suggested fix
if (!c) { flb_plg_error(ctx->ins, "cannot create HTTP client"); if (compressed && payload_buf != body) { flb_free(payload_buf); } flb_upstream_conn_release(u_conn); - cmt_counter_inc(ctx->cmt_errors_total, ts = cfl_time_now(), - 1, (char *[]) {"http_client"}); + if (ctx->cmt_errors_total) { + cmt_counter_inc(ctx->cmt_errors_total, ts = cfl_time_now(), + 1, (char *[]) {"http_client"}); + } return FLB_RETRY; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmake/plugins_options.cmakeplugins/CMakeLists.txtplugins/out_parseable/CMakeLists.txtplugins/out_parseable/parseable.cplugins/out_parseable/parseable.h
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/out_parseable/CMakeLists.txt
- plugins/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
cmake/plugins_options.cmake
🧬 Code graph analysis (1)
plugins/out_parseable/parseable.c (3)
src/flb_pack.c (1)
flb_pack_msgpack_to_json_format(1169-1450)lib/cfl/src/cfl_sds.c (1)
cfl_sds_len(88-91)src/flb_http_client.c (2)
flb_http_client(864-909)flb_http_add_header(1013-1045)
🔇 Additional comments (11)
plugins/out_parseable/parseable.h (1)
20-82: LGTM!The header file is well-structured with appropriate includes, clear macro definitions, and a properly organized context structure. The copyright year has been updated to 2025 as previously requested.
cmake/plugins_options.cmake (1)
138-138: LGTM!The plugin option follows the established pattern and is correctly placed in alphabetical order among other output plugin options.
plugins/out_parseable/parseable.c (9)
64-68: Well-designed safety macro.The
SDS_CAT_OR_GOTOmacro provides a clean pattern for safe SDS concatenation with deterministic cleanup on allocation failure.
122-267: Dynamic stream extraction logic is sound.The function properly handles multiple fallback strategies for stream name resolution, includes NULL checks for all SDS allocations, and correctly implements the exclusion annotation check.
421-507: K8s enrichment implementation is correct.The function properly counts extra fields, packs the enriched msgpack structure, and adds both K8s context fields (namespace, pod, container, node) and unified service tags (environment, service, version).
644-817: Initialization is robust.The function properly validates required configuration, auto-sets URIs based on data type with NULL checks, handles compression configuration, sets up TLS/upstream connections, and initializes metrics with non-fatal warnings if creation fails.
819-845: String escaping with proper NULL checking.The function correctly checks for allocation failure after each
flb_sds_catcall and propagates errors by returning NULL.
850-994: Well-structured attribute flattening with proper error handling.The function consistently uses the
SDS_CAT_OR_GOTOmacro for safe SDS operations and implements proper cleanup paths via labeled gotos.
1940-1976: HTTP response handling with appropriate retry logic.The function correctly distinguishes between non-retryable client errors (4xx except 429/408) and retryable server errors, updates metrics on success, and provides detailed error logging.
2158-2203: Proper cleanup implementation.The
parseable_config_destroyfunction correctly frees all allocated SDS string fields and the upstream connection. The exit callback properly delegates to this cleanup function, addressing previously flagged memory leak concerns.
2206-2217: Plugin descriptor is correctly configured.The plugin properly declares support for logs, metrics, and traces via
FLB_OUTPUT_LOGS | FLB_OUTPUT_METRICS | FLB_OUTPUT_TRACES, enables network and TLS flags, and configures 2 workers.
Add a new output plugin for Parseable.
Features:
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
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.