Merge pull request #122 from skriss/logging-fixes

logging improvements

docs/cli-reference/ark_server.md

@@ -14,7 +14,8 @@ ark server [flags]
 ### Options
 
 ```
-  -h, --help   help for server
+  -h, --help        help for server
+      --log-level   the level at which to log. Valid values are debug, info, warning, error, fatal, panic. (default info)
 ```
 
 ### Options inherited from parent commands

examples/azure/00-ark-deployment.yaml

@@ -34,9 +34,6 @@ spec:
             - /ark
           args:
             - server
-            - --logtostderr
-            - --v
-            - "4"
           envFrom:
             - secretRef:
                 name: cloud-credentials

examples/common/10-deployment.yaml

@@ -34,9 +34,6 @@ spec:
             - /ark
           args:
             - server
-            - --logtostderr
-            - --v
-            - "4"
           volumeMounts:
             - name: cloud-credentials
               mountPath: /credentials

pkg/cmd/server/server.go

@@ -22,6 +22,8 @@ import (
 	"io/ioutil"
 	"os"
 	"reflect"
+	"sort"
+	"strings"
 	"sync"
 	"time"
 
@@ -48,6 +50,7 @@ import (
 	"github.com/heptio/ark/pkg/cloudprovider/azure"
 	"github.com/heptio/ark/pkg/cloudprovider/gcp"
 	"github.com/heptio/ark/pkg/cmd"
+	"github.com/heptio/ark/pkg/cmd/util/flag"
 	"github.com/heptio/ark/pkg/controller"
 	arkdiscovery "github.com/heptio/ark/pkg/discovery"
 	"github.com/heptio/ark/pkg/generated/clientset"
@@ -60,15 +63,30 @@ import (
 )
 
 func NewCommand() *cobra.Command {
-	var kubeconfig string
+	var (
+		kubeconfig      string
+		sortedLogLevels = getSortedLogLevels()
+		logLevelFlag    = flag.NewEnum(logrus.InfoLevel.String(), sortedLogLevels...)
+	)
 
 	var command = &cobra.Command{
 		Use:   "server",
 		Short: "Run the ark server",
 		Long:  "Run the ark server",
 		Run: func(c *cobra.Command, args []string) {
-			logger := logrus.New()
-			logger.Hooks.Add(&logging.ErrorLocationHook{})
+			logLevel := logrus.InfoLevel
+
+			if parsed, err := logrus.ParseLevel(logLevelFlag.String()); err == nil {
+				logLevel = parsed
+			} else {
+				// This should theoretically never happen assuming the enum flag
+				// is constructed correctly because the enum flag will not allow
+				//  an invalid value to be set.
+				logrus.Errorf("log-level flag has invalid value %s", strings.ToUpper(logLevelFlag.String()))
+			}
+			logrus.Infof("setting log-level to %s", strings.ToUpper(logLevel.String()))
+
+			logger := newLogger(logLevel, &logging.ErrorLocationHook{}, &logging.LogLocationHook{})
 
 			s, err := newServer(kubeconfig, fmt.Sprintf("%s-%s", c.Parent().Name(), c.Name()), logger)
 
@@ -78,11 +96,43 @@ func NewCommand() *cobra.Command {
 		},
 	}
 
+	command.Flags().Var(logLevelFlag, "log-level", fmt.Sprintf("the level at which to log. Valid values are %s.", strings.Join(sortedLogLevels, ", ")))
 	command.Flags().StringVar(&kubeconfig, "kubeconfig", "", "Path to the kubeconfig file to use to talk to the Kubernetes apiserver. If unset, try the environment variable KUBECONFIG, as well as in-cluster configuration")
 
 	return command
 }
 
+func newLogger(level logrus.Level, hooks ...logrus.Hook) *logrus.Logger {
+	logger := logrus.New()
+	logger.Level = level
+
+	for _, hook := range hooks {
+		logger.Hooks.Add(hook)
+	}
+
+	return logger
+}
+
+// getSortedLogLevels returns a string slice containing all of the valid logrus
+// log levels (based on logrus.AllLevels), sorted in ascending order of severity.
+func getSortedLogLevels() []string {
+	var (
+		sortedLogLevels  = make([]logrus.Level, len(logrus.AllLevels))
+		logLevelsStrings []string
+	)
+
+	copy(sortedLogLevels, logrus.AllLevels)
+
+	// logrus.Panic has the lowest value, so the compare function uses ">"
+	sort.Slice(sortedLogLevels, func(i, j int) bool { return sortedLogLevels[i] > sortedLogLevels[j] })
+
+	for _, level := range sortedLogLevels {
+		logLevelsStrings = append(logLevelsStrings, level.String())
+	}
+
+	return logLevelsStrings
+}
+
 type server struct {
 	kubeClient            kubernetes.Interface
 	arkClient             clientset.Interface

pkg/cmd/util/flag/enum.go

@@ -31,12 +31,12 @@ type Enum struct {
 }
 
 // NewEnum returns a new enum flag with the specified list
-// of allowed values. The first value specified is used
-// as the default.
-func NewEnum(allowedValues ...string) Enum {
-	return Enum{
+// of allowed values, and the specified default value if
+// none is set.
+func NewEnum(defaultValue string, allowedValues ...string) *Enum {
+	return &Enum{
 		allowedValues: sets.NewString(allowedValues...),
-		value:         allowedValues[0],
+		value:         defaultValue,
 	}
 }
 
@@ -61,5 +61,8 @@ func (e *Enum) Set(s string) error {
 // Type returns a string representation of the
 // Enum type.
 func (e *Enum) Type() string {
-	return "enum"
+	// we don't want the help text to display anything regarding
+	// the type because the usage text for the flag should capture
+	// the possible options.
+	return ""
 }

pkg/util/logging/error_location_hook.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2017 Heptio Inc.
+
+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 logging
 
 import (
@@ -14,7 +30,7 @@ const (
 
 // ErrorLocationHook is a logrus hook that attaches error location information
 // to log entries if an error is being logged and it has stack-trace information
-// (i.e. if it originates from or is wrapped by github.com/pkg/errors)
+// (i.e. if it originates from or is wrapped by github.com/pkg/errors).
 type ErrorLocationHook struct {
 }
 

pkg/util/logging/error_location_hook_test.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2017 Heptio Inc.
+
+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 logging
 
 import (

pkg/util/logging/log_location_hook.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2017 Heptio Inc.
+
+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 logging
+
+import (
+	"fmt"
+	"runtime"
+	"strings"
+
+	"github.com/sirupsen/logrus"
+)
+
+const logLocationField = "logSource"
+
+// LogLocationHook is a logrus hook that attaches location information
+// to log entries, i.e. the file and line number of the logrus log call.
+type LogLocationHook struct {
+}
+
+func (h *LogLocationHook) Levels() []logrus.Level {
+	return logrus.AllLevels
+}
+
+func (h *LogLocationHook) Fire(entry *logrus.Entry) error {
+	pcs := make([]uintptr, 64)
+
+	// skip 2 frames:
+	//   runtime.Callers
+	//   github.com/heptio/ark/pkg/util/logging/(*LogLocationHook).Fire
+	n := runtime.Callers(2, pcs)
+
+	// re-slice pcs based on the number of entries written
+	frames := runtime.CallersFrames(pcs[:n])
+
+	// now traverse up the call stack looking for the first non-logrus
+	// func, which will be the logrus invoker
+	var (
+		frame runtime.Frame
+		more  = true
+	)
+
+	for more {
+		frame, more = frames.Next()
+
+		if strings.Contains(frame.File, "github.com/sirupsen/logrus") {
+			continue
+		}
+
+		entry.Data[logLocationField] = fmt.Sprintf("%s:%d", frame.File, frame.Line)
+		break
+	}
+
+	return nil
+}