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

promtail: Allow docker_sd_config to handle log lines longer than 64k #6256

Merged
merged 1 commit into from
Jun 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions clients/pkg/promtail/targets/docker/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,38 @@ func extractTs(line string) (time.Time, string, error) {
return ts, pair[1], nil
}

// https://devmarkpro.com/working-big-files-golang
func readLine(r *bufio.Reader) (string, error) {
var (
isPrefix = true
err error
line, ln []byte
)

for isPrefix && err == nil {
line, isPrefix, err = r.ReadLine()
ln = append(ln, line...)
}

return string(ln), err
}
Comment on lines +151 to +165
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind putting this function into a utility package, e.g. clients/pkg/promtail/common? I think it may be used in various places.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fine either way on that actually, we can also wait until we need it somewhere else to figure out the best package for it to go into, or what the interface should look like, ie. there might be more duplication to reduce if we refactor our the section in process for processing a whole file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm fine either way, happy to move it if that's the desired approach...


func (t *Target) process(r io.Reader, logStream string) {
defer func() {
t.wg.Done()
}()

scanner := bufio.NewScanner(r)
for scanner.Scan() {
line := scanner.Text()
reader := bufio.NewReader(r)
for {
line, err := readLine(reader)
if err != nil {
if err == io.EOF {
break
}
level.Error(t.logger).Log("msg", "error reading docker log line, skipping line", "err", err)
t.metrics.dockerErrors.Inc()
}

ts, line, err := extractTs(line)
if err != nil {
level.Error(t.logger).Log("msg", "could not extract timestamp, skipping line", "err", err)
Expand Down Expand Up @@ -189,12 +213,6 @@ func (t *Target) process(r io.Reader, logStream string) {
t.metrics.dockerEntries.Inc()
t.positions.Put(positions.CursorKey(t.containerName), ts.Unix())
}

err := scanner.Err()
if err != nil {
level.Warn(t.logger).Log("msg", "finished scanning logs lines with an error", "err", err)
}

}

// startIfNotRunning starts processing container logs. The operation is idempotent , i.e. the processing cannot be started twice.
Expand Down
3 changes: 2 additions & 1 deletion clients/pkg/promtail/targets/docker/targetmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func Test_TargetManager(t *testing.T) {
require.True(t, ta.Ready())

require.Eventually(t, func() bool {
return len(entryHandler.Received()) >= 5
return len(entryHandler.Received()) >= 6
Copy link
Collaborator

Choose a reason for hiding this comment

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

what caused this value to change? I don't see any changes to the test setup.

Copy link
Contributor Author

@srstsavage srstsavage Jun 1, 2022

Choose a reason for hiding this comment

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

I inserted a Very Long Log Line into flog.log at line 6 to test against (over 65k). I had to increase by one here to read in that extra line.

Copy link
Collaborator

Choose a reason for hiding this comment

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

got it, and that's a binary file which is why I'm not seeing the diff, thanks!

}, 20*time.Second, 100*time.Millisecond)

received := entryHandler.Received()
Expand All @@ -109,4 +109,5 @@ func Test_TargetManager(t *testing.T) {
actualLines = append(actualLines, entry.Line)
}
require.ElementsMatch(t, actualLines, expectedLines)
require.Equal(t, 99969, len(received[5].Line))
}
Binary file modified clients/pkg/promtail/targets/docker/testdata/flog.log
Binary file not shown.