feat(http): pretty stack trace for the http panic recovery handler (i…

…nfluxdata#13509)

When printing out a stacktrace with a logged error message, it is better
to set the `Stack` property on the entry than to include it as a string
within the context fields. It is also better than using `zap.Stack()`
too.

This is because certain encoders will perform a pretty print of the
stacktrace. The default zap-logfmt will read this property and include
it as a field so this code is identical when using that encoder, but the
console encoder, which is commonly used in local development because it
is automatically used with a TTY available, will print out a newline and
pretty print the stacktrace if it is included on the entry itself.

http/router.go

@@ -11,6 +11,7 @@ import (
 	influxlogger "github.com/influxdata/influxdb/logger"
 	"github.com/julienschmidt/httprouter"
 	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 )
 
 // NewRouter returns a new router with a 404 handler, a 405 handler, and a panic handler.
@@ -56,11 +57,10 @@ func panicHandler(w http.ResponseWriter, r *http.Request, rcv interface{}) {
 	}
 
 	l := getPanicLogger()
-	l.Error(
-		pe.Msg,
-		zap.String("err", pe.Err.Error()),
-		zap.String("stack", fmt.Sprintf("%s", debug.Stack())),
-	)
+	if entry := l.Check(zapcore.ErrorLevel, pe.Msg); entry != nil {
+		entry.Stack = string(debug.Stack())
+		entry.Write(zap.Error(pe.Err))
+	}
 
 	EncodeError(ctx, pe, w)
 }

logger/style_guide.md

@@ -190,3 +190,36 @@ then `61m` should be used.
 
 For anything lower than milliseconds that is static, the duration should
 be truncated. A value of zero should be shown as `0s`.
+
+## Stacktraces
+
+Logging stacktraces is special within the zap library. There are two
+different ways to do it, but one of them will lead to more user-friendly
+output and is the preferred way to log stacktraces.
+
+A stacktrace should only be computed if it is known that the log message
+will be logged and the stacktrace should be attached to the log entry
+using the special struct field rather than within the context.
+
+Below is a code sample using the zap logger.
+
+```go
+var logger *zap.Logger
+
+// ...
+
+if entry := logger.Check(zapcore.InfoLevel, "A panic happened"); entry != nil {
+    entry.Stack = string(debug.Stack())
+    entry.Write(/* additional context here */)
+}
+```
+
+The reason for this is because certain encoders will handle the `Stack`
+field in a special way. The console encoder, the user-friendly one used
+when a TTY is present, will print out a newline and then pretty-print
+the stack separately from the context. The logfmt encoder will encode
+the stack as a normal context key so that it can follow the logfmt encoding.
+
+If the `zap.Stack(string)` method is used and included as part of the context,
+then the stack will always be included within the context instead of handled
+in the special way dictated by the encoder.