From 84ef35cedb89258165ce49663e6b48a9b04b1042 Mon Sep 17 00:00:00 2001
From: Silas Davis <silas@erisindustries.com>
Date: Tue, 29 Aug 2017 16:57:01 +0100
Subject: [PATCH] Fix various logging issues exposed by PaaS deployment

---
 .circleci/config.yml                          |  1 +
 glide.lock                                    |  2 +-
 glide.yaml                                    |  2 +-
 logging/adapters/tendermint_log15/capture.go  | 63 ++++++++++++++
 logging/adapters/tendermint_log15/convert.go  | 83 -------------------
 logging/config/sinks.go                       |  5 ++
 logging/errors/multiple_errors.go             | 24 ++++++
 logging/lifecycle/lifecycle.go                | 38 ++++++---
 logging/lifecycle/lifecycle_test.go           | 49 ++++++++---
 logging/loggers/channel_logger.go             | 27 ++++--
 logging/loggers/channel_logger_test.go        | 12 ++-
 logging/loggers/graylog_logger.go             |  1 -
 logging/loggers/info_trace_logger.go          | 24 +++---
 logging/loggers/info_trace_logger_test.go     |  2 +-
 logging/loggers/multiple_output_logger.go     | 26 +-----
 .../loggers/multiple_output_logger_test.go    |  6 +-
 ...int_log15_loggers.go => output_loggers.go} | 22 +++--
 logging/loggers/shared_test.go                |  2 +-
 manager/burrow-mint/state/permissions_test.go |  2 +-
 rpc/tendermint/test/shared.go                 |  5 +-
 rpc/v0/rest_server_test.go                    |  3 +-
 test/server/scumbag_test.go                   |  3 +-
 22 files changed, 234 insertions(+), 168 deletions(-)
 delete mode 100644 logging/adapters/tendermint_log15/convert.go
 create mode 100644 logging/errors/multiple_errors.go
 delete mode 100644 logging/loggers/graylog_logger.go
 rename logging/loggers/{tendermint_log15_loggers.go => output_loggers.go} (74%)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index d850e99c..5ac863b8 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -82,6 +82,7 @@ workflows:
             - test
             - test_integration
           filters:
+            <<: *tags_filters
             branches:
               # Although we seem to exclude the master branch below, since
               # matching on tags is independent we will still build tags that
diff --git a/glide.lock b/glide.lock
index e03eb97b..b6a1515c 100644
--- a/glide.lock
+++ b/glide.lock
@@ -39,7 +39,7 @@ imports:
   - binding
   - render
 - name: github.com/go-kit/kit
-  version: fadad6fffe0466b19df9efd9acde5c9a52df5fa4
+  version: a9ca6725cbbea455e61c6bc8a1ed28e81eb3493b
   subpackages:
   - log
 - name: github.com/go-logfmt/logfmt
diff --git a/glide.yaml b/glide.yaml
index c97075eb..53641e17 100644
--- a/glide.yaml
+++ b/glide.yaml
@@ -17,7 +17,7 @@ import:
   subpackages:
   - http2
 - package: github.com/go-kit/kit
-  version: ~0.4.0
+  version: ~0.5.0
 - package: github.com/eapache/channels
   version: ~1.1.0
 - package: github.com/go-logfmt/logfmt
diff --git a/logging/adapters/tendermint_log15/capture.go b/logging/adapters/tendermint_log15/capture.go
index 3534ca3e..1ad8550f 100644
--- a/logging/adapters/tendermint_log15/capture.go
+++ b/logging/adapters/tendermint_log15/capture.go
@@ -15,8 +15,13 @@
 package tendermint_log15
 
 import (
+	"time"
+
 	kitlog "github.com/go-kit/kit/log"
+	"github.com/go-stack/stack"
+	"github.com/hyperledger/burrow/logging/structure"
 	"github.com/hyperledger/burrow/logging/types"
+	. "github.com/hyperledger/burrow/util/slice"
 	"github.com/tendermint/log15"
 )
 
@@ -59,3 +64,61 @@ func InfoTraceLoggerAsLog15Handler(logger types.InfoTraceLogger) log15.Handler {
 		logger: logger,
 	}
 }
