parser: Address wrong assignments of timezone at midnight#11677
parser: Address wrong assignments of timezone at midnight#11677
Conversation
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
📝 WalkthroughWalkthroughThis pull request fixes a timezone-related bug in timestamp parsing when logs contain only time information without a date. The fix modifies Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/internal/parser.c (1)
586-588: Minor: Redundant parser destruction.
flb_parser_destroy(parser)explicitly destroys the parser and removes it from the config list.flb_parser_exit(config)then iterates remaining parsers to destroy them. Since only one parser exists and it's already destroyed,flb_parser_exitbecomes a no-op for parsers (though it still callsflb_ml_exit).You could simplify by removing line 586 and letting
flb_parser_exithandle cleanup, or keep the explicit destroy if you prefer the clarity.♻️ Optional simplification
- flb_parser_destroy(parser); flb_parser_exit(config); flb_config_exit(config);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/internal/parser.c` around lines 586 - 588, The test currently calls flb_parser_destroy(parser) then flb_parser_exit(config) which redundantly attempts parser cleanup; remove the explicit flb_parser_destroy(parser) call and let flb_parser_exit(config) (followed by flb_config_exit(config)) handle parser teardown, or conversely keep flb_parser_destroy(parser) and remove parser cleanup from the flb_parser_exit path—pick one approach and make the code consistent by referencing the parser variable and the flb_parser_exit/flb_config_exit cleanup flow accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/internal/parser.c`:
- Around line 586-588: The test currently calls flb_parser_destroy(parser) then
flb_parser_exit(config) which redundantly attempts parser cleanup; remove the
explicit flb_parser_destroy(parser) call and let flb_parser_exit(config)
(followed by flb_config_exit(config)) handle parser teardown, or conversely keep
flb_parser_destroy(parser) and remove parser cleanup from the flb_parser_exit
path—pick one approach and make the code consistent by referencing the parser
variable and the flb_parser_exit/flb_config_exit cleanup flow accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4daaa31-5316-4039-83b9-4f002b26eff4
📒 Files selected for processing (2)
src/flb_parser.ctests/internal/parser.c
Closes #11648.
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