add journal support #33
Conversation
| return | ||
| } | ||
| if line == "" { | ||
| time.Since(100 * time.Millisecond) |
There was a problem hiding this comment.
oh......stupid mistake :(
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
|
@adohe Thanks a lot! Will review soon. Before that, it seems that there is something wrong with Godeps, the test is failing. |
|
I will fix this after my training is over. |
|
@adohe Friendly ping~ :) |
|
@Random-Liu I just go back from my National Holiday, will fix this asap. |
|
CLAs look good, thanks! |
|
@Random-Liu I just update the godep, but it seems still some problem: Seems there is no libsystemd development/headers files available in the system search path. Could you confirm this? From the build log, we are running tests with docker, I am afraid there is no libsystemd in the image. |
|
@adohe Hm, let me take a look. |
|
@Random-Liu maybe we can modify |
|
@Random-Liu The travis failed because of this: and I just found this on stackoverflow |
|
@Random-Liu Friendly ping :) |
| for { | ||
|
|
| } | ||
| if err == nil { | ||
| buffer.WriteString(line) | ||
| // trime `\n` |
| glog.Errorf("exiting kernel log watch with error: %v", err) | ||
| return | ||
| } | ||
| if line == "" { |
There was a problem hiding this comment.
I think once we read till EOF, we should wait.
And less indent is preferred.
If err == io.EOF {
buffer.WriterString(line)
time.Sleep(pollPeriod) // Please use a const number
continue
}
// Parsing the log
| return nil, fmt.Errorf("Error opening journal: %v", err) | ||
| } | ||
| if r == nil { | ||
| return nil, fmt.Errorf("Got a nil reader") |
There was a problem hiding this comment.
Error should start with small letter. got a nil reader and same with the errors above.
|
|
||
| func tryJournal() (io.Reader, error) { | ||
| r, err := sdjournal.NewJournalReader(sdjournal.JournalReaderConfig{ | ||
| NumFromTail: uint64(0), |
There was a problem hiding this comment.
Please pass Since timestamp instead of tail to journald. See the lookback option above.
| @@ -17,15 +17,20 @@ limitations under the License. | |||
| package kernelmonitor | |||
|
|
|||
| import ( | |||
| "bufio" | |||
|
@adohe It seems that May be you can try ubuntu 14.04 with: And install |
|
@adohe And can we add a build tag for systemd? We may usually build on non-systemd machine. and And use |
|
@Random-Liu Thanks for your kind review, I will update this when I get up. |
|
@adohe I believe this time journal header file stuff is solved, but there is a real failure. |
|
@Random-Liu yes, I am fixing this failure. |
|
@adohe Hi, this coming week will be the last week before code freeze. We need this feature for 1.5. Do you have time to work on this? If not, we can get your current code in in a harmless way, then I can continue your following work. :) |
|
@Random-Liu sorry I just forget the timeline, I will fix this today. Really sorry about this |
|
@adohe Never mind. Really appreciate your help~Thanks a lot! |
|
@Random-Liu I just fixed the failed tests, I believe there are some strange part that you may want to take a look. I will refactor some code tomorrow morning, it's to late now, I must go to sleep. |
Thanks! I don't quite understand your current fix in fact... Will review again after your cleanup. :) |
|
@adohe I'll merge this first and I'll finish the following work. |
needs to fix dependency and add test.