+
+// Convert a go-kit log line (i.e. keyvals... interface{}) into a log15 record
+// This allows us to use log15 output handlers
+func LogLineToRecord(keyvals ...interface{}) *log15.Record {
+	vals, ctx := structure.ValuesAndContext(keyvals, structure.TimeKey,
+		structure.MessageKey, structure.CallerKey, structure.LevelKey)
+
+	// Mapping of log line to Record is on a best effort basis
+	theTime, _ := vals[structure.TimeKey].(time.Time)
+	call, _ := vals[structure.CallerKey].(stack.Call)
+	level, _ := vals[structure.LevelKey].(string)
+	message, _ := vals[structure.MessageKey].(string)
+
+	return &log15.Record{
+		Time: theTime,
+		Lvl:  Log15LvlFromString(level),
+		Msg:  message,
+		Call: call,
+		Ctx:  ctx,
+		KeyNames: log15.RecordKeyNames{
+			Time: structure.TimeKey,
+			Msg:  structure.MessageKey,
+			Lvl:  structure.LevelKey,
+		}}
+}
+
+// Convert a log15 record to a go-kit log line (i.e. keyvals... interface{})
+// This allows us to capture output from dependencies using log15
+func RecordToLogLine(record *log15.Record) []interface{} {
+	return Concat(
+		Slice(
+			structure.CallerKey, record.Call,
+			structure.LevelKey, record.Lvl.String(),
+		),
+		record.Ctx,
+		Slice(
+			structure.MessageKey, record.Msg,
+		))
+}
+
+// Collapse our weak notion of leveling and log15's into a log15.Lvl
+func Log15LvlFromString(level string) log15.Lvl {
+	if level == "" {
+		return log15.LvlDebug
+	}
+	switch level {
+	case types.InfoLevelName:
+		return log15.LvlInfo
+	case types.TraceLevelName:
+		return log15.LvlDebug
+	default:
+		lvl, err := log15.LvlFromString(level)
+		if err == nil {
+			return lvl
+		}
+		return log15.LvlDebug
+	}
+}
diff --git a/logging/adapters/tendermint_log15/convert.go b/logging/adapters/tendermint_log15/convert.go
deleted file mode 100644
index 95ec0756..00000000
--- a/logging/adapters/tendermint_log15/convert.go
+++ /dev/null
@@ -1,83 +0,0 @@
-// Copyright 2017 Monax Industries Limited
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package tendermint_log15
-
-import (
-	"time"
-
-	"github.com/go-stack/stack"
-	"github.com/hyperledger/burrow/logging/structure"
-	"github.com/hyperledger/burrow/logging/types"
-	. "github.com/hyperledger/burrow/util/slice"
-	"github.com/tendermint/log15"
-)
-
-// Convert a go-kit log line (i.e. keyvals... interface{}) into a log15 record
-// This allows us to use log15 output handlers
-func LogLineToRecord(keyvals ...interface{}) *log15.Record {
-	vals, ctx := structure.ValuesAndContext(keyvals, structure.TimeKey,
-		structure.MessageKey, structure.CallerKey, structure.LevelKey)
-
-	// Mapping of log line to Record is on a best effort basis
-	theTime, _ := vals[structure.TimeKey].(time.Time)
-	call, _ := vals[structure.CallerKey].(stack.Call)
-	level, _ := vals[structure.LevelKey].(string)
-	message, _ := vals[structure.MessageKey].(string)
-
-	return &log15.Record{
-		Time: theTime,
-		Lvl:  Log15LvlFromString(level),
-		Msg:  message,
-		Call: call,
-		Ctx:  ctx,
-		KeyNames: log15.RecordKeyNames{
-			Time: structure.TimeKey,
-			Msg:  structure.MessageKey,
-			Lvl:  structure.LevelKey,
-		}}
-}
-
-// Convert a log15 record to a go-kit log line (i.e. keyvals... interface{})
-// This allows us to capture output from dependencies using log15
-func RecordToLogLine(record *log15.Record) []interface{} {
-	return Concat(
-		Slice(
-			structure.CallerKey, record.Call,
-			structure.LevelKey, record.Lvl.String(),
-		),
-		record.Ctx,
-		Slice(
-			structure.MessageKey, record.Msg,
-		))
-}
-
-// Collapse our weak notion of leveling and log15's into a log15.Lvl
-func Log15LvlFromString(level string) log15.Lvl {
-	if level == "" {
-		return log15.LvlDebug
-	}
-	switch level {
-	case types.InfoLevelName:
-		return log15.LvlInfo
-	case types.TraceLevelName:
-		return log15.LvlDebug
-	default:
-		lvl, err := log15.LvlFromString(level)
-		if err == nil {
-			return lvl
-		}
-		return log15.LvlDebug
-	}
-}
diff --git a/logging/config/sinks.go b/logging/config/sinks.go
index 96da4c42..fbea38d8 100644
--- a/logging/config/sinks.go
+++ b/logging/config/sinks.go
@@ -170,6 +170,11 @@ func (sinkConfig *SinkConfig) SetOutput(output *OutputConfig) *SinkConfig {
 	return sinkConfig
 }
 
