-
Notifications
You must be signed in to change notification settings - Fork 16
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
Support handle invalid logs #19
Changes from 7 commits
dab3e40
d2fbf9c
71a9f15
ca397ce
e1084ad
f53e8a8
b08939a
f386b97
b5ead57
2f47a32
b935fdb
f43ae93
b3464a1
650740f
f628718
56008cf
6921c86
35d98d7
82bb8b1
f25244b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ on: | |
- master | ||
|
||
jobs: | ||
release_packages: | ||
test: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,31 +73,25 @@ func resolveFiles(logFilePath string, beginTime, endTime int64) ([]logFile, erro | |
return nil | ||
} | ||
reader := bufio.NewReader(file) | ||
// Skip this file if cannot read the first line | ||
firstLine, err := readLine(reader) | ||
if err != nil && err != io.EOF { | ||
skipFiles = append(skipFiles, file) | ||
return nil | ||
} | ||
// Skip this file if the first line is not a valid log message | ||
firstItem, err := parseLogItem(firstLine) | ||
|
||
firstItem, err := readFirstValidLog(reader, 0) | ||
if err != nil { | ||
skipFiles = append(skipFiles, file) | ||
return nil | ||
} | ||
// Skip this file if cannot read the last line | ||
lastLine := readLastLine(file) | ||
// Skip this file if the last line is not a valid log message | ||
lastItem, err := parseLogItem(lastLine) | ||
|
||
lastItem, err := readLastValidLog(file, 0) | ||
if err != nil { | ||
skipFiles = append(skipFiles, file) | ||
return nil | ||
} | ||
|
||
// Reset position to the start and skip this file if cannot seek to start | ||
if _, err := file.Seek(0, io.SeekStart); err != nil { | ||
skipFiles = append(skipFiles, file) | ||
return nil | ||
} | ||
|
||
if beginTime > lastItem.Time || endTime < firstItem.Time { | ||
skipFiles = append(skipFiles, file) | ||
} else { | ||
|
@@ -109,18 +103,63 @@ func resolveFiles(logFilePath string, beginTime, endTime int64) ([]logFile, erro | |
} | ||
return nil | ||
}) | ||
|
||
defer func() { | ||
for _, f := range skipFiles { | ||
_ = f.Close() | ||
} | ||
}() | ||
|
||
// Sort by start time | ||
sort.Slice(logFiles, func(i, j int) bool { | ||
return logFiles[i].begin < logFiles[j].begin | ||
}) | ||
return logFiles, err | ||
} | ||
|
||
// parameter tryLines: if value is 0, means unlimited | ||
func readFirstValidLog(reader *bufio.Reader, tryLines int64) (*pb.LogMessage, error) { | ||
var tried int64 | ||
for { | ||
line, err := readLine(reader) | ||
if err != nil { | ||
return nil, err | ||
} | ||
item, err := parseLogItem(line) | ||
if err == nil { | ||
return item, nil | ||
} | ||
tried++ | ||
if tryLines > 0 && tried >= tryLines { | ||
break | ||
} | ||
} | ||
return nil, errors.New("not a valid log file") | ||
} | ||
|
||
// parameter tryLines: if value is 0, means unlimited | ||
func readLastValidLog(file *os.File, tryLines int64) (*pb.LogMessage, error) { | ||
var tried int64 | ||
var endCursor int64 | ||
for { | ||
line := readLineReverse(file, endCursor) | ||
// read out the file | ||
if len(line) == 0 { | ||
break | ||
} | ||
endCursor -= int64(len(line)) | ||
item, err := parseLogItem(line) | ||
if err == nil { | ||
return item, nil | ||
} | ||
tried++ | ||
if tryLines > 0 && tried >= tryLines { | ||
break | ||
} | ||
} | ||
return nil, errors.New("not a valid log file") | ||
} | ||
|
||
// Read a line from a reader. | ||
func readLine(reader *bufio.Reader) (string, error) { | ||
var line, b []byte | ||
|
@@ -136,20 +175,22 @@ func readLine(reader *bufio.Reader) (string, error) { | |
return string(line), nil | ||
} | ||
|
||
func readLastLine(file *os.File) string { | ||
// Read a line from the end of a file. | ||
// TODO: read byte by byte is low efficiency, we can improve it, for example, read 1024 bytes one time | ||
func readLineReverse(file *os.File, endCursor int64) string { | ||
var line []byte | ||
var cursor int64 | ||
var cursor = endCursor | ||
stat, _ := file.Stat() | ||
filesize := stat.Size() | ||
for { | ||
cursor -= 1 | ||
cursor-- | ||
file.Seek(cursor, io.SeekEnd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if the log has appended some new logs after the file is opened when calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch, I think we should add some tests for these scenarios. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, I will do some tests to check it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @lonng , add a test method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will continue to fix it in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed it by |
||
|
||
char := make([]byte, 1) | ||
file.Read(char) | ||
|
||
// stop if we find a line | ||
if cursor != -1 && (char[0] == 10 || char[0] == 13) { | ||
if cursor != endCursor-1 && (char[0] == 10 || char[0] == 13) { | ||
break | ||
} | ||
line = append(line, char[0]) | ||
|
@@ -163,7 +204,7 @@ func readLastLine(file *os.File) string { | |
return string(line) | ||
} | ||
|
||
// Returns LogLevel from string and return LogLevel_Info if | ||
// ParseLogLevel returns LogLevel from string and return LogLevel_Info if | ||
// the string is an invalid level string | ||
func ParseLogLevel(s string) pb.LogLevel { | ||
switch s { | ||
|
@@ -244,6 +285,7 @@ type logIterator struct { | |
fileIndex int | ||
reader *bufio.Reader | ||
pending []*os.File | ||
preTime int64 | ||
} | ||
|
||
// The Close method close all resources the iterator has. | ||
|
@@ -274,21 +316,33 @@ nextLine: | |
iter.reader.Reset(iter.pending[iter.fileIndex]) | ||
continue | ||
} | ||
if len(line) < len(TimeStampLayout) { | ||
line = strings.TrimSpace(line) | ||
if len(line) == 0 { | ||
continue | ||
} | ||
// Skip invalid log item | ||
item, err := parseLogItem(line) | ||
if err != nil { | ||
continue | ||
if iter.preTime == 0 { | ||
continue | ||
} | ||
// handle invalid log | ||
// make whole line as log message with pre time and unknown log_level | ||
baurine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
item = &pb.LogMessage{ | ||
Time: iter.preTime, | ||
Level: pb.LogLevel_UNKNOWN, | ||
Message: line, | ||
} | ||
} else { | ||
iter.preTime = item.Time | ||
} | ||
if item.Time > iter.end { | ||
return nil, io.EOF | ||
} | ||
if item.Time < iter.begin { | ||
continue | ||
} | ||
if iter.levelFlag != 0 && iter.levelFlag&(1<<item.Level) == 0 { | ||
// always keep unknown log_level | ||
if item.Level > pb.LogLevel_UNKNOWN && iter.levelFlag != 0 && iter.levelFlag&(1<<item.Level) == 0 { | ||
continue | ||
} | ||
if len(iter.patterns) > 0 { | ||
|
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.
Can we refine by this way? (not sure whether it has some pitfalls.)
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.
Sure, we can add a todo here.