Merge pull request #1301 from skriss/plugins-error-location

log error locations from plugin logger and don't overwrite in client

changelogs/unreleased/1301-skriss

@@ -0,0 +1 @@
+log error locations from plugin logger, and don't overwrite them in the client logger if they exist already

pkg/plugin/framework/logger.go

@@ -50,6 +50,9 @@ func newLogger() *logrus.Logger {
 	// server logger that the location has been set within a hook.
 	logger.Hooks.Add((&logging.LogLocationHook{}).WithLoggerName("plugin"))
 
+	// make sure we attempt to record the error location
+	logger.Hooks.Add(&logging.ErrorLocationHook{})
+
 	// this hook adjusts the string representation of WarnLevel to "warn"
 	// rather than "warning" to make it parseable by go-plugin within the
 	// Velero server code

pkg/plugin/framework/logger_test.go

@@ -37,6 +37,7 @@ func TestNewLogger(t *testing.T) {
 
 	expectedHooks := []logrus.Hook{
 		(&logging.LogLocationHook{}).WithLoggerName("plugin"),
+		&logging.ErrorLocationHook{},
 		&logging.HcLogLevelHook{},
 	}
 

pkg/util/logging/error_location_hook.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 the Velero contributors.
+Copyright 2017, 2019 the Velero contributors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -47,6 +47,17 @@ func (h *ErrorLocationHook) Fire(entry *logrus.Entry) error {
 		return nil
 	}
 
+	if _, ok := entry.Data[errorFileField]; ok {
+		// If there is already an error file field, preserve it instead of overwriting it. This field will already exist if
+		// the log message occurred in the server half of a plugin.
+		return nil
+	}
+	if _, ok := entry.Data[errorFunctionField]; ok {
+		// If there is already an error function field, preserve it instead of overwriting it. This field will already exist if
+		// the log message occurred in the server half of a plugin.
+		return nil
+	}
+
 	err, ok := errObj.(error)
 	if !ok {
 		return errors.New("object logged as error does not satisfy error interface")

pkg/util/logging/error_location_hook_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 the Velero contributors.
+Copyright 2017, 2019 the Velero contributors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -58,6 +58,19 @@ func TestFire(t *testing.T) {
 				errorFunctionField: "github.com/heptio/velero/pkg/util/logging.TestFire",
 			},
 		},
+		{
+			name: "already have error file and function fields",
+			preEntryFields: map[string]interface{}{
+				logrus.ErrorKey:    pkgerrs.New("a pkg/errors error"),
+				errorFileField:     "some_file.go:123",
+				errorFunctionField: "SomeFunction",
+			},
+			expectedEntryFields: map[string]interface{}{
+				logrus.ErrorKey:    pkgerrs.New("a pkg/errors error"),
+				errorFileField:     "some_file.go:123",
+				errorFunctionField: "SomeFunction",
+			},
+		},
 	}
 
 	for _, test := range tests {