Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Refactor logsink to make it reusable for migration log transfer #6573
Conversation
|
This appears to be a spurious failure caused by Mongo dropping all connections on Windows. |
|
!!build!! |
|
Weird error from mongo: |
|
!!build!! |
| "gopkg.in/natefinch/lumberjack.v2" | ||
| "github.com/juju/juju/apiserver/common" | ||
| "github.com/juju/juju/apiserver/params" | ||
| "github.com/juju/juju/state" | ||
| ) | ||
| -func newLogSinkHandler(h httpContext, w io.Writer) http.Handler { | ||
| - return &logSinkHandler{ctxt: h, fileLogger: w} | ||
| +type LoggingStrategy interface { |
| } | ||
| -func newLogSinkWriter(logDir string) (io.WriteCloser, error) { | ||
| - logPath := filepath.Join(logDir, "logsink.log") | ||
| +type AgentLoggingStrategy struct { |
babbageclunk
Nov 20, 2016
Member
No, good point. In fact all of this is internal to the package. I left the interface and its methods as public because it feels weird to define a private interface.
| + fileLogger io.Writer | ||
| +} | ||
| + | ||
| +func (s *AgentLoggingStrategy) Authenticate(req *http.Request) error { |
| + return nil | ||
| +} | ||
| + | ||
| +func (s *AgentLoggingStrategy) Start() { |
| + s.dbLogger = state.NewEntityDbLogger(s.st, s.entity, s.version) | ||
| +} | ||
| + | ||
| +func (s *AgentLoggingStrategy) Log(m params.LogRecord) bool { |
| + return dbErr == nil && fileErr == nil | ||
| +} | ||
| + | ||
| +func (s *AgentLoggingStrategy) Stop() { |
howbazaar
Nov 18, 2016
Owner
And here. I'm assuming that once you have stopped, you can never start again?
| @@ -175,9 +218,10 @@ func (h *logSinkHandler) sendError(w io.Writer, req *http.Request, err error) { | ||
| } | ||
| // logToFile writes a single log message to the logsink log file. | ||
| -func (h *logSinkHandler) logToFile(prefix string, m params.LogRecord) error { | ||
| - _, err := h.fileLogger.Write([]byte(strings.Join([]string{ | ||
| +func logToFile(logger io.Writer, prefix string, m params.LogRecord) error { |
howbazaar
Nov 18, 2016
Owner
logger is often used as a global loggo.Logger variable name. Perhaps use "writer" here?
| @@ -189,8 +189,8 @@ func (s *logsinkSuite) TestLogging(c *gc.C) { | ||
| logPath := filepath.Join(s.LogDir, "logsink.log") | ||
| logContents, err := ioutil.ReadFile(logPath) | ||
| c.Assert(err, jc.ErrorIsNil) | ||
| - line0 := modelUUID + " machine-0: 2015-06-01 23:02:01 INFO some.where foo.go:42 all is well\n" | ||
| - line1 := modelUUID + " machine-0: 2015-06-01 23:02:02 ERROR else.where bar.go:99 oh noes\n" | ||
| + line0 := modelUUID + ": machine-0 2015-06-01 23:02:01 INFO some.where foo.go:42 all is well\n" |
babbageclunk
Nov 20, 2016
Member
Because the entity stopped being part of the prefix and started being part of the log record. I got the impression from Menno that the log is for backup but anything that wants the data structured is reading from the DB, so a small change to the format here wouldn't be a problem. Do you think that's right?
howbazaar
Nov 20, 2016
Owner
Yeah, the logsink is purely a last ditch backup if neeeded. It is fine, I was just wondering.
babbageclunk
added some commits
Nov 7, 2016
| func (s *ConfigSuite) TestDefaultsPassValidation(c *gc.C) { | ||
| attrs := testing.FakeConfig().Merge(testing.Attrs{ | ||
| - "type": "rackspace", | ||
| + "type": "rackspace", |
|
$$merge$$ On Mon, 21 Nov 2016, 11:19 Tim Penhey, notifications@github.com wrote:
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
babbageclunk commentedNov 17, 2016
Extract the authentication and logging that the logsink handler does into AgentLoggingStrategy. This enables using a different logging strategy for migration log transfer (which will be in a follow-up PR) - in that case the logs won't be coming from a specific machine agent so each record will include the entity that originally logged it.
This also slightly changes the format of logsink.log - entity is moved from the line prefix to a normal column in the log file.