From b60901e766cc6a8a8720f1f07117a0c13f3fd47e Mon Sep 17 00:00:00 2001
From: Chris Goller <goller@gmail.com>
Date: Mon, 6 Mar 2017 15:16:45 -0600
Subject: [PATCH] Add structured logging to underlying http server

---
 chronograf.go          |  8 +++++---
 kapacitor/validate.go  |  2 --
 log/log.go             |  5 +++++
 server/server.go       | 24 +++++++++++++++++++-----
 server/url_prefixer.go |  6 +++---
 5 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/chronograf.go b/chronograf.go
index 1a8b7730d..fc624bea4 100644
--- a/chronograf.go
+++ b/chronograf.go
@@ -2,6 +2,7 @@ package chronograf
 
 import (
 	"context"
+	"io"
 	"net/http"
 )
 
@@ -32,12 +33,13 @@ func (e Error) Error() string {
 type Logger interface {
 	Debug(...interface{})
 	Info(...interface{})
-	Warn(...interface{})
 	Error(...interface{})
-	Fatal(...interface{})
-	Panic(...interface{})
 
 	WithField(string, interface{}) Logger
+
+	// Logger can be transformed into an io.Writer.
+	// That writer is the end of an io.Pipe and it is your responsibility to close it.
+	Writer() *io.PipeWriter
 }
 
 // Assets returns a handler to serve the website.
diff --git a/kapacitor/validate.go b/kapacitor/validate.go
index 4a81a9d0e..b7984fc16 100644
--- a/kapacitor/validate.go
+++ b/kapacitor/validate.go
@@ -3,7 +3,6 @@ package kapacitor
 import (
 	"bytes"
 	"fmt"
-	"log"
 	"time"
 
 	"github.com/influxdata/chronograf"
@@ -25,7 +24,6 @@ func ValidateAlert(service string) error {
 func formatTick(tickscript string) (chronograf.TICKScript, error) {
 	node, err := ast.Parse(tickscript)
 	if err != nil {
-		log.Fatalf("parse execution: %s", err)
 		return "", err
 	}
 
diff --git a/log/log.go b/log/log.go
index 2eb6b3ae9..1b00e3332 100644
--- a/log/log.go
+++ b/log/log.go
@@ -1,6 +1,7 @@
 package log
 
 import (
+	"io"
 	"os"
 
 	"github.com/Sirupsen/logrus"
@@ -81,6 +82,10 @@ func (ll *logrusLogger) WithField(key string, value interface{}) chronograf.Logg
 	return &logrusLogger{ll.l.WithField(key, value)}
 }
 
+func (ll *logrusLogger) Writer() *io.PipeWriter {
+	return ll.l.Logger.WriterLevel(logrus.ErrorLevel)
+}
+
 // New wraps a logrus Logger
 func New(l Level) chronograf.Logger {
 	logger := &logrus.Logger{
diff --git a/server/server.go b/server/server.go
index 738f0d906..3a494b9f8 100644
--- a/server/server.go
+++ b/server/server.go
@@ -2,9 +2,11 @@ package server
 
 import (
 	"crypto/tls"
+	"log"
 	"math/rand"
 	"net"
 	"net/http"
+	"os"
 	"runtime"
 	"strconv"
 	"time"
@@ -57,7 +59,7 @@ type Server struct {
 	HerokuOrganizations []string `long:"heroku-organization" description:"Heroku Organization Memberships a user is required to have for access to Chronograf (comma separated)" env:"HEROKU_ORGS" env-delim:","`
 
 	ReportingDisabled bool   `short:"r" long:"reporting-disabled" description:"Disable reporting of usage stats (os,arch,version,cluster_id,uptime) once every 24hr" env:"REPORTING_DISABLED"`
-	LogLevel          string `short:"l" long:"log-level" value-name:"choice" choice:"debug" choice:"info" choice:"warn" choice:"error" choice:"fatal" choice:"panic" default:"info" description:"Set the logging level" env:"LOG_LEVEL"`
+	LogLevel          string `short:"l" long:"log-level" value-name:"choice" choice:"debug" choice:"info" choice:"error" default:"info" description:"Set the logging level" env:"LOG_LEVEL"`
 	Basepath          string `short:"p" long:"basepath" description:"A URL path prefix under which all chronograf routes will be mounted" env:"BASE_PATH"`
 	ShowVersion       bool   `short:"v" long:"version" description:"Show Chronograf version info"`
 	BuildInfo         BuildInfo
@@ -211,10 +213,21 @@ func (s *Server) Serve() error {
 	}
 	s.Listener = listener
 
-	httpServer := &graceful.Server{Server: new(http.Server)}
+	// Using a log writer for http server logging
+	w := logger.Writer()
+	defer w.Close()
+	stdLog := log.New(w, "", 0)
+
+	// TODO: Remove graceful when changing to go 1.8
+	httpServer := &graceful.Server{
+		Server: &http.Server{
+			ErrorLog: stdLog,
+			Handler:  s.handler,
+		},
+		Logger:       stdLog,
+		TCPKeepAlive: 5 * time.Second,
+	}
 	httpServer.SetKeepAlivesEnabled(true)
-	httpServer.TCPKeepAlive = 5 * time.Second
-	httpServer.Handler = s.handler
 
 	if !s.ReportingDisabled {
 		go reportUsageStats(s.BuildInfo, logger)
@@ -247,7 +260,8 @@ func openService(boltPath, cannedPath string, logger chronograf.Logger, useAuth
 	if err := db.Open(); err != nil {
 		logger.
 			WithField("component", "boltstore").
-			Fatal("Unable to open boltdb; is there a chronograf already running?  ", err)
+			Error("Unable to open boltdb; is there a chronograf already running?  ", err)
+		os.Exit(1)
 	}
 
 	// These apps are those handled from a directory
diff --git a/server/url_prefixer.go b/server/url_prefixer.go
index 6cbed57cb..10387c08e 100644
--- a/server/url_prefixer.go
+++ b/server/url_prefixer.go
@@ -80,9 +80,9 @@ func (up *URLPrefixer) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	// extract the flusher for flushing chunks
 	flusher, ok := rw.(http.Flusher)
 	if !ok {
-		up.Logger.
-			WithField("component", "prefixer").
-			Fatal("Expected http.ResponseWriter to be an http.Flusher, but wasn't")
+		msg := "Expected http.ResponseWriter to be an http.Flusher, but wasn't"
+		Error(rw, http.StatusInternalServerError, msg, up.Logger)
+		return
 	}
 
 	nextRead, nextWrite := io.Pipe()