go-kit / go-kit/kit

Using a Windows color writer with an underlying sync writer negates the benefit of the sync writer

Open
#559 3 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Go
Stars
27.4k
Forks
2.4k
PR merge metrics
No merged PRs in 30d

Description

We stumbled upon a weird issue where two goroutines writing to the same go-kit logger would have their logs interleaved even though the logger writes to stdout through a [SyncWriter](https://godoc.org/github.com/go-kit/kit/log#NewSyncWriter).

As it happens our logger was created with [`term.NewColorWriter`](https://godoc.org/github.com/go-kit/kit/log/term#NewColorWriter) which, on **Windows**, is implemented in a [particular fashion that annihilates the benefits of synchronization, as provided by the `SyncWriter`](https://github.com/go-kit/kit/blob/master/log/term/colorwriter_windows.go#L43).

Namely, the writer iterates over each character and performs a single `Write()` for each character. When two coroutines use the same logger in that scenario, the color writer - via the SyncWriter - lock and unlocks the mutex **for each character**. Not is this totally uneffecient, but it also causes the characters to be interleaved as we observed.

I can totally come up with a fix for that in the next couple of days, but I'd like to make sure with you that my approach at solving that is ok to you.

# Suggested solution

Basically, I'd write every character in the Windows color writer inside a `bytes.Buffer` first, then `io.Copy` that buffer to the real underlying writer. This would restore writing atomicity as well as performance.

I don't expect log lines to be memory heavy (at least, not to the extent where this two-pass write would cause a real issue) so I assume this approach is fine.

What do you think ?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.