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

Data races in use of witchcraft logging #56

Open
udgeetham opened this issue Aug 15, 2019 · 1 comment
Open

Data races in use of witchcraft logging #56

udgeetham opened this issue Aug 15, 2019 · 1 comment

Comments

@udgeetham
Copy link

udgeetham commented Aug 15, 2019

We want to incorporate a race detector in our project's integration tests and circle ci to help catch race conditions earlier on. At this point, we can't do so due to races we see related to the witchcraft logger.

We have goroutines that reassign the value of context. This creates a data race to which parameters a log will have at write time. The impact here is that logs are wrapped with params set by another goroutine (ex: one goroutine's log params are rewritten by another goroutine's log params), making it difficult to reliably debug our product and degrades log accuracy.

ctx = svc1log.WithLoggerParams(ctx, svc1log.SafeParams(safeParams))

It's not obvious from the witchraft logging documentation how to correctly use the context and the logging functions. Would like to open up a discussion here on this.

@bmoylan
Copy link
Contributor

bmoylan commented Aug 16, 2019

Contexts are immutable so any changes should create a new context and not change anything about the existing one. Is the issue that the params themselves are pointers whose values are changing?

Are you able to write a test in this repo that demonstrates the problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants