-
Notifications
You must be signed in to change notification settings - Fork 938
Update from openark/golib to support TZ with logging #1017
Conversation
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.
Hey @jfudally! Please note https://github.com/github/orchestrator/blob/master/vendor/github.com/openark/golib/log/log.go exists, and so you'd like to merge your log.go
changes into that file instead of creating a new log.go
file in a different path.
@shlomi-noach Ah my bad I missed a directory. Fixed now. |
@jfudally in our testing cluster, before deploying this, our logs show:
immediately after deploying:
Notice the 8-hour jump from before/after. Before==PST, after==UTC. On our testing cluster:
should this happen? |
…rator into jfudally-timezone-support
@shlomi-noach
Also, what's the best way to run a snapshot to test the logging that I updated? |
…rator into jfudally-timezone-support
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.
assuming testing works - please merge!
Raft snapshot logging looks good:
And the integrity of the snapshot is unaffected:
|
This PR allows setting the timezone via
TZ
environment variable for localizing logging timestamps:This PR also removes the inconsistent use of the standard
log
package ingo/raft/file_snapshot.go
in favor of theopenark/golib/log
package used by the rest of the application.gofmt
(please avoidgoimports
)./build.sh
go test ./go/...
Closes #1004