logger (& many more, to accommodate): drop explicit syslog.

logger.ConsoleLog printed to standard output and to syslog, an idea which has proven to be at least partly misguided: snapd's logs would typically be duplicated as systemd logs the output already. snap only rarely logged anything relevant to syslog. Aditionally, syslog was always written to (even for DEBUG), which has been deemed a misfeature. This drops logger.ConsoleLog and introudces logger.Log (constructor goes from logger.NewConsoleLog to logger.New); the new type has no connection with syslog. 

7 files changed, 73 insertions, 117 deletions

diff --git a/cmd/snap/cmd_auto_import_test.go b/cmd/snap/cmd_auto_import_test.go
index 8a451a14b8..51b687fa28 100644
--- a/cmd/snap/cmd_auto_import_test.go
+++ b/cmd/snap/cmd_auto_import_test.go
@@ -85,7 +85,7 @@ func (s *SnapSuite) TestAutoImportAssertsHappy(c *C) {
	restore = snap.MockMountInfoPath(makeMockMountInfo(c, content))
	defer restore()
 
-	l, err := logger.NewConsoleLog(s.stderr, 0)
+	l, err := logger.New(s.stderr, 0)
	c.Assert(err, IsNil)
	logger.SetLogger(l)
 
@@ -190,7 +190,7 @@ func (s *SnapSuite) TestAutoImportIntoSpool(c *C) {
	dirs.SetRootDir(c.MkDir())
	defer dirs.SetRootDir("")
 
-	l, err := logger.NewConsoleLog(s.stderr, 0)
+	l, err := logger.New(s.stderr, 0)
	c.Assert(err, IsNil)
	logger.SetLogger(l)
 
@@ -266,7 +266,7 @@ func (s *SnapSuite) TestAutoImportFromSpoolHappy(c *C) {
	err = ioutil.WriteFile(fakeAssertsFn, fakeAssertData, 0644)
	c.Assert(err, IsNil)
 
-	l, err := logger.NewConsoleLog(s.stderr, 0)
+	l, err := logger.New(s.stderr, 0)
	c.Assert(err, IsNil)
	logger.SetLogger(l)
 
@@ -290,7 +290,7 @@ func (s *SnapSuite) TestAutoImportIntoSpoolUnhappyTooBig(c *C) {
	dirs.SetRootDir(c.MkDir())
	defer dirs.SetRootDir("")
 
-	l, err := logger.NewConsoleLog(s.stderr, 0)
+	l, err := logger.New(s.stderr, 0)
	c.Assert(err, IsNil)
	logger.SetLogger(l)
 
diff --git a/httputil/logger_test.go b/httputil/logger_test.go
index 10a14a4dd2..d90bcba9f5 100644
--- a/httputil/logger_test.go
+++ b/httputil/logger_test.go
@@ -51,7 +51,7 @@ func (loggerSuite) TearDownTest(c *check.C) {
 func (s *loggerSuite) SetUpTest(c *check.C) {
	os.Setenv("SNAPD_DEBUG", "true")
	s.logbuf = bytes.NewBuffer(nil)
-	l, err := logger.NewConsoleLog(s.logbuf, logger.DefaultFlags)
+	l, err := logger.New(s.logbuf, logger.DefaultFlags)
	c.Assert(err, check.IsNil)
	logger.SetLogger(l)
 }
diff --git a/logger/export_test.go b/logger/export_test.go
new file mode 100644
index 0000000000..cd238bf7c2
--- /dev/null
+++ b/logger/export_test.go
@@ -0,0 +1,27 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2014,2015,2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package logger
+
+func GetLogger() Logger {
+	lock.Lock()
+	defer lock.Unlock()
+
+	return logger
+}
diff --git a/logger/logger.go b/logger/logger.go
index 6730b07e1c..216aa79ad7 100644
--- a/logger/logger.go
+++ b/logger/logger.go
@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2014-2015 Canonical Ltd
+ * Copyright (C) 2014,2015,2017 Canonical Ltd
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License version 3 as
@@ -22,9 +22,7 @@ package logger
 import (
	"fmt"
	"io"
-	"io/ioutil"
	"log"
-	"log/syslog"
	"os"
	"sync"
 
@@ -42,10 +40,6 @@ type Logger interface {
 const (
	// DefaultFlags are passed to the default console log.Logger
	DefaultFlags = log.Ldate | log.Ltime | log.Lmicroseconds | log.Lshortfile
-	// SyslogFlags are passed to the default syslog log.Logger
-	SyslogFlags = log.Lshortfile
-	// SyslogPriority for the default syslog log.Logger
-	SyslogPriority = syslog.LOG_DEBUG | syslog.LOG_USER
 )
 
 type nullLogger struct{}
@@ -100,59 +94,32 @@ func SetLogger(l Logger) {
	logger = l
 }
 
-// ConsoleLog sends Notices to a log.Logger and Debugs to syslog
-type ConsoleLog struct {
+type Log struct {
	log *log.Logger
-	sys *log.Logger
 }
 
-// Debug sends the msg to syslog
-func (l *ConsoleLog) Debug(msg string) {
-	s := "DEBUG: " + msg
-	l.sys.Output(3, s)
-
+// Debug only prints if SNAPD_DEBUG is set
+func (l Log) Debug(msg string) {
	if osutil.GetenvBool("SNAPD_DEBUG") {
-	l.log.Output(3, s)
+	l.log.Output(3, "DEBUG: "+msg)
	}
 }
 
 // Notice alerts the user about something, as well as putting it syslog
-func (l *ConsoleLog) Notice(msg string) {
-	l.sys.Output(3, msg)
+func (l Log) Notice(msg string) {
	l.log.Output(3, msg)
 }
 
-// variable to allow mocking the syslog.NewLogger call in the tests
-var newSyslog = newSyslogImpl
-
-func newSyslogImpl() (*log.Logger, error) {
-	return syslog.NewLogger(SyslogPriority, SyslogFlags)
-}
-
-// NewConsoleLog creates a ConsoleLog with a log.Logger using the given
-// io.Writer and flag, and a syslog.Writer.
-func NewConsoleLog(w io.Writer, flag int) (*ConsoleLog, error) {
-	clog := log.New(w, "", flag)
-
-	sys, err := newSyslog()
-	if err != nil {
-	clog.Output(3, "WARNING: cannot create syslog logger")
-	sys = log.New(ioutil.Discard, "", flag)
-	}
-
-	return &ConsoleLog{
-	log: clog,
-	sys: sys,
-	}, nil
+// New creates a log.Logger using the given io.Writer and flag.
+func New(w io.Writer, flag int) (Logger, error) {
+	return Log{log: log.New(w, "", flag)}, nil
 }
 
 // SimpleSetup creates the default (console) logger
 func SimpleSetup() error {
-	l, err := NewConsoleLog(os.Stderr, DefaultFlags)
-	if err != nil {
-	return err
+	l, err := New(os.Stderr, DefaultFlags)
+	if err == nil {
+	SetLogger(l)
	}
-	SetLogger(l)
-
-	return nil
+	return err
 }
diff --git a/logger/logger_test.go b/logger/logger_test.go
index 2189442343..9996f04f62 100644
--- a/logger/logger_test.go
+++ b/logger/logger_test.go
@@ -17,18 +17,17 @@
 *
 */
 
-package logger
+package logger_test
 
 import (
	"bytes"
-	"fmt"
-	"log"
	"os"
	"testing"
 
-	"github.com/snapcore/snapd/testutil"
-
	. "gopkg.in/check.v1"
+
+	"github.com/snapcore/snapd/logger"
+	"github.com/snapcore/snapd/testutil"
 )
 
 // Hook up check.v1 into the "go test" runner
@@ -37,116 +36,79 @@ func Test(t *testing.T) { TestingT(t) }
 var _ = Suite(&LogSuite{})
 
 type LogSuite struct {
-	sysbuf *bytes.Buffer
+	oldLogger logger.Logger
 }
 
 func (s *LogSuite) SetUpTest(c *C) {
-	c.Assert(logger, Equals, NullLogger)
-
-	// we do not want to pollute syslog in our tests (and sbuild
-	// will also not let us do that)
-	newSyslog = func() (*log.Logger, error) {
-	s.sysbuf = bytes.NewBuffer(nil)
-	return log.New(s.sysbuf, "", SyslogFlags), nil
-	}
+	s.oldLogger = logger.GetLogger()
+	logger.SetLogger(logger.NullLogger)
 }
 
 func (s *LogSuite) TearDownTest(c *C) {
-	SetLogger(NullLogger)
-	newSyslog = newSyslogImpl
+	logger.SetLogger(s.oldLogger)
 }
 
 func (s *LogSuite) TestDefault(c *C) {
-	if logger != nil {
-	SetLogger(nil)
+	if logger.GetLogger() != nil {
+	logger.SetLogger(nil)
	}
-	c.Check(logger, IsNil)
+	c.Check(logger.GetLogger(), IsNil)
 
-	err := SimpleSetup()
+	err := logger.SimpleSetup()
	c.Check(err, IsNil)
-	c.Check(logger, NotNil)
-	SetLogger(nil)
+	c.Check(logger.GetLogger(), NotNil)
 }
 
 func (s *LogSuite) TestNew(c *C) {
	var buf bytes.Buffer
-	l, err := NewConsoleLog(&buf, DefaultFlags)
+	l, err := logger.New(&buf, logger.DefaultFlags)
	c.Assert(err, IsNil)
	c.Assert(l, NotNil)
-	c.Check(l.sys, NotNil)
-	c.Check(l.log, NotNil)
 }
 
 func (s *LogSuite) TestDebugf(c *C) {
	var logbuf bytes.Buffer
-	l, err := NewConsoleLog(&logbuf, DefaultFlags)
+	l, err := logger.New(&logbuf, logger.DefaultFlags)
	c.Assert(err, IsNil)
 
-	SetLogger(l)
+	logger.SetLogger(l)
 
-	Debugf("xyzzy")
-	c.Check(s.sysbuf.String(), Matches, `(?m).*logger_test\.go:\d+: DEBUG: xyzzy`)
+	logger.Debugf("xyzzy")
	c.Check(logbuf.String(), Equals, "")
 }
 
 func (s *LogSuite) TestDebugfEnv(c *C) {
	var logbuf bytes.Buffer
-	l, err := NewConsoleLog(&logbuf, DefaultFlags)
+	l, err := logger.New(&logbuf, logger.DefaultFlags)
	c.Assert(err, IsNil)
 
-	SetLogger(l)
+	logger.SetLogger(l)
 
	os.Setenv("SNAPD_DEBUG", "1")
	defer os.Unsetenv("SNAPD_DEBUG")
 
-	Debugf("xyzzy")
-	c.Check(s.sysbuf.String(), Matches, `(?m).*logger_test\.go:\d+: DEBUG: xyzzy`)
+	logger.Debugf("xyzzy")
	c.Check(logbuf.String(), testutil.Contains, `DEBUG: xyzzy`)
 }
 
 func (s *LogSuite) TestNoticef(c *C) {
	var logbuf bytes.Buffer
-	l, err := NewConsoleLog(&logbuf, DefaultFlags)
+	l, err := logger.New(&logbuf, logger.DefaultFlags)
	c.Assert(err, IsNil)
 
-	SetLogger(l)
+	logger.SetLogger(l)
 
-	Noticef("xyzzy")
-	c.Check(s.sysbuf.String(), Matches, `(?m).*logger_test\.go:\d+: xyzzy`)
+	logger.Noticef("xyzzy")
	c.Check(logbuf.String(), Matches, `(?m).*logger_test\.go:\d+: xyzzy`)
 }
 
 func (s *LogSuite) TestPanicf(c *C) {
	var logbuf bytes.Buffer
-	l, err := NewConsoleLog(&logbuf, DefaultFlags)
+	l, err := logger.New(&logbuf, logger.DefaultFlags)
	c.Assert(err, IsNil)
 
-	SetLogger(l)
+	logger.SetLogger(l)
 
-	c.Check(func() { Panicf("xyzzy") }, Panics, "xyzzy")
-	c.Check(s.sysbuf.String(), Matches, `(?m).*logger_test\.go:\d+: PANIC xyzzy`)
+	c.Check(func() { logger.Panicf("xyzzy") }, Panics, "xyzzy")
	c.Check(logbuf.String(), Matches, `(?m).*logger_test\.go:\d+: PANIC xyzzy`)
 }
-
-func (s *LogSuite) TestSyslogFails(c *C) {
-	var logbuf bytes.Buffer
-
-	// pretend syslog is not available (e.g. because of no /dev/log in
-	// a chroot or something)
-	newSyslog = func() (*log.Logger, error) {
-	return nil, fmt.Errorf("nih nih")
-	}
-
-	// ensure a warning is displayed
-	l, err := NewConsoleLog(&logbuf, DefaultFlags)
-	c.Assert(err, IsNil)
-	c.Check(logbuf.String(), Matches, `(?m).*:\d+: WARNING: cannot create syslog logger`)
-
-	// ensure that even without a syslog the console log works and we
-	// do not crash
-	logbuf.Reset()
-	SetLogger(l)
-	Noticef("I do not want to crash")
-	c.Check(logbuf.String(), Matches, `(?m).*logger_test\.go:\d+: I do not want to crash`)
-
-}
diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go
index 25c407bfc7..2747fd9b5a 100644
--- a/overlord/snapstate/snapstate_test.go
+++ b/overlord/snapstate/snapstate_test.go
@@ -4384,7 +4384,7 @@ func (s *snapmgrTestSuite) TestEnsureRefreshRefusesWeekdaySchedules(c *C) {
	snapstate.CanAutoRefresh = func(*state.State) (bool, error) { return true, nil }
 
	logbuf := bytes.NewBuffer(nil)
-	l, err := logger.NewConsoleLog(logbuf, logger.DefaultFlags)
+	l, err := logger.New(logbuf, logger.DefaultFlags)
	c.Assert(err, IsNil)
	logger.SetLogger(l)
	defer logger.SetLogger(logger.NullLogger)
diff --git a/store/store_test.go b/store/store_test.go
index 031337c76d..082ce277a7 100644
--- a/store/store_test.go
+++ b/store/store_test.go
@@ -226,7 +226,7 @@ func (t *remoteRepoTestSuite) SetUpTest(c *C) {
	t.AddCleanup(func() { os.Setenv("SNAPD_DEBUG", oldSnapdDebug) })
 
	t.logbuf = bytes.NewBuffer(nil)
-	l, err := logger.NewConsoleLog(t.logbuf, logger.DefaultFlags)
+	l, err := logger.New(t.logbuf, logger.DefaultFlags)
	c.Assert(err, IsNil)
	logger.SetLogger(l)