Remove Windows EventLog logging hook

kevpar · kevpar · commit 5023d96ad2d1 · 2019-08-19T10:24:37.000-07:00
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 &lt;kevpar@microsoft.com&gt;
diff --git 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