+func (outputConfig *OutputConfig) SetFormat(format string) *OutputConfig {
+	outputConfig.Format = format
+	return outputConfig
+}
+
 func StdoutOutput() *OutputConfig {
 	return &OutputConfig{
 		OutputType: Stdout,
diff --git a/logging/errors/multiple_errors.go b/logging/errors/multiple_errors.go
new file mode 100644
index 00000000..905da55f
--- /dev/null
+++ b/logging/errors/multiple_errors.go
@@ -0,0 +1,24 @@
+package errors
+
+import "strings"
+
+type MultipleErrors []error
+
+func CombineErrors(errs []error) error {
+	switch len(errs) {
+	case 0:
+		return nil
+	case 1:
+		return errs[0]
+	default:
+		return MultipleErrors(errs)
+	}
+}
+
+func (errs MultipleErrors) Error() string {
+	var errStrings []string
+	for _, err := range errs {
+		errStrings = append(errStrings, err.Error())
+	}
+	return strings.Join(errStrings, ";")
+}
diff --git a/logging/lifecycle/lifecycle.go b/logging/lifecycle/lifecycle.go
index c8f7f8cd..bd9d6a83 100644
--- a/logging/lifecycle/lifecycle.go
+++ b/logging/lifecycle/lifecycle.go
@@ -20,14 +20,17 @@ import (
 
 	"time"
 
-	"github.com/hyperledger/burrow/logging"
 	"github.com/hyperledger/burrow/logging/adapters/stdlib"
 	tmLog15adapter "github.com/hyperledger/burrow/logging/adapters/tendermint_log15"
 	"github.com/hyperledger/burrow/logging/config"
 	"github.com/hyperledger/burrow/logging/loggers"
 	"github.com/hyperledger/burrow/logging/structure"
 
+	"fmt"
+
+	"github.com/eapache/channels"
 	kitlog "github.com/go-kit/kit/log"
+	"github.com/hyperledger/burrow/logging"
 	"github.com/hyperledger/burrow/logging/types"
 	"github.com/streadway/simpleuuid"
 	tmLog15 "github.com/tendermint/log15"
@@ -38,14 +41,27 @@ import (
 
 // Obtain a logger from a LoggingConfig
 func NewLoggerFromLoggingConfig(loggingConfig *config.LoggingConfig) (types.InfoTraceLogger, error) {
+	var logger types.InfoTraceLogger
+	var errCh channels.Channel
 	if loggingConfig == nil {
-		return NewStdErrLogger(), nil
-	}
-	outputLogger, err := infoTraceLoggerFromLoggingConfig(loggingConfig)
-	if err != nil {
-		return nil, err
+		logger, errCh = NewStdErrLogger()
+	} else {
+		outputLogger, err := infoTraceLoggerFromLoggingConfig(loggingConfig)
+		if err != nil {
+			return nil, err
+		}
+		logger, errCh = NewLogger(outputLogger)
 	}
-	return NewLogger(outputLogger), nil
+	go func() {
+		err := <-errCh.Out()
+		if err != nil {
+			logger.Info("logging_error", err,
+				"logging_config", loggingConfig.RootTOMLString(),
+				"logger", fmt.Sprintf("%#v", logger))
+		}
+	}()
+
+	return logger, nil
 }
 
 // Hot swap logging config by replacing output loggers of passed InfoTraceLogger
@@ -60,21 +76,21 @@ func SwapOutputLoggersFromLoggingConfig(logger types.InfoTraceLogger,
 	return nil
 }
 
-func NewStdErrLogger() types.InfoTraceLogger {
+func NewStdErrLogger() (types.InfoTraceLogger, channels.Channel) {
 	logger := loggers.NewStreamLogger(os.Stderr, "terminal")
 	return NewLogger(logger)
 }
 
 // Provided a standard logger that outputs to the supplied underlying outputLogger
-func NewLogger(outputLogger kitlog.Logger) types.InfoTraceLogger {
-	infoTraceLogger := loggers.NewInfoTraceLogger(outputLogger)
+func NewLogger(outputLogger kitlog.Logger) (types.InfoTraceLogger, channels.Channel) {
+	infoTraceLogger, errCh := loggers.NewInfoTraceLogger(outputLogger)
 	// Create a random ID based on start time
 	uuid, _ := simpleuuid.NewTime(time.Now())
 	var runId string
 	if uuid != nil {
 		runId = uuid.String()
 	}
-	return logging.WithMetadata(infoTraceLogger.With(structure.RunId, runId))
+	return logging.WithMetadata(infoTraceLogger.With(structure.RunId, runId)), errCh
 }
 
 func CaptureTendermintLog15Output(infoTraceLogger types.InfoTraceLogger) {
diff --git a/logging/lifecycle/lifecycle_test.go b/logging/lifecycle/lifecycle_test.go
index 9c88ce3b..f0de3789 100644
--- a/logging/lifecycle/lifecycle_test.go
+++ b/logging/lifecycle/lifecycle_test.go
@@ -6,10 +6,45 @@ import (
 
 	"bufio"
 
+	. "github.com/hyperledger/burrow/logging/config"
 	"github.com/stretchr/testify/assert"
+	"github.com/tendermint/log15"
 )
 
 func TestNewLoggerFromLoggingConfig(t *testing.T) {
+	reader := CaptureStderr(t, func() {
+		logger, err := NewLoggerFromLoggingConfig(nil)
+		assert.NoError(t, err)
+		logger.Info("Quick", "Test")
+	})
+	line, _, err := reader.ReadLine()
+	assert.NoError(t, err)
+	lineString := string(line)
+	assert.NotEmpty(t, lineString)
+}
+
+func TestCaptureTendermintLog15Output(t *testing.T) {
+	reader := CaptureStderr(t, func() {
+		loggingConfig := &LoggingConfig{
+			RootSink: Sink().
+				SetOutput(StderrOutput().SetFormat("logfmt")).
+				SetTransform(FilterTransform(ExcludeWhenAllMatch,
+					"log_channel", "Trace",
+				)),
+		}
+		outputLogger, err := NewLoggerFromLoggingConfig(loggingConfig)
+		assert.NoError(t, err)
+		CaptureTendermintLog15Output(outputLogger)
+		log15Logger := log15.New()
+		log15Logger.Info("bar", "number_of_forks", 2)
+	})
+	line, _, err := reader.ReadLine()
+	assert.NoError(t, err)
+	assert.Contains(t, string(line), "number_of_forks=2")
+	assert.Contains(t, string(line), "message=bar")
+}
+
+func CaptureStderr(t *testing.T, runner func()) *bufio.Reader {
 	stderr := os.Stderr
 	defer func() {
 		os.Stderr = stderr
@@ -17,14 +52,8 @@ func TestNewLoggerFromLoggingConfig(t *testing.T) {
 	r, w, err := os.Pipe()
 	assert.NoError(t, err, "Couldn't make fifo")
 	os.Stderr = w
-	logger, err := NewLoggerFromLoggingConfig(nil)
-	assert.NoError(t, err)
-	logger.Info("Quick", "Test")
-	reader := bufio.NewReader(r)
-	assert.NoError(t, err)
-	line, _, err := reader.ReadLine()
-	assert.NoError(t, err)
-	// This test shouldn't really depend on colour codes, if you find yourself
-	// changing it then assert.NotEmpty() should do
-	assert.Contains(t, string(line), "\x1b[34mQuick\x1b[0m=Test")
+
+	runner()
+
+	return bufio.NewReader(r)
 }
diff --git a/logging/loggers/channel_logger.go b/logging/loggers/channel_logger.go
index da26a81b..47aeb381 100644
--- a/logging/loggers/channel_logger.go
+++ b/logging/loggers/channel_logger.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/eapache/channels"
 	kitlog "github.com/go-kit/kit/log"
+	"github.com/hyperledger/burrow/logging/errors"
 )
 
 const (
@@ -92,26 +93,35 @@ func readLogLine(logLine interface{}, ok bool) []interface{} {
 }
 
 // Enters an infinite loop that will drain any log lines from the passed logger.
+// You may pass in a channel
 //
 // Exits if the channel is closed.
-func (cl *ChannelLogger) DrainForever(logger kitlog.Logger) {
+func (cl *ChannelLogger) DrainForever(logger kitlog.Logger, errCh channels.Channel) {
 	// logLine could be nil if channel was closed while waiting for next line
 	for logLine := cl.WaitReadLogLine(); logLine != nil; logLine = cl.WaitReadLogLine() {
-		logger.Log(logLine...)
+		err := logger.Log(logLine...)
+		if err != nil && errCh != nil {
+			errCh.In() <- err
+		}
 	}
 }
 
 // Drains everything that is available at the time of calling
-func (cl *ChannelLogger) Flush(logger kitlog.Logger) {
+func (cl *ChannelLogger) Flush(logger kitlog.Logger) error {
 	// Grab the buffer at the here rather than within loop condition so that we
 	// do not drain the buffer forever
 	bufferLength := cl.BufferLength()
+	var errs []error
 	for i := 0; i < bufferLength; i++ {
 		logLine := cl.WaitReadLogLine()
 		if logLine != nil {
-			logger.Log(logLine...)
+			err := logger.Log(logLine...)
+			if err != nil {
+				errs = append(errs, err)
+			}
 		}
 	}
+	return errors.CombineErrors(errs)
 }
 
 // Drains the next contiguous segment of loglines up to the buffer cap waiting
@@ -137,9 +147,10 @@ func (cl *ChannelLogger) Reset() {
 }
 
 // Returns a Logger that wraps the outputLogger passed and does not block on
-// calls to Log.
-func NonBlockingLogger(outputLogger kitlog.Logger) *ChannelLogger {
+// calls to Log and a channel of any errors from the underlying logger
+func NonBlockingLogger(outputLogger kitlog.Logger) (*ChannelLogger, channels.Channel) {
 	cl := NewChannelLogger(DefaultLoggingRingBufferCap)
-	go cl.DrainForever(outputLogger)
-	return cl
+	errCh := channels.NewRingChannel(cl.BufferCap())
+	go cl.DrainForever(outputLogger, errCh)
+	return cl, errCh
 }
diff --git a/logging/loggers/channel_logger_test.go b/logging/loggers/channel_logger_test.go
index 0badbde1..ee8a105e 100644
--- a/logging/loggers/channel_logger_test.go
+++ b/logging/loggers/channel_logger_test.go
@@ -19,6 +19,8 @@ import (
 
 	"time"
 
+	"fmt"
+
 	"github.com/eapache/channels"
 	"github.com/stretchr/testify/assert"
 )
@@ -62,7 +64,7 @@ func TestChannelLogger_Reset(t *testing.T) {
 
 func TestNonBlockingLogger(t *testing.T) {
 	tl := newTestLogger()
-	nbl := NonBlockingLogger(tl)
+	nbl, _ := NonBlockingLogger(tl)
 	nbl.Log("Foo", "Bar")
 	nbl.Log("Baz", "Bur")
 	nbl.Log("Badger", "Romeo")
@@ -74,3 +76,11 @@ func TestNonBlockingLogger(t *testing.T) {
 		"Baz", "Bur", "",
 		"Badger", "Romeo"), lls)
 }
+
+func TestNonBlockingLoggerErrors(t *testing.T) {
+	el := newErrorLogger("Should surface")
+	nbl, errCh := NonBlockingLogger(el)
+	nbl.Log("failure", "true")
+	assert.Equal(t, "Should surface",
+		fmt.Sprintf("%s", <-errCh.Out()))
+}
diff --git a/logging/loggers/graylog_logger.go b/logging/loggers/graylog_logger.go
deleted file mode 100644
index 769cc92b..00000000
--- a/logging/loggers/graylog_logger.go
+++ /dev/null
@@ -1 +0,0 @@
-package loggers
diff --git a/logging/loggers/info_trace_logger.go b/logging/loggers/info_trace_logger.go
index 341e907d..2b9a5539 100644
--- a/logging/loggers/info_trace_logger.go
+++ b/logging/loggers/info_trace_logger.go
@@ -15,15 +15,17 @@
 package loggers
 
 import (
+	"github.com/eapache/channels"
 	kitlog "github.com/go-kit/kit/log"
 	"github.com/hyperledger/burrow/logging/structure"
 	"github.com/hyperledger/burrow/logging/types"
 )
 
 type infoTraceLogger struct {
-	infoContext  kitlog.Logger
-	traceContext kitlog.Logger
-	outputLogger *kitlog.SwapLogger
+	infoContext        kitlog.Logger
+	traceContext       kitlog.Logger
+	outputLogger       *kitlog.SwapLogger
+	outputLoggerErrors channels.Channel
 }
 
 // Interface assertions
@@ -31,28 +33,28 @@ var _ types.InfoTraceLogger = (*infoTraceLogger)(nil)
 var _ kitlog.Logger = (types.InfoTraceLogger)(nil)
 
 // Create an InfoTraceLogger by passing the initial outputLogger.
-func NewInfoTraceLogger(outputLogger kitlog.Logger) types.InfoTraceLogger {
+func NewInfoTraceLogger(outputLogger kitlog.Logger) (types.InfoTraceLogger, channels.Channel) {
 	// We will never halt the progress of a log emitter. If log output takes too
 	// long will start dropping log lines by using a ring buffer.
 	swapLogger := new(kitlog.SwapLogger)
 	swapLogger.Swap(outputLogger)
-	wrappedOutputLogger := wrapOutputLogger(swapLogger)
+	wrappedOutputLogger, errCh := wrapOutputLogger(swapLogger)
 	return &infoTraceLogger{
-		outputLogger: swapLogger,
+		outputLogger:       swapLogger,
+		outputLoggerErrors: errCh,
 		// logging contexts
 		infoContext: kitlog.With(wrappedOutputLogger,
 			structure.ChannelKey, types.InfoChannelName,
-			structure.LevelKey, types.InfoLevelName,
 		),
 		traceContext: kitlog.With(wrappedOutputLogger,
 			structure.ChannelKey, types.TraceChannelName,
-			structure.LevelKey, types.TraceLevelName,
 		),
-	}
+	}, errCh
 }
 
 func NewNoopInfoTraceLogger() types.InfoTraceLogger {
-	return NewInfoTraceLogger(nil)
+	logger, _ := NewInfoTraceLogger(nil)
+	return logger
 }
 
 func (l *infoTraceLogger) With(keyvals ...interface{}) types.InfoTraceLogger {
@@ -94,6 +96,6 @@ func (l *infoTraceLogger) Log(keyvals ...interface{}) error {
 
 // Wrap the output loggers with a a set of standard transforms, a non-blocking
 // ChannelLogger and an outer context
-func wrapOutputLogger(outputLogger kitlog.Logger) kitlog.Logger {
+func wrapOutputLogger(outputLogger kitlog.Logger) (kitlog.Logger, channels.Channel) {
 	return NonBlockingLogger(BurrowFormatLogger(VectorValuedLogger(outputLogger)))
 }
diff --git a/logging/loggers/info_trace_logger_test.go b/logging/loggers/info_trace_logger_test.go
index 7db1c0e1..2356748d 100644
--- a/logging/loggers/info_trace_logger_test.go
+++ b/logging/loggers/info_trace_logger_test.go
@@ -23,7 +23,7 @@ import (
 
 func TestLogger(t *testing.T) {
 	stderrLogger := kitlog.NewLogfmtLogger(os.Stderr)
-	logger := NewInfoTraceLogger(stderrLogger)
+	logger, _ := NewInfoTraceLogger(stderrLogger)
 	logger.Trace("hello", "barry")
 }
 
diff --git a/logging/loggers/multiple_output_logger.go b/logging/loggers/multiple_output_logger.go
index 14316024..601cba2f 100644
--- a/logging/loggers/multiple_output_logger.go
+++ b/logging/loggers/multiple_output_logger.go
@@ -15,9 +15,8 @@
 package loggers
 
 import (
-	"strings"
-
 	kitlog "github.com/go-kit/kit/log"
+	"github.com/hyperledger/burrow/logging/errors"
 )
 
 // This represents an 'AND' type logger. When logged to it will log to each of
@@ -34,7 +33,7 @@ func (mol MultipleOutputLogger) Log(keyvals ...interface{}) error {
 			errs = append(errs, err)
 		}
 	}
-	return combineErrors(errs)
+	return errors.CombineErrors(errs)
 }
 
 // Creates a logger that forks log messages to each of its outputLoggers
@@ -50,24 +49,3 @@ func NewMultipleOutputLogger(outputLoggers ...kitlog.Logger) kitlog.Logger {
 	}
 	return moLogger
 }
-
-type multipleErrors []error
-
-func combineErrors(errs []error) error {
-	switch len(errs) {
-	case 0:
-		return nil
-	case 1:
-		return errs[0]
-	default:
-		return multipleErrors(errs)
-	}
-}
-
-func (errs multipleErrors) Error() string {
-	var errStrings []string
-	for _, err := range errs {
-		errStrings = append(errStrings, err.Error())
-	}
-	return strings.Join(errStrings, ";")
-}
diff --git a/logging/loggers/multiple_output_logger_test.go b/logging/loggers/multiple_output_logger_test.go
index d310328b..ad41b3ca 100644
--- a/logging/loggers/multiple_output_logger_test.go
+++ b/logging/loggers/multiple_output_logger_test.go
@@ -17,11 +17,13 @@ package loggers
 import (
 	"testing"
 
+	"github.com/hyperledger/burrow/logging/errors"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestNewMultipleOutputLogger(t *testing.T) {
-	a, b := newErrorLogger("error a"), newErrorLogger("error b")
+	a := newErrorLogger("error a")
+	b := newErrorLogger("error b")
 	mol := NewMultipleOutputLogger(a, b)
 	logLine := []interface{}{"msg", "hello"}
 	errLog := mol.Log(logLine...)
@@ -32,5 +34,5 @@ func TestNewMultipleOutputLogger(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, expected, logLineA)
 	assert.Equal(t, expected, logLineB)
-	assert.IsType(t, multipleErrors{}, errLog)
+	assert.IsType(t, errors.MultipleErrors{}, errLog)
 }
diff --git a/logging/loggers/tendermint_log15_loggers.go b/logging/loggers/output_loggers.go
similarity index 74%
rename from logging/loggers/tendermint_log15_loggers.go
rename to logging/loggers/output_loggers.go
index 14b2ea09..28f46fae 100644
--- a/logging/loggers/tendermint_log15_loggers.go
+++ b/logging/loggers/output_loggers.go
@@ -13,12 +13,22 @@ import (
 
 const (
 	syslogPriority    = syslog.LOG_LOCAL0
-	defaultFormatName = "terminal"
+	JSONFormat        = "json"
+	LogfmtFormat      = "logfmt"
+	TerminalFormat    = "terminal"
+	defaultFormatName = TerminalFormat
 )
 
 func NewStreamLogger(writer io.Writer, formatName string) kitlog.Logger {
-	return log15a.Log15HandlerAsKitLogger(log15.StreamHandler(writer,
-		format(formatName)))
+	switch formatName {
+	case JSONFormat:
+		return kitlog.NewJSONLogger(writer)
+	case LogfmtFormat:
+		return kitlog.NewLogfmtLogger(writer)
+	default:
+		return log15a.Log15HandlerAsKitLogger(log15.StreamHandler(writer,
+			format(formatName)))
+	}
 }
 
 func NewFileLogger(path string, formatName string) (kitlog.Logger, error) {
@@ -45,11 +55,11 @@ func NewSyslogLogger(tag, formatName string) (kitlog.Logger, error) {
 
 func format(name string) log15.Format {
 	switch name {
-	case "json":
+	case JSONFormat:
 		return log15.JsonFormat()
-	case "logfmt":
+	case LogfmtFormat:
 		return log15.LogfmtFormat()
-	case "terminal":
+	case TerminalFormat:
 		return log15.TerminalFormat()
 	default:
 		return format(defaultFormatName)
diff --git a/logging/loggers/shared_test.go b/logging/loggers/shared_test.go
index 86783f49..94c2a15e 100644
--- a/logging/loggers/shared_test.go
+++ b/logging/loggers/shared_test.go
@@ -53,7 +53,7 @@ func makeTestLogger(err error) *testLogger {
 	go cl.DrainForever(kitlog.LoggerFunc(func(keyvals ...interface{}) error {
 		logLineCh <- keyvals
 		return nil
-	}))
+	}), nil)
 	return &testLogger{
 		channelLogger: cl,
 		logLineCh:     logLineCh,
diff --git a/manager/burrow-mint/state/permissions_test.go b/manager/burrow-mint/state/permissions_test.go
index a02f7406..4f550efd 100644
--- a/manager/burrow-mint/state/permissions_test.go
+++ b/manager/burrow-mint/state/permissions_test.go
@@ -110,7 +110,7 @@ x 		- roles: has, add, rm
 // keys
 var user = makeUsers(10)
 var chainID = "testchain"
-var logger = lifecycle.NewStdErrLogger()
+var logger, _ = lifecycle.NewStdErrLogger()
 
 func makeUsers(n int) []*acm.PrivAccount {
 	accounts := []*acm.PrivAccount{}
diff --git a/rpc/tendermint/test/shared.go b/rpc/tendermint/test/shared.go
index 3d6a7d96..4ca904dd 100644
--- a/rpc/tendermint/test/shared.go
+++ b/rpc/tendermint/test/shared.go
@@ -23,6 +23,8 @@ import (
 	"strconv"
 	"testing"
 
+	"time"
+
 	acm "github.com/hyperledger/burrow/account"
 	"github.com/hyperledger/burrow/config"
 	"github.com/hyperledger/burrow/core"
@@ -42,7 +44,6 @@ import (
 	"github.com/tendermint/go-crypto"
 	rpcclient "github.com/tendermint/go-rpc/client"
 	"github.com/tendermint/tendermint/types"
-	"time"
 )
 
 const chainID = "RPC_Test_Chain"
@@ -156,7 +157,7 @@ func initGlobalVariables(ffs *fixtures.FileFixtures) error {
 	// Set up priv_validator.json before we start tendermint (otherwise it will
 	// create its own one.
 	saveNewPriv()
-	logger := lifecycle.NewStdErrLogger()
+	logger, _ := lifecycle.NewStdErrLogger()
 	// To spill tendermint logs on the floor:
 	// lifecycle.CaptureTendermintLog15Output(loggers.NewNoopInfoTraceLogger())
 	lifecycle.CaptureTendermintLog15Output(logger)
diff --git a/rpc/v0/rest_server_test.go b/rpc/v0/rest_server_test.go
index fa4b27dc..2134e951 100644
--- a/rpc/v0/rest_server_test.go
+++ b/rpc/v0/rest_server_test.go
@@ -33,13 +33,12 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/hyperledger/burrow/logging/lifecycle"
-	logging_types "github.com/hyperledger/burrow/logging/types"
 	"github.com/hyperledger/burrow/rpc/v0/shared"
 	"github.com/stretchr/testify/suite"
 	"github.com/tendermint/log15"
 )
 
-var logger logging_types.InfoTraceLogger = lifecycle.NewStdErrLogger()
+var logger, _ = lifecycle.NewStdErrLogger()
 
 func init() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
diff --git a/test/server/scumbag_test.go b/test/server/scumbag_test.go
index 7fcf23c2..92eb0c03 100644
--- a/test/server/scumbag_test.go
+++ b/test/server/scumbag_test.go
@@ -24,11 +24,10 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/hyperledger/burrow/logging/lifecycle"
-	logging_types "github.com/hyperledger/burrow/logging/types"
 	"github.com/tendermint/log15"
 )
 
-var logger logging_types.InfoTraceLogger = lifecycle.NewStdErrLogger()
+var logger, _ = lifecycle.NewStdErrLogger()
 
 func init() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
-- 
GitLab