multiline: fix handling of return error #10928
Conversation
WalkthroughMultiline parser now propagates a negative return from a sub-parser in text input flow (returns -1 immediately), enabling the next parser in the chain to handle the input; a regression test for a docker→CRI parser chain (non-newline-terminated input + explicit flush) was added and expectations updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Src as SourceLine
participant Chain as ParserChain
participant Dock as DockerParser
participant CRI as CRIParser
Src->>Chain: deliver(line)
Chain->>Dock: try parse(line)
alt Docker returns < 0 (decline/error)
Dock-->>Chain: -1
note right of Chain #E8F6F3: New behavior — propagate failure immediately
Chain->>CRI: try parse(line)
CRI-->>Chain: success / records
else Docker accepts or buffers
Dock-->>Chain: success / buffer
end
Chain-->>Src: result / next action
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)tests/internal/multiline.c (4)
⏰ 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). (29)
🔇 Additional comments (5)
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 |
edsiper
force-pushed
the
multiline-docker-issue
branch
from
5052074 to
65078e3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/internal/multiline.c (2)
127-145: Add stderr and partial (P) variants to harden the regression guard.Optional: expand coverage to ensure both streams and partial records are correctly declined by docker and consumed by CRI without buffering.
Apply this diff to extend the datasets minimally:
struct record_check docker_cri_chain_input[] = { {"2025-09-22T19:07:06.115398289Z stdout F first message"}, {"2025-09-22T19:07:06.116725604Z stdout F second message"}, {"2025-09-22T19:07:08.582112316Z stdout F third message"}, + {"2025-09-22T19:07:09.000000000Z stderr F fourth message"}, + {"2025-09-22T19:07:10.000000000Z stdout P partial "}, + {"2025-09-22T19:07:10.000000001Z stdout F tail"}, }; struct record_check docker_cri_chain_output[] = { {"first message"}, {"second message"}, {"third message"}, + {"fourth message"}, + {"partial tail"}, };Note: Add corresponding assertion count update in the test below.
631-691: Capture and assert parser return codes for earlier failure detection.Small robustness win: check flb_ml_append_text return values.
Apply this diff:
- /* Package as msgpack */ - flb_ml_append_text(ml, stream_id, &tm, r->buf, len); + /* Package as msgpack */ + ret = flb_ml_append_text(ml, stream_id, &tm, r->buf, len); + TEST_CHECK(ret >= 0);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/internal/multiline.c(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/internal/multiline.c (4)
src/flb_config.c (2)
flb_config_init(225-438)flb_config_exit(440-617)src/multiline/flb_ml.c (4)
flb_ml_create(871-923)flb_ml_append_text(667-757)flb_ml_flush_pending_now(143-149)flb_ml_destroy(984-1009)src/multiline/flb_ml_parser.c (1)
flb_ml_parser_instance_create(261-312)src/multiline/flb_ml_stream.c (1)
flb_ml_stream_create(223-276)
⏰ 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). (29)
- 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 (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- 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, clang, clang++, 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-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
- 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=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_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 (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: PR - fuzzing test
- 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, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=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, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=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_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=Off, 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_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
🔇 Additional comments (4)
tests/internal/multiline.c (4)
110-115: Comment clarifies intended interleaving semantics—good.Accurately documents the expected flush order between docker-buffered fragments and CRI records in a chained setup.
123-125: Updated expectations match actual arrival order.Placing "bbccdd-out\n" and "dd-err\n" after the CRI outputs aligns with input sequencing and the chain handoff behavior.
680-685: Good: explicit flush + count check prevents latent buffering.Ensures no pending records remain and guards against regressions in chain handoff.
1650-1650: Test registered in TEST_LIST—consistent naming and placement.Keeps suite coverage coherent.
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
edsiper
force-pushed
the
multiline-docker-issue
branch
from
65078e3 to
ca90813
Compare
|
rebased on top of master, #10930 should fix the processor issue |
Fixes #10927
After the last refactoring, we are missing to check the return value when multiple parsers are loaded in a chain. Issue was not found before because the missing unit test for that specific case.
This PR implements the fix and proper unit test to avoid regressions.
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
Tests