Skip to content
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

Use log timestamps from docker logs. #401

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
21 changes: 18 additions & 3 deletions router/pump.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (p *LogsPump) pumpLogs(event *docker.APIEvents, backlog bool, inactivityTim

// RawTerminal with container Tty=false injects binary headers into
// the log stream that show up as garbage unicode characters
rawTerminal := false
rawTerminal := false
if allowTTY && container.Config.Tty {
rawTerminal = true
}
Expand All @@ -235,6 +235,7 @@ func (p *LogsPump) pumpLogs(event *docker.APIEvents, backlog bool, inactivityTim
Since: sinceTime.Unix(),
InactivityTimeout: inactivityTimeout,
RawTerminal: rawTerminal,
Timestamps: true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set this to true only if this feature is enabled

})
if err != nil {
debug("pump.pumpLogs():", id, "stopped with error:", err)
Expand Down Expand Up @@ -361,10 +362,15 @@ func newContainerPump(container *docker.Container, stdout, stderr io.Reader) *co
}
return
}
logMessage, logTime, err := parseLogLine(line)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseLogLine should only be called if this feature is enabled. Also, if we get an error, perhaps we should fall back to default behaviour of using Time.Now()

if err != nil {
debug("pump.newContainerPump():", normalID(container.ID), ", failed to parse log line:", err)
continue
}
cp.send(&Message{
Data: strings.TrimSuffix(line, "\n"),
Data: logMessage,
Container: container,
Time: time.Now(),
Time: logTime,
gbolo marked this conversation as resolved.
Show resolved Hide resolved
Source: source,
})
}
Expand Down Expand Up @@ -396,3 +402,12 @@ func (cp *containerPump) remove(logstream chan *Message) {
defer cp.Unlock()
delete(cp.logstreams, logstream)
}

func parseLogLine(line string) (string, time.Time, error) {
logEntry := strings.SplitN(strings.TrimSuffix(line, "\n"), " ", 2)
logTime, err := time.Parse(time.RFC3339Nano, logEntry[0])
if err != nil {
return "", time.Time{}, err
}
return logEntry[1], logTime, nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no guarantee that logEntry[1] exists. You should rework this function to check the length of logEntry

}