-
Notifications
You must be signed in to change notification settings - Fork 73
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
Fixes #21736: when a key is multi-line, the reporting fails because agent add the datetime in fron of each new line #4474
Fixes #21736: when a key is multi-line, the reporting fails because agent add the datetime in fron of each new line #4474
Conversation
not finished yet. |
PR updated with a new commit |
2 similar comments
PR updated with a new commit |
PR updated with a new commit |
fn simpleline_until_metadata(i: &str) -> IResult<&str, &str> { | ||
let (i, _) = not(tag("@@"))(i)?; | ||
let (i, _) = opt(line_timestamp)(i)?; | ||
// Try to parse as sigle line metadata. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Try to parse as sigle line metadata. | |
// Try to parse as single line metadata. |
PR updated with a new commit |
@@ -594,6 +682,41 @@ mod tests { | |||
maybe_report(report).unwrap().1, | |||
Err("2018-08-24T15:55:01+00:00 R: @@Common@@broken".to_string()) | |||
); | |||
let report = "garbage\n2018-08-24T15:55:01+00:00 R: @@Common@@result_repaired@@hasPolicyServer-root@@common-root@@0@@CRON Daemon@@multi\r\n2018-08-24T15:55:01+00:00 line@@2018-08-24 15:55:01 +00:00##root@#Cron daemon status was repaired\r\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there a date in the after the newline in the component key? On windows node we do no report like that. The date prefix is only added on new report lines. Not on newline inside a report.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because on Linux the timestamp do not come from the logs but are added by the wrapper, so we can't really know which line require a prefix, so we add it on all lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added another test for the full windows case.
PR updated with a new commit |
This PR is not mergeable to upper versions. |
OK, squash merging this PR |
…gent add the datetime in fron of each new line
6055149
to
109e064
Compare
https://issues.rudder.io/issues/21736