routing: fix issue with missing input alias#11717
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR changes router parsing to bind routing rules to specific input instances by their iteration index. It adds Changes
Sequence DiagramsequenceDiagram
participant Parser as Config Parser
participant InputMgr as Input Instance Lookup
participant Router as Router Config
Parser->>Parser: iterate input sections
Parser->>InputMgr: parse_input_section(config_section, input_index)
InputMgr->>InputMgr: find_input_instance_by_index(config, input_index)
InputMgr-->>Parser: return input instance (or NULL)
Parser->>Parser: assign input->instance from returned value
Parser->>Parser: increment input_index
Parser->>Router: finish parsing, produce input_routes
Router->>Router: apply routes using bound input instances
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/internal/router_config.c (1)
1793-1797: Assert both direct routes target the expected outputs.This only checks the fan-out count. A regression that still creates two paths but connects the wrong output instances would pass here. Please verify that
input_three.routes_directcontains one path tooutput_oneand one tooutput_two.Based on learnings: Add regression tests for: mixed signals, processor drop/modify paths, multi-route fan-out, backlog + live ingestion parity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/internal/router_config.c` around lines 1793 - 1797, The test only asserts the fan-out count for input_three.routes_direct but not that the two direct routes target the expected outputs; update the assertion after flb_router_apply_config to iterate input_three.routes_direct and verify that one route points to output_one and the other to output_two (e.g., inspect each route's destination/output pointer or identifier on the route entries stored in input_three.routes_direct and assert equality to output_one and output_two), keeping the existing count check to ensure exactly two entries; use the same list/traversal helpers already used in tests to find and assert the presence of both outputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/flb_router_config.c`:
- Around line 1083-1111: find_input_instance_by_index is skipping list entries
with ins->p == NULL while the caller's indexing counts every input section,
causing index drift; remove the early continue and ensure the loop increments
the index for every list entry (i.e., treat NULL-ins as placeholders when
counting) so the comparison "if (index == input_index)" uses the same population
as the caller; make the same change to the other identical loop in this file
that uses the same indexing logic.
In `@tests/integration/scenarios/out_stdout/tests/test_out_stdout_001.py`:
- Around line 214-223: The test stops the service immediately after waiting for
"[0] topic1:" which can arrive earlier than other expected lines; change it to
wait for all required log lines before calling service.stop(): call
service.wait_for_log_contains for each of the remaining expected strings ("no
matching route for input chunk" and "tag 'firstdummy'") with appropriate
timeouts (or use a helper that waits for all expected substrings), then stop the
service and finally collect log_text for assertions (use the existing
service.wait_for_log_contains/service.get_logs helpers and the log_text
variable). Ensure you reference the existing service.wait_for_log_contains calls
and keep the same assertions.
---
Nitpick comments:
In `@tests/internal/router_config.c`:
- Around line 1793-1797: The test only asserts the fan-out count for
input_three.routes_direct but not that the two direct routes target the expected
outputs; update the assertion after flb_router_apply_config to iterate
input_three.routes_direct and verify that one route points to output_one and the
other to output_two (e.g., inspect each route's destination/output pointer or
identifier on the route entries stored in input_three.routes_direct and assert
equality to output_one and output_two), keeping the existing count check to
ensure exactly two entries; use the same list/traversal helpers already used in
tests to find and assert the presence of both outputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bbc8c3a-5f60-46ed-aaa5-b0b1c154aea0
📒 Files selected for processing (5)
src/flb_router_config.ctests/integration/scenarios/out_stdout/config/out_stdout_routing_no_alias.yamltests/integration/scenarios/out_stdout/config/out_stdout_routing_with_alias.yamltests/integration/scenarios/out_stdout/tests/test_out_stdout_001.pytests/internal/router_config.c
Comment on lines
+214
to
+223
| log_text = service.wait_for_log_contains("[0] topic1:", timeout=10) | ||
| service.stop() | ||
|
|
||
| assert "connected input 'dummy.2' route 'topic1' to output 'stdout1'" in log_text | ||
| assert "[0] topic1:" in log_text | ||
| assert "\"topic\"=>\"topic1\"" in log_text | ||
| assert "\"message\"=>\"custom dummy\"" in log_text | ||
| assert "no matching route for input chunk" in log_text | ||
| assert "tag 'firstdummy'" in log_text | ||
|
|
There was a problem hiding this comment.
Wait for all asserted log lines before stopping the service.
This returns as soon as "[0] topic1:" appears, but the test also requires "no matching route for input chunk" and "tag 'firstdummy'". Those lines can arrive later, so this is prone to flakes in slower runs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration/scenarios/out_stdout/tests/test_out_stdout_001.py` around
lines 214 - 223, The test stops the service immediately after waiting for "[0]
topic1:" which can arrive earlier than other expected lines; change it to wait
for all required log lines before calling service.stop(): call
service.wait_for_log_contains for each of the remaining expected strings ("no
matching route for input chunk" and "tag 'firstdummy'") with appropriate
timeouts (or use a helper that waits for all expected substrings), then stop the
service and finally collect log_text for assertions (use the existing
service.wait_for_log_contains/service.get_logs helpers and the log_text
variable). Ensure you reference the existing service.wait_for_log_contains calls
and keep the same assertions.
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Member
Author
|
CI issue: related to macos issue in a http client runtime test, being fixed in #11717 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes input route binding when multiple inputs use the same plugin and only one declares routes. Previously, a routed input without alias could bind its routes to the wrong instance, which caused matching records from the intended input to never be routed. The router now binds route blocks to the exact input section that declared them.
This also adds regression coverage in both internal and Python integration tests. The new tests cover the reported dummy input case with and without alias, and verify that the routed third input emits the expected topic1 output in both normal and valgrind-strict integration runs.
Problem
Changes
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
Tests