-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
fix(parsers.csv): Remove direct checks for the parser type #11825
Conversation
Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
This fixes the bug caused by the last two wrapped parsers listed in #11820. It's a fix for the regression in 1.24.0 and should be included in 1.24.1. When we have more time, we should revisit the approach of using a parser specific error (originally parsers.ErrNotEnoughData in this PR but now named parsers.ErrEOF). If the input plugin is always going to ignore the error, it would be simpler if the parser didn't return an error to begin with. Maybe parsers whose Parse methods calls their own ParseLine method need it? If we do decide parsers.ErrEOF is needed, we should check the other parsers besides csv and update them to use it. For example, xml could return it if it hasn't received data including a closing tag, and influx could use it when it has received part of a line but not the end of line character. |
36f435e
to
d1dc8a3
Compare
d1dc8a3
to
e14e7a7
Compare
(cherry picked from commit 4897f86)
partially resolves #11820
This PR introduces a new error (
ErrNotEnoughData
) for parsers that need to consume more data to output metrics. This is the case for CSV when fed linewise with skipping rows, metadata or other headers involved.As a consequence, we can remove the direct check for the CSV parser in
inputs.tail
andinputs.directory_monitor
and open the possibility for other parsers to also use this error.