MM-23261 plugin stderr debug logs (#14166)

* explicitly assert panic as error log

* Revert "[MM-18150] plugin panic trace should not be lost (#13559)"

This reverts commit 5d928b4f94, while leaving the unit tests intact
and now asserting debug logs instead.

* missing license header

app/plugin_test.go

@@ -13,7 +13,6 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
@@ -21,8 +20,10 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/plugin"
+	"github.com/mattermost/mattermost-server/v5/testlib"
 	"github.com/mattermost/mattermost-server/v5/utils"
 	"github.com/mattermost/mattermost-server/v5/utils/fileutils"
 )
@@ -665,8 +666,7 @@ func TestPluginPanicLogs(t *testing.T) {
 		_, err := th.App.CreatePost(post, th.BasicChannel, false)
 		assert.Nil(t, err)
 
-		logs := th.LogBuffer.String()
+		testlib.AssertLog(t, th.LogBuffer, mlog.LevelDebug, "panic: some text from panic")
-		assert.True(t, strings.Contains(logs, "some text from panic"))
 	})
 }
 

mlog/log.go

@@ -144,15 +144,6 @@ func (l *Logger) StdLogWriter() io.Writer {
 	return &loggerWriter{f}
 }
 
-// StdErrPanicLogWriter returns a writer that can be hooked up to the output of a golang standard logger
-// all of the stderr will be interpreted as log entry.
-func (l *Logger) StdErrPanicLogWriter() io.Writer {
-	newLogger := *l
-	newLogger.zap = newLogger.zap.WithOptions(zap.AddCallerSkip(4), getStdLogOption())
-	f := newLogger.Error
-	return &panicLoggerWriter{f}
-}
-
 func (l *Logger) WithCallerSkip(skip int) *Logger {
 	newlogger := *l
 	newlogger.zap = newlogger.zap.WithOptions(zap.AddCallerSkip(skip))

mlog/stdlog.go

@@ -85,13 +85,3 @@ func (l *loggerWriter) Write(p []byte) (int, error) {
 	}
 	return len(p), nil
 }
-
-type panicLoggerWriter struct {
-	logFunc func(msg string, fields ...Field)
-}
-
-func (l *panicLoggerWriter) Write(p []byte) (int, error) {
-	trimmed := string(bytes.TrimSpace(p))
-	l.logFunc(trimmed)
-	return len(p), nil
-}

plugin/supervisor.go

@@ -63,7 +63,6 @@ func newSupervisor(pluginInfo *model.BundleInfo, apiImpl API, parentLogger *mlog
 		HandshakeConfig: handshake,
 		Plugins:         pluginMap,
 		Cmd:             cmd,
-		Stderr:          wrappedLogger.With(mlog.String("source", "plugin_stderr_panic")).StdErrPanicLogWriter(),
 		SyncStdout:      wrappedLogger.With(mlog.String("source", "plugin_stdout")).StdLogWriter(),
 		SyncStderr:      wrappedLogger.With(mlog.String("source", "plugin_stderr")).StdLogWriter(),
 		Logger:          hclogAdaptedLogger,

testlib/assertions.go

@@ -0,0 +1,33 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package testlib
+
+import (
+	"bytes"
+	"encoding/json"
+	"io"
+	"testing"
+)
+
+// AssertLog asserts that a JSON-encoded buffer of logs contains one with the given level and message.
+func AssertLog(t *testing.T, logs *bytes.Buffer, level, message string) {
+	dec := json.NewDecoder(logs)
+	for {
+		var log struct {
+			Level string
+			Msg   string
+		}
+		if err := dec.Decode(&log); err == io.EOF {
+			break
+		} else if err != nil {
+			continue
+		}
+
+		if log.Level == level && log.Msg == message {
+			return
+		}
+	}
+
+	t.Fatalf("failed to find %s log message: %s", level, message)
+}