Refactor router config parser and add route precedence test #11039
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis PR introduces a telemetry routing system for Fluent Bit, comprising documentation, public API definitions (structs and functions), router condition evaluation logic, configuration parsing from YAML, and comprehensive unit and runtime tests with test data files. The implementation supports signal-based routing (logs, metrics, traces) with condition evaluation and output fallback handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as YAML Config
participant Parser as flb_router_config_parse()
participant Validator as Validators<br/>(signal, rules, etc.)
participant Memory as Data Structures<br/>(routes, conditions)
participant List as input_routes List
Config->>Parser: Parse inputs & routes
activate Parser
Parser->>Validator: Validate signal flags
activate Validator
Validator-->>Parser: OK/Error
deactivate Validator
Parser->>Validator: Validate condition rules<br/>field alignment
activate Validator
Validator-->>Parser: OK/Error
deactivate Validator
Parser->>Memory: Allocate & populate<br/>input_routes, routes,<br/>conditions, outputs
activate Memory
Memory-->>Parser: Constructed trees
deactivate Memory
Parser->>List: Append input_routes
activate List
List-->>Parser: Added
deactivate List
Parser-->>Config: Return success/failure
deactivate Parser
sequenceDiagram
participant Event as Event Chunk
participant Signal as flb_router_signal_from_chunk()
participant Router as flb_route_condition_eval()
participant Evaluator as Per-Signal<br/>Evaluator<br/>(logs/metrics/traces)
participant Route as Route Match<br/>Decision
Event->>Signal: chunk.type
activate Signal
Signal-->>Router: signal (LOGS/METRICS/TRACES/0)
deactivate Signal
activate Router
Router->>Router: Check route null<br/>& signals bitmask
alt Route has no condition
Router->>Route: Return TRUE
else Route has default condition
Router->>Route: Return TRUE
else Route condition has no rules
Router->>Route: Return TRUE
else Route condition has rules
Router->>Evaluator: Dispatch by signal
activate Evaluator
Evaluator-->>Route: Evaluate rules<br/>(logs/metrics/traces)<br/>Return TRUE/FALSE
deactivate Evaluator
else Unknown signal
Router->>Route: Return FALSE
end
deactivate Router
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes The PR introduces substantial new functionality spanning multiple logical layers: public API definitions with eight new data structures, two implementation files with dense logic including configuration parsing and condition evaluation, extensive memory management with cleanup paths, and comprehensive test coverage. While individual cohorts (test data, build files) are straightforward, the core implementation files ( Suggested reviewers
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (13)
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 (route->condition->is_default) { | ||
| return FLB_TRUE; | ||
| } | ||
|
|
||
| if (cfl_list_is_empty(&route->condition->rules) == 0) { | ||
| return FLB_TRUE; | ||
| } |
There was a problem hiding this comment.
The router condition evaluator returns FLB_TRUE whenever route->condition->rules is non-empty, so any route that declares at least one rule will match every chunk regardless of its contents. This bypasses the signal-specific evaluators entirely (they are never called) and causes conditional routes to supersede default routes even though the rule logic is not yet implemented. As a result, routing decisions will ignore the configured predicates and send all traffic to the first conditional route.
Useful? React with 👍 / 👎.
| if (add_processor_properties(processor, kvlist) != 0) { | ||
| route_processors_destroy(&processor->properties); | ||
| if (processor->name) { | ||
| flb_sds_destroy(processor->name); | ||
| } |
There was a problem hiding this comment.
When add_processor_properties fails, the code calls route_processors_destroy(&processor->properties) for cleanup. route_processors_destroy expects a list of struct flb_route_processor, but processor->properties contains struct flb_route_processor_property. Iterating that list as if it contained processors will dereference invalid fields and free bogus pointers, leading to crashes or memory corruption on malformed input or allocation failures. The cleanup needs a dedicated routine that frees properties rather than invoking the processor list destructor.
Useful? React with 👍 / 👎.
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Documentation
Tests