From 5023d96ad2d1cd948ca87a78cba5478656945f08 Mon Sep 17 00:00:00 2001 From: Kevin Parsons Date: Mon, 19 Aug 2019 10:24:37 -0700 Subject: [PATCH] Remove Windows EventLog logging hook EventLog is very old and provides a poor experience. We have supported ETW for logging for a while, which is much better. We have also observed an issue where EventLog keeps containerd.exe open, preventing containerd from being upgraded to a new version. Due to all of this, it makes sense to remove the old EventLog hook in favor of using ETW logging on Windows as the primary diagnostic experience. Signed-off-by: Kevin Parsons --- cmd/containerd/command/service_windows.go | 118 +--------------------- 1 file changed, 1 insertion(+), 117 deletions(-) diff --git a/cmd/containerd/command/service_windows.go b/cmd/containerd/command/service_windows.go index 7b417ba57..f759a7131 100644 --- a/cmd/containerd/command/service_windows.go +++ b/cmd/containerd/command/service_windows.go @@ -17,7 +17,6 @@ package command import ( - "bytes" "fmt" "io/ioutil" "log" @@ -35,7 +34,6 @@ import ( "golang.org/x/sys/windows" "golang.org/x/sys/windows/svc" "golang.org/x/sys/windows/svc/debug" - "golang.org/x/sys/windows/svc/eventlog" "golang.org/x/sys/windows/svc/mgr" ) @@ -54,18 +52,6 @@ var ( service *handler ) -const ( - // These should match the values in event_messages.mc. - eventInfo = 1 - eventWarn = 1 - eventError = 1 - eventDebug = 2 - eventPanic = 3 - eventFatal = 4 - - eventExtraOffset = 10 // Add this to any event to get a string that supports extended data -) - // serviceFlags returns an array of flags for configuring containerd to run // as a Windows service under control of SCM. func serviceFlags() []cli.Flag { @@ -124,93 +110,6 @@ type handler struct { done chan struct{} // Indicates back to app main to quit } -type etwHook struct { - log *eventlog.Log -} - -func (h *etwHook) Levels() []logrus.Level { - return []logrus.Level{ - logrus.PanicLevel, - logrus.FatalLevel, - logrus.ErrorLevel, - logrus.WarnLevel, - logrus.InfoLevel, - logrus.DebugLevel, - } -} - -func (h *etwHook) Fire(e *logrus.Entry) error { - var ( - etype uint16 - eid uint32 - ) - - switch e.Level { - case logrus.PanicLevel: - etype = windows.EVENTLOG_ERROR_TYPE - eid = eventPanic - case logrus.FatalLevel: - etype = windows.EVENTLOG_ERROR_TYPE - eid = eventFatal - case logrus.ErrorLevel: - etype = windows.EVENTLOG_ERROR_TYPE - eid = eventError - case logrus.WarnLevel: - etype = windows.EVENTLOG_WARNING_TYPE - eid = eventWarn - case logrus.InfoLevel: - etype = windows.EVENTLOG_INFORMATION_TYPE - eid = eventInfo - case logrus.DebugLevel: - etype = windows.EVENTLOG_INFORMATION_TYPE - eid = eventDebug - default: - return errors.Wrap(errdefs.ErrInvalidArgument, "unknown level") - } - - // If there is additional data, include it as a second string. - exts := "" - if len(e.Data) > 0 { - fs := bytes.Buffer{} - for k, v := range e.Data { - fs.WriteString(k) - fs.WriteByte('=') - fmt.Fprint(&fs, v) - fs.WriteByte(' ') - } - - exts = fs.String()[:fs.Len()-1] - eid += eventExtraOffset - } - - if h.log == nil { - fmt.Fprintf(os.Stderr, "%s [%s]\n", e.Message, exts) - return nil - } - - var ( - ss [2]*uint16 - err error - ) - - ss[0], err = windows.UTF16PtrFromString(e.Message) - if err != nil { - return err - } - - count := uint16(1) - if exts != "" { - ss[1], err = windows.UTF16PtrFromString(exts) - if err != nil { - return err - } - - count++ - } - - return windows.ReportEvent(h.log.Handle, etype, 0, eid, 0, count, 0, &ss[0], nil) -} - func getServicePath() (string, error) { p, err := exec.LookPath(os.Args[0]) if err != nil { @@ -283,7 +182,7 @@ func registerService() error { return err } - return eventlog.Install(serviceNameFlag, p, false, eventlog.Info|eventlog.Warning|eventlog.Error) + return nil } func unregisterService() error { @@ -299,7 +198,6 @@ func unregisterService() error { } defer s.Close() - eventlog.Remove(serviceNameFlag) err = s.Delete() if err != nil { return err @@ -345,20 +243,6 @@ func registerUnregisterService(root string) (bool, error) { return true, err } - interactive, err := svc.IsAnInteractiveSession() - if err != nil { - return true, err - } - - var log *eventlog.Log - if !interactive { - log, err = eventlog.Open(serviceNameFlag) - if err != nil { - return true, err - } - } - - logrus.AddHook(&etwHook{log}) logrus.SetOutput(ioutil.Discard) } return false, nil