use zap for logging (#1112)

Use zap for logging

executor/api.go

@@ -21,12 +21,12 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
-	"log"
 	"net/http"
 	"strings"
 
 	"github.com/gorilla/mux"
 	"go.opencensus.io/plugin/ochttp"
+	"go.uber.org/zap"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/fission/fission"
@@ -50,7 +50,10 @@ func (executor *Executor) getServiceForFunctionApi(w http.ResponseWriter, r *htt
 	serviceName, err := executor.getServiceForFunction(r.Context(), &m)
 	if err != nil {
 		code, msg := fission.GetHTTPError(err)
-		log.Printf("Error: %v: %v", code, msg)
+		executor.logger.Error("error getting service for function",
+			zap.Error(err),
+			zap.String("function", m.Name),
+			zap.String("fission_http_error", msg))
 		http.Error(w, msg, code)
 		return
 	}
@@ -69,14 +72,19 @@ func (executor *Executor) getServiceForFunctionApi(w http.ResponseWriter, r *htt
 // invalidates the cache entry if the pod address was cached.
 func (executor *Executor) getServiceForFunction(ctx context.Context, m *metav1.ObjectMeta) (string, error) {
 	// Check function -> svc cache
-	log.Printf("[%v] Checking for cached function service", m.Name)
+	executor.logger.Info("checking for cached function service",
+		zap.String("function_name", m.Name),
+		zap.String("function_namespace", m.Namespace))
 	fsvc, err := executor.fsCache.GetByFunction(m)
 	if err == nil {
 		if executor.isValidAddress(fsvc) {
 			// Cached, return svc address
 			return fsvc.Address, nil
 		} else {
-			log.Printf("[%v] Deleting cache entry for invalid address : %s", m.Name, fsvc.Address)
+			executor.logger.Info("deleting cache entry for invalid address",
+				zap.String("function_name", m.Name),
+				zap.String("function_namespace", m.Namespace),
+				zap.String("address", fsvc.Address))
 			executor.fsCache.DeleteEntry(fsvc)
 		}
 	}
@@ -98,6 +106,7 @@ func (executor *Executor) getServiceForFunction(ctx context.Context, m *metav1.O
 func (executor *Executor) tapService(w http.ResponseWriter, r *http.Request) {
 	body, err := ioutil.ReadAll(r.Body)
 	if err != nil {
+		executor.logger.Error("failed to read tap service request", zap.Error(err))
 		http.Error(w, "Failed to read request", http.StatusInternalServerError)
 		return
 	}
@@ -106,7 +115,10 @@ func (executor *Executor) tapService(w http.ResponseWriter, r *http.Request) {
 
 	err = executor.fsCache.TouchByAddress(svcHost)
 	if err != nil {
-		log.Printf("funcSvc tap error: %v", err)
+		executor.logger.Error("error tapping function service",
+			zap.Error(err),
+			zap.String("service", svcName),
+			zap.String("host", svcHost))
 		http.Error(w, "Not found", http.StatusNotFound)
 		return
 	}
@@ -123,15 +135,15 @@ func (executor *Executor) Serve(port int) {
 	r.HandleFunc("/v2/tapService", executor.tapService).Methods("POST")
 	r.HandleFunc("/healthz", executor.healthHandler).Methods("GET")
 	address := fmt.Sprintf(":%v", port)
-	log.Printf("starting executor at port %v", port)
+	executor.logger.Info("starting executor", zap.Int("port", port))
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	executor.ndm.Run(ctx)
 	executor.gpm.Run(ctx)
-	r.Use(fission.LoggingMiddleware)
+	r.Use(fission.LoggingMiddleware(executor.logger))
 	err := http.ListenAndServe(address, &ochttp.Handler{
 		Handler: r,
 		// Propagation: &b3.HTTPFormat{},
 	})
-	log.Fatal(err)
+	executor.logger.Fatal("done listening", zap.Error(err))
 }

executor/client/client.go

@@ -21,13 +21,14 @@ import (
 	"context"
 	"encoding/json"
 	"io/ioutil"
-	"log"
 	"net/http"
 	"net/url"
 	"strings"
 	"time"
 
+	"github.com/pkg/errors"
 	"go.opencensus.io/plugin/ochttp"
+	"go.uber.org/zap"
 	"golang.org/x/net/context/ctxhttp"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -35,14 +36,16 @@ import (
 )
 
 type Client struct {
+	logger      *zap.Logger
 	executorUrl string
 	tappedByUrl map[string]bool
 	requestChan chan string
 	httpClient  *http.Client
 }
 
-func MakeClient(executorUrl string) *Client {
+func MakeClient(logger *zap.Logger, executorUrl string) *Client {
 	c := &Client{
+		logger:      logger.Named("executor_client"),
 		executorUrl: strings.TrimSuffix(executorUrl, "/"),
 		tappedByUrl: make(map[string]bool),
 		requestChan: make(chan string),
@@ -59,12 +62,12 @@ func (c *Client) GetServiceForFunction(ctx context.Context, metadata *metav1.Obj
 
 	body, err := json.Marshal(metadata)
 	if err != nil {
-		return "", err
+		return "", errors.Wrap(err, "could not marshal request body for getting service for function")
 	}
 
 	resp, err := ctxhttp.Post(ctx, c.httpClient, executorUrl, "application/json", bytes.NewReader(body))
 	if err != nil {
-		return "", err
+		return "", errors.Wrap(err, "error posting to getting service for function")
 	}
 	defer resp.Body.Close()
 
@@ -74,8 +77,7 @@ func (c *Client) GetServiceForFunction(ctx context.Context, metadata *metav1.Obj
 
 	svcName, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		log.Printf("Returning from ioutil read body")
-		return "", err
+		return "", errors.Wrap(err, "error reading response body from getting service for function")
 	}
 
 	return string(svcName), nil
@@ -95,10 +97,10 @@ func (c *Client) service() {
 					for u := range urls {
 						err := c._tapService(u)
 						if err != nil {
-							log.Printf("Error tapping function service address %v: %v", u, err)
+							c.logger.Error("error tapping function service address", zap.Error(err), zap.String("address", u))
 						}
 					}
-					log.Printf("Tapped %v services in batch", len(urls))
+					c.logger.Info("tapped services in batch", zap.Int("service_count", len(urls)))
 				}()
 			}
 		}

executor/executor.go

@@ -19,7 +19,6 @@ package executor
 import (
 	"context"
 	"fmt"
-	"log"
 	"net/http"
 	"runtime/debug"
 	"strings"
@@ -29,6 +28,7 @@ import (
 	"github.com/dchest/uniuri"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
+	"go.uber.org/zap"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/fission/fission"
@@ -41,6 +41,8 @@ import (
 
 type (
 	Executor struct {
+		logger *zap.Logger
+
 		gpm *poolmgr.GenericPoolManager
 		ndm *newdeploy.NewDeploy
 
@@ -62,8 +64,9 @@ type (
 	}
 )
 
-func MakeExecutor(gpm *poolmgr.GenericPoolManager, ndm *newdeploy.NewDeploy, fissionClient *crd.FissionClient, fsCache *fscache.FunctionServiceCache) *Executor {
+func MakeExecutor(logger *zap.Logger, gpm *poolmgr.GenericPoolManager, ndm *newdeploy.NewDeploy, fissionClient *crd.FissionClient, fsCache *fscache.FunctionServiceCache) *Executor {
 	executor := &Executor{
+		logger:        logger.Named("executor"),
 		gpm:           gpm,
 		ndm:           ndm,
 		fissionClient: fissionClient,
@@ -111,7 +114,8 @@ func (executor *Executor) serveCreateFuncServices() {
 		} else {
 			// There's an existing request for this function, wait for it to finish
 			go func() {
-				log.Printf("Waiting for concurrent request for the same function: %v", m)
+				executor.logger.Info("waiting for concurrent request for the same function",
+					zap.Any("function", m))
 				wg.Wait()
 
 				// get the function service from the cache
@@ -121,7 +125,9 @@ func (executor *Executor) serveCreateFuncServices() {
 				// It normally happened if there are multiple requests are
 				// waiting for the same function and executor failed to cre-
 				// ate service for function.
-				err = errors.Wrap(err, fmt.Sprintf("Error getting service for function %v in namespace %v", m.Name, m.Namespace))
+				err = errors.Wrapf(err, "error getting service for function",
+					zap.String("function_name", m.Name),
+					zap.String("function_namespace", m.Namespace))
 				req.respChan <- &createFuncServiceResponse{
 					funcSvc: fsvc,
 					err:     err,
@@ -140,7 +146,9 @@ func (executor *Executor) getFunctionExecutorType(meta *metav1.ObjectMeta) (fiss
 }
 
 func (executor *Executor) createServiceForFunction(ctx context.Context, meta *metav1.ObjectMeta) (*fscache.FuncSvc, error) {
-	log.Printf("[%v] No cached function service found, creating one", meta.Name)
+	executor.logger.Info("no cached function service found, creating one",
+		zap.String("function_name", meta.Name),
+		zap.String("function_namespace", meta.Namespace))
 
 	executorType, err := executor.getFunctionExecutorType(meta)
 	if err != nil {
@@ -158,8 +166,12 @@ func (executor *Executor) createServiceForFunction(ctx context.Context, meta *me
 	}
 
 	if fsvcErr != nil {
-		fsvcErr = errors.Wrap(fsvcErr, fmt.Sprintf("[%v] Error creating service for function", meta.Name))
-		log.Print(fsvcErr)
+		e := "error creating service for function"
+		executor.logger.Error(e,
+			zap.Error(fsvcErr),
+			zap.String("function_name", meta.Name),
+			zap.String("function_namespace", meta.Namespace))
+		fsvcErr = errors.Wrap(fsvcErr, fmt.Sprintf("[%s] %s", meta.Name, e))
 	} else if fsvc != nil {
 		_, err = executor.fsCache.Add(*fsvc)
 		if err != nil {
@@ -185,16 +197,18 @@ func dumpStackTrace() {
 	debug.PrintStack()
 }
 
-func serveMetric() {
+func serveMetric(logger *zap.Logger) {
 	// Expose the registered metrics via HTTP.
 	metricAddr := ":8080"
 	http.Handle("/metrics", promhttp.Handler())
-	log.Fatal(http.ListenAndServe(metricAddr, nil))
+	err := http.ListenAndServe(metricAddr, nil)
+
+	logger.Fatal("done listening on metrics endpoint", zap.Error(err))
 }
 
 // StartExecutor Starts executor and the executor components such as Poolmgr,
 // deploymgr and potential future executor types
-func StartExecutor(fissionNamespace string, functionNamespace string, envBuilderNamespace string, port int) error {
+func StartExecutor(logger *zap.Logger, fissionNamespace string, functionNamespace string, envBuilderNamespace string, port int) error {
 	// setup a signal handler for SIGTERM
 	fission.SetupStackTraceHandler()
 
@@ -202,33 +216,34 @@ func StartExecutor(fissionNamespace string, functionNamespace string, envBuilder
 
 	err = fissionClient.WaitForCRDs()
 	if err != nil {
-		log.Fatalf("Error waiting for CRDs: %v", err)
+		return errors.Wrap(err, "error waiting for CRDs")
 	}
 
 	restClient := fissionClient.GetCrdClient()
 	if err != nil {
-		log.Printf("Failed to get kubernetes client: %v", err)
-		return err
+		return errors.Wrap(err, "failed to get kubernetes client")
 	}
 
-	fsCache := fscache.MakeFunctionServiceCache()
+	fsCache := fscache.MakeFunctionServiceCache(logger)
 
 	poolID := strings.ToLower(uniuri.NewLen(8))
-	reaper.CleanupOldExecutorObjects(kubernetesClient, poolID)
-	go reaper.CleanupRoleBindings(kubernetesClient, fissionClient, functionNamespace, envBuilderNamespace, time.Minute*30)
+	reaper.CleanupOldExecutorObjects(logger, kubernetesClient, poolID)
+	go reaper.CleanupRoleBindings(logger, kubernetesClient, fissionClient, functionNamespace, envBuilderNamespace, time.Minute*30)
 
 	gpm := poolmgr.MakeGenericPoolManager(
+		logger,
 		fissionClient, kubernetesClient,
 		functionNamespace, poolID)
 
 	ndm := newdeploy.MakeNewDeploy(
+		logger,
 		fissionClient, kubernetesClient, restClient,
 		functionNamespace, poolID)
 
-	api := MakeExecutor(gpm, ndm, fissionClient, fsCache)
+	api := MakeExecutor(logger, gpm, ndm, fissionClient, fsCache)
 
 	go api.Serve(port)
-	go serveMetric()
+	go serveMetric(logger)
 
 	return nil
 }

executor/executor_test.go

@@ -33,6 +33,7 @@ import (
 	"testing"
 	"time"
 
+	"go.uber.org/zap"
 	apiv1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -44,6 +45,12 @@ import (
 	"github.com/fission/fission/executor/client"
 )
 
+func panicIf(err error) {
+	if err != nil {
+		log.Panicf("Error: %v", err)
+	}
+}
+
 // return the number of pods in the given namespace matching the given labels
 func countPods(kubeClient *kubernetes.Clientset, ns string, labelz map[string]string) int {
 	pods, err := kubeClient.Pods(ns).List(metav1.ListOptions{
@@ -134,8 +141,11 @@ func TestExecutor(t *testing.T) {
 	createTestNamespace(kubeClient, functionNs)
 	defer kubeClient.Namespaces().Delete(functionNs, nil)
 
+	logger, err := zap.NewDevelopment()
+	panicIf(err)
+
 	// make sure CRD types exist on cluster
-	err = crd.EnsureFissionCRDs(apiExtClient)
+	err = crd.EnsureFissionCRDs(logger, apiExtClient)
 	if err != nil {
 		log.Panicf("failed to ensure crds: %v", err)
 	}
@@ -165,13 +175,13 @@ func TestExecutor(t *testing.T) {
 
 	// create poolmgr
 	port := 9999
-	err = StartExecutor(fissionNs, functionNs, port)
+	err = StartExecutor(logger, fissionNs, functionNs, "fission-builder", port)
 	if err != nil {
 		log.Panicf("failed to start poolmgr: %v", err)
 	}
 
 	// connect poolmgr client
-	poolmgrClient := client.MakeClient(fmt.Sprintf("http://localhost:%v", port))
+	poolmgrClient := client.MakeClient(logger, fmt.Sprintf("http://localhost:%v", port))
 
 	// Wait for pool to be created (we don't actually need to do
 	// this, since the API should do the right thing in any case).

executor/fscache/functionServiceCache.go

@@ -17,9 +17,11 @@ limitations under the License.
 package fscache
 
 import (
-	"log"
+	"fmt"
 	"time"
 
+	"github.com/pkg/errors"
+	"go.uber.org/zap"
 	apiv1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -57,6 +59,7 @@ type (
 	}
 
 	FunctionServiceCache struct {
+		logger        *zap.Logger
 		byFunction    *cache.Cache // function-key -> funcSvc  : map[string]*funcSvc
 		byAddress     *cache.Cache // address      -> function : map[string]metav1.ObjectMeta
 		byFunctionUID *cache.Cache // function uid -> function : map[string]metav1.ObjectMeta
@@ -91,8 +94,9 @@ func IsNameExistError(err error) bool {
 	return false
 }
 
-func MakeFunctionServiceCache() *FunctionServiceCache {
+func MakeFunctionServiceCache(logger *zap.Logger) *FunctionServiceCache {
 	fsc := &FunctionServiceCache{
+		logger:         logger.Named("function_service_cache"),
 		byFunction:     cache.MakeCache(0, 0),
 		byAddress:      cache.MakeCache(0, 0),
 		byFunctionUID:  cache.MakeCache(0, 0),
@@ -122,14 +126,16 @@ func (fsc *FunctionServiceCache) service() {
 			}
 			resp.objects = funcObjects
 		case LOG:
+			fsc.logger.Info("dumping function service cache")
 			funcCopy := fsc.byFunction.Copy()
-			log.Printf("Cache has %v entries", len(funcCopy))
+			info := []string{}
 			for key, fsvcI := range funcCopy {
 				fsvc := fsvcI.(*FuncSvc)
 				for _, kubeObj := range fsvc.KubernetesObjects {
-					log.Printf("%v\t%v\t%v", key, kubeObj.Kind, kubeObj.Name)
+					info = append(info, fmt.Sprintf("%v\t%v\t%v", key, kubeObj.Kind, kubeObj.Name))
 				}
 			}
+			fsc.logger.Info("function service cache", zap.Int("item_count", len(funcCopy)), zap.Strings("cache", info))
 		}
 		req.responseChannel <- resp
 	}
@@ -196,9 +202,8 @@ func (fsc *FunctionServiceCache) Add(fsvc FuncSvc) (*FuncSvc, error) {
 	if err != nil {
 		if IsNameExistError(err) {
 			err = nil
-		}
-		if err != nil {
-			log.Printf("error caching fsvc: %v", err)
+		} else {
+			err = errors.Wrap(err, "error caching fsvc")
 		}
 		return nil, err
 	}
@@ -209,9 +214,8 @@ func (fsc *FunctionServiceCache) Add(fsvc FuncSvc) (*FuncSvc, error) {
 	if err != nil {
 		if IsNameExistError(err) {
 			err = nil
-		}
-		if err != nil {
-			log.Printf("error caching fsvc by function uid: %v", err)
+		} else {
+			err = errors.Wrap(err, "error caching fsvc by function uid")
 		}
 		return nil, err
 	}
@@ -278,12 +282,12 @@ func (fsc *FunctionServiceCache) ListOld(age time.Duration) ([]*FuncSvc, error)
 }
 
 func (fsc *FunctionServiceCache) Log() {
-	log.Printf("--- FunctionService Cache Contents")
+	fsc.logger.Info("--- FunctionService Cache Contents")
 	responseChannel := make(chan *fscResponse)
 	fsc.requestChannel <- &fscRequest{
 		requestType:     LOG,
 		responseChannel: responseChannel,
 	}
 	<-responseChannel
-	log.Printf("--- FunctionService Cache Contents End")
+	fsc.logger.Info("--- FunctionService Cache Contents End")
 }

executor/fscache/functionServiceCache_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"go.uber.org/zap"
 	apiv1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -12,8 +13,17 @@ import (
 	"github.com/fission/fission/crd"
 )
 
+func panicIf(err error) {
+	if err != nil {
+		log.Panicf("Error: %v", err)
+	}
+}
+
 func TestFunctionServiceCache(t *testing.T) {
-	fsc := MakeFunctionServiceCache()
+	logger, err := zap.NewDevelopment()
+	panicIf(err)
+
+	fsc := MakeFunctionServiceCache(logger)
 	if fsc == nil {
 		log.Panicf("error creating cache")
 	}

executor/newdeploy/newdeploy.go

@@ -20,11 +20,11 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"log"
 	"path/filepath"
 	"time"
 
 	"github.com/hashicorp/go-multierror"
+	"go.uber.org/zap"
 	asv1 "k8s.io/api/autoscaling/v1"
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
@@ -59,10 +59,9 @@ func (deploy *NewDeploy) createOrGetDeployment(fn *crd.Function, env *crd.Enviro
 	existingDepl, err := deploy.kubernetesClient.ExtensionsV1beta1().Deployments(deployNamespace).Get(deployName, metav1.GetOptions{})
 	if err == nil {
 		if waitForDeploy {
-			err = scaleDeployment(deploy.kubernetesClient,
-				existingDepl.Namespace, existingDepl.Name, minScale)
+			err = deploy.scaleDeployment(existingDepl.Namespace, existingDepl.Name, minScale)
 			if err != nil {
-				log.Printf("Error scaling up deployment for function %v: %v", fn.Metadata.Name, err)
+				deploy.logger.Error("error scaling up function deployment", zap.Error(err), zap.String("function", fn.Metadata.Name))
 				return nil, err
 			}
 
@@ -86,7 +85,11 @@ func (deploy *NewDeploy) createOrGetDeployment(fn *crd.Function, env *crd.Enviro
 
 		depl, err := deploy.kubernetesClient.ExtensionsV1beta1().Deployments(deployNamespace).Create(deployment)
 		if err != nil {
-			log.Printf("Error while creating deployment: %v", err)
+			deploy.logger.Error("error while creating function deployment",
+				zap.Error(err),
+				zap.String("function", fn.Metadata.Name),
+				zap.String("deployment_name", deployName),
+				zap.String("deployment_namespace", deployNamespace))
 			return nil, err
 		}
 
@@ -105,25 +108,40 @@ func (deploy *NewDeploy) setupRBACObjs(deployNamespace string, fn *crd.Function)
 	// create fetcher SA in this ns, if not already created
 	_, err := fission.SetupSA(deploy.kubernetesClient, fission.FissionFetcherSA, deployNamespace)
 	if err != nil {
-		log.Printf("Error : %v creating %s in ns : %s for function: %s.%s", err, fission.FissionFetcherSA, deployNamespace, fn.Metadata.Name, fn.Metadata.Namespace)
+		deploy.logger.Error("error creating fission fetcher service account for function",
+			zap.Error(err),
+			zap.String("service_account_name", fission.FissionFetcherSA),
+			zap.String("service_account_namespace", deployNamespace),
+			zap.String("function_name", fn.Metadata.Name),
+			zap.String("function_namespace", fn.Metadata.Namespace))
 		return err
 	}
 
 	// create a cluster role binding for the fetcher SA, if not already created, granting access to do a get on packages in any ns
-	err = fission.SetupRoleBinding(deploy.kubernetesClient, fission.PackageGetterRB, fn.Spec.Package.PackageRef.Namespace, fission.PackageGetterCR, fission.ClusterRole, fission.FissionFetcherSA, deployNamespace)
+	err = fission.SetupRoleBinding(deploy.logger, deploy.kubernetesClient, fission.PackageGetterRB, fn.Spec.Package.PackageRef.Namespace, fission.PackageGetterCR, fission.ClusterRole, fission.FissionFetcherSA, deployNamespace)
 	if err != nil {
-		log.Printf("Error : %v creating %s RoleBinding for function: %s.%s", err, fission.PackageGetterRB, fn.Metadata.Name, fn.Metadata.Namespace)
+		deploy.logger.Error("error creating role binding for function",
+			zap.Error(err),
+			zap.String("role_binding", fission.PackageGetterRB),
+			zap.String("function_name", fn.Metadata.Name),
+			zap.String("function_namespace", fn.Metadata.Namespace))
 		return err
 	}
 
 	// create rolebinding in function namespace for fetcherSA.envNamespace to be able to get secrets and configmaps
-	err = fission.SetupRoleBinding(deploy.kubernetesClient, fission.SecretConfigMapGetterRB, fn.Metadata.Namespace, fission.SecretConfigMapGetterCR, fission.ClusterRole, fission.FissionFetcherSA, deployNamespace)
+	err = fission.SetupRoleBinding(deploy.logger, deploy.kubernetesClient, fission.SecretConfigMapGetterRB, fn.Metadata.Namespace, fission.SecretConfigMapGetterCR, fission.ClusterRole, fission.FissionFetcherSA, deployNamespace)
 	if err != nil {
-		log.Printf("Error : %v creating %s RoleBinding for function %s.%s", err, fission.SecretConfigMapGetterRB, fn.Metadata.Name, fn.Metadata.Namespace)
+		deploy.logger.Error("error creating role binding for function",
+			zap.Error(err),
+			zap.String("role_binding", fission.SecretConfigMapGetterRB),
+			zap.String("function_name", fn.Metadata.Name),
+			zap.String("function_namespace", fn.Metadata.Namespace))
 		return err
 	}
 
-	log.Printf("Set up all RBAC objects for function : %s.%s", fn.Metadata.Name, fn.Metadata.Namespace)
+	deploy.logger.Info("set up all RBAC objects for function",
+		zap.String("function_name", fn.Metadata.Name),
+		zap.String("function_namespace", fn.Metadata.Namespace))
 	return nil
 }
 
@@ -188,7 +206,7 @@ func (deploy *NewDeploy) getDeploymentSpec(fn *crd.Function, env *crd.Environmen
 
 	fetcherResources, err := util.GetFetcherResources()
 	if err != nil {
-		log.Printf("Error while parsing fetcher resources: %v", err)
+		deploy.logger.Error("error while parsing fetcher resources", zap.Error(err))
 		return nil, err
 	}
 
@@ -515,19 +533,28 @@ func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error {
 
 	err := deploy.deleteSvc(ns, name)
 	if err != nil {
-		log.Printf("Error deleting service for newdeploy function %v in namespace %v, error: %v", name, ns, err)
+		deploy.logger.Error("error deleting service for newdeploy function",
+			zap.Error(err),
+			zap.String("function_name", name),
+			zap.String("function_namespace", ns))
 		multierror.Append(multierr, err)
 	}
 
 	err = deploy.deleteHpa(ns, name)
 	if err != nil {
-		log.Printf("Error deleting HPA for newdeploy function %v in namespace %v, error: %v", name, ns, err)
+		deploy.logger.Error("error deleting service for newdeploy function",
+			zap.Error(err),
+			zap.String("function_name", name),
+			zap.String("function_namespace", ns))
 		multierror.Append(multierr, err)
 	}
 
 	err = deploy.deleteDeployment(ns, name)
 	if err != nil {
-		log.Printf("Error deleting deployment for newdeploy function %v in namespace %v, error: %v", name, ns, err)
+		deploy.logger.Error("error deleting deployment for newdeploy function",
+			zap.Error(err),
+			zap.String("function_name", name),
+			zap.String("function_namespace", ns))
 		multierror.Append(multierr, err)
 	}
 	return multierr.ErrorOrNil()

executor/newdeploy/newdeploymgr.go

@@ -19,7 +19,6 @@ package newdeploy
 import (
 	"context"
 	"fmt"
-	"log"
 	"os"
 	"strconv"
 	"strings"
@@ -29,6 +28,7 @@ import (
 	"github.com/fission/fission/throttler"
 	multierror "github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
+	"go.uber.org/zap"
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
 	k8sErrs "k8s.io/apimachinery/pkg/api/errors"
@@ -46,6 +46,8 @@ import (
 
 type (
 	NewDeploy struct {
+		logger *zap.Logger
+
 		kubernetesClient *kubernetes.Clientset
 		fissionClient    *crd.FissionClient
 		crdClient        *rest.RESTClient
@@ -72,6 +74,7 @@ type (
 )
 
 func MakeNewDeploy(
+	logger *zap.Logger,
 	fissionClient *crd.FissionClient,
 	kubernetesClient *kubernetes.Clientset,
 	crdClient *rest.RESTClient,
@@ -79,7 +82,7 @@ func MakeNewDeploy(
 	instanceID string,
 ) *NewDeploy {
 
-	log.Printf("Creating NewDeploy ExecutorType")
+	logger.Info("creating NewDeploy ExecutorType")
 
 	fetcherImg := os.Getenv("FETCHER_IMAGE")
 	if len(fetcherImg) == 0 {
@@ -92,19 +95,21 @@ func MakeNewDeploy(
 	if len(os.Getenv("ENABLE_ISTIO")) > 0 {
 		istio, err := strconv.ParseBool(os.Getenv("ENABLE_ISTIO"))
 		if err != nil {
-			log.Println("Failed to parse ENABLE_ISTIO")
+			logger.Info("failed to parse 'ENABLE_ISTIO'")
 		}
 		enableIstio = istio
 	}
 
 	nd := &NewDeploy{
+		logger: logger.Named("new_deploy"),
+
 		fissionClient:    fissionClient,
 		kubernetesClient: kubernetesClient,
 		crdClient:        crdClient,
 		instanceID:       instanceID,
 
 		namespace: namespace,
-		fsCache:   fscache.MakeFunctionServiceCache(),
+		fsCache:   fscache.MakeFunctionServiceCache(logger),
 		throttler: throttler.MakeThrottler(1 * time.Minute),
 
 		fetcherImg:             fetcherImg,
@@ -142,14 +147,18 @@ func (deploy *NewDeploy) initFuncController() (k8sCache.Store, k8sCache.Controll
 			fn := obj.(*crd.Function)
 			_, err := deploy.createFunction(fn, true)
 			if err != nil {
-				log.Printf("Error eager creating function: %v", err)
+				deploy.logger.Error("error eager creating function",
+					zap.Error(err),
+					zap.Any("function", fn))
 			}
 		},
 		DeleteFunc: func(obj interface{}) {
 			fn := obj.(*crd.Function)
 			err := deploy.deleteFunction(fn)
 			if err != nil {
-				log.Printf("Error deleing the function: %v", err)
+				deploy.logger.Error("error deleting function",
+					zap.Error(err),
+					zap.Any("function", fn))
 			}
 		},
 		UpdateFunc: func(oldObj interface{}, newObj interface{}) {
@@ -157,7 +166,10 @@ func (deploy *NewDeploy) initFuncController() (k8sCache.Store, k8sCache.Controll
 			newFn := newObj.(*crd.Function)
 			err := deploy.updateFunction(oldFn, newFn)
 			if err != nil {
-				log.Printf("Error deleing the function: %v", err)
+				deploy.logger.Error("error updating function",
+					zap.Error(err),
+					zap.Any("old_function", oldFn),
+					zap.Any("new_function", newFn))
 			}
 		},
 	})
@@ -186,7 +198,7 @@ func (deploy *NewDeploy) createFunction(fn *crd.Function, firstcreate bool) (*fs
 
 	fsvc, ok := fsvcObj.(*fscache.FuncSvc)
 	if !ok {
-		log.Panic("Receive unknown object, expect pointer of function service object")
+		deploy.logger.Panic("receive unknown object while creating function - expected pointer of function service object")
 	}
 
 	return fsvc, err
@@ -198,7 +210,7 @@ func (deploy *NewDeploy) deleteFunction(fn *crd.Function) error {
 	}
 	err := deploy.fnDelete(fn)
 	if err != nil {
-		err = errors.Wrap(err, fmt.Sprintf("error deleting kubernetes objects of function %v", fn.Metadata))
+		err = errors.Wrapf(err, "error deleting kubernetes objects of function %v", fn.Metadata)
 	}
 	return err
 }
@@ -235,22 +247,23 @@ func (deploy *NewDeploy) fnCreate(fn *crd.Function, firstcreate bool) (*fscache.
 	// then deployment) to take advantage of waiting time.
 	svc, err := deploy.createOrGetSvc(deployLabels, objName, ns)
 	if err != nil {
-		log.Printf("Error creating the service %v: %v", objName, err)
+		deploy.logger.Error("error creating service", zap.Error(err), zap.String("service", objName))
 		go deploy.cleanupNewdeploy(ns, objName)
-		return nil, errors.Wrap(err, fmt.Sprintf("error creating service %v", objName))
+		return nil, errors.Wrapf(err, "error creating service %v", objName)
 	}
 	svcAddress := fmt.Sprintf("%v.%v", svc.Name, svc.Namespace)
 	depl, err := deploy.createOrGetDeployment(fn, env, objName, deployLabels, ns, firstcreate)
 	if err != nil {
-		log.Printf("Error creating the deployment %v: %v", objName, err)
+		deploy.logger.Error("error creating deployment", zap.Error(err), zap.String("deployment", objName))
 		go deploy.cleanupNewdeploy(ns, objName)
-		return nil, errors.Wrap(err, fmt.Sprintf("error creating deployment %v", objName))
+		return nil, errors.Wrapf(err, "error creating deployment %v", objName)
 	}
 
 	hpa, err := deploy.createOrGetHpa(objName, &fn.Spec.InvokeStrategy.ExecutionStrategy, depl)
 	if err != nil {
+		deploy.logger.Error("error creating HPA", zap.Error(err), zap.String("hpa", objName))
 		go deploy.cleanupNewdeploy(ns, objName)
-		return nil, errors.Wrap(err, fmt.Sprintf("error creating the HPA %v:", objName))
+		return nil, errors.Wrapf(err, "error creating the HPA %v", objName)
 	}
 
 	kubeObjRefs := []apiv1.ObjectReference{
@@ -292,7 +305,7 @@ func (deploy *NewDeploy) fnCreate(fn *crd.Function, firstcreate bool) (*fscache.
 
 	_, err = deploy.fsCache.Add(*fsvc)
 	if err != nil {
-		log.Printf("Error adding the function to cache: %v", err)
+		deploy.logger.Error("error adding function to cache", zap.Error(err), zap.Any("function", fsvc.Function))
 		return fsvc, err
 	}
 	return fsvc, nil
@@ -313,7 +326,8 @@ func (deploy *NewDeploy) updateFunction(oldFn *crd.Function, newFn *crd.Function
 	// Executor type is no longer New Deployment
 	if newFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy &&
 		oldFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fission.ExecutorTypeNewdeploy {
-		log.Printf("function does not use new deployment executor anymore, deleting resources: %v", newFn)
+		deploy.logger.Info("function does not use new deployment executor anymore, deleting resources",
+			zap.Any("function", newFn))
 		// IMP - pass the oldFn, as the new/modified function is not in cache
 		return deploy.deleteFunction(oldFn)
 	}
@@ -321,17 +335,19 @@ func (deploy *NewDeploy) updateFunction(oldFn *crd.Function, newFn *crd.Function
 	// Executor type changed to New Deployment from something else
 	if oldFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy &&
 		newFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fission.ExecutorTypeNewdeploy {
-		log.Printf("function type changed to new deployment, creating resources: %v %v", oldFn.Metadata, newFn.Metadata)
+		deploy.logger.Info("function type changed to new deployment, creating resources",
+			zap.Any("old_function", oldFn.Metadata),
+			zap.Any("new_function", newFn.Metadata))
 		_, err := deploy.createFunction(newFn, true)
 		if err != nil {
-			updateStatus(oldFn, err, "error changing the function's type to newdeploy")
+			deploy.updateStatus(oldFn, err, "error changing the function's type to newdeploy")
 		}
 		return err
 	}
 
 	fsvc, err := deploy.fsCache.GetByFunctionUID(newFn.Metadata.UID)
 	if err != nil {
-		err = errors.Wrap(err, fmt.Sprintf("error updating function due to unable to find function service cache: %v", oldFn))
+		err = errors.Wrapf(err, "error updating function due to unable to find function service cache: %v", oldFn)
 		return err
 	}
 
@@ -349,7 +365,7 @@ func (deploy *NewDeploy) updateFunction(oldFn *crd.Function, newFn *crd.Function
 
 		hpa, err := deploy.getHpa(ns, fnObjName)
 		if err != nil {
-			updateStatus(oldFn, err, "error getting HPA while updating function")
+			deploy.updateStatus(oldFn, err, "error getting HPA while updating function")
 			return err
 		}
 
@@ -375,7 +391,7 @@ func (deploy *NewDeploy) updateFunction(oldFn *crd.Function, newFn *crd.Function
 		if hpaChanged {
 			err := deploy.updateHpa(hpa)
 			if err != nil {
-				updateStatus(oldFn, err, "error updating HPA while updating function")
+				deploy.updateStatus(oldFn, err, "error updating HPA while updating function")
 				return err
 			}
 		}
@@ -413,15 +429,15 @@ func (deploy *NewDeploy) updateFunction(oldFn *crd.Function, newFn *crd.Function
 		env, err := deploy.fissionClient.Environments(newFn.Spec.Environment.Namespace).
 			Get(newFn.Spec.Environment.Name)
 		if err != nil {
-			updateStatus(oldFn, err, "failed to get environment while updating function")
+			deploy.updateStatus(oldFn, err, "failed to get environment while updating function")
 			return err
 		}
 
 		deployLabels := deploy.getDeployLabels(oldFn, env)
-		log.Printf("updating %v deployment due to function %v update", fnObjName, newFn.Metadata.Name)
+		deploy.logger.Info("updating deployment due to function update", zap.String("deployment", fnObjName), zap.String("function", newFn.Metadata.Name))
 		newDeployment, err := deploy.getDeploymentSpec(newFn, env, fnObjName, deployLabels)
 		if err != nil {
-			updateStatus(oldFn, err, "failed to get new deployment spec while updating function")
+			deploy.updateStatus(oldFn, err, "failed to get new deployment spec while updating function")
 			return err
 		}
 
@@ -434,7 +450,7 @@ func (deploy *NewDeploy) updateFunction(oldFn *crd.Function, newFn *crd.Function
 
 		err = deploy.updateDeployment(newDeployment, ns)
 		if err != nil {
-			updateStatus(oldFn, err, "failed to update deployment while updating function")
+			deploy.updateStatus(oldFn, err, "failed to update deployment while updating function")
 			return err
 		}
 	}
@@ -506,13 +522,13 @@ func (deploy *NewDeploy) updateKubeObjRefRV(fsvc *fscache.FuncSvc, objKind strin
 		}
 	}
 	fsvc.KubernetesObjects = kubeObjs
-	return errors.New(fmt.Sprintf("error finding kubernetes object reference with kind: %v", objKind))
+	return fmt.Errorf("error finding kubernetes object reference with kind: %v", objKind)
 }
 
 // updateStatus is a function which updates status of update.
 // Current implementation only logs messages, in future it will update function status
-func updateStatus(fn *crd.Function, err error, message string) {
-	log.Println(message, fn, err)
+func (deploy *NewDeploy) updateStatus(fn *crd.Function, err error, message string) {
+	deploy.logger.Info("function status update", zap.Error(err), zap.Any("function", fn), zap.String("message", message))
 }
 
 // IsValid does a get on the service address to ensure it's a valid service, then
@@ -526,20 +542,20 @@ func (deploy *NewDeploy) IsValid(fsvc *fscache.FuncSvc) bool {
 
 	_, err := deploy.kubernetesClient.CoreV1().Services(service[1]).Get(service[0], metav1.GetOptions{})
 	if err != nil {
-		log.Printf("Error validating service address for function %v: %v", fsvc.Function.Name, err)
+		deploy.logger.Error("error validating function service address", zap.String("function", fsvc.Function.Name), zap.Error(err))
 		return false
 	}
 
 	deployObj := getDeploymentObj(fsvc.KubernetesObjects)
 	if deployObj == nil {
-		log.Printf("Deployment obj for function %v does not exist", fsvc.Function.Name)
+		deploy.logger.Error("deployment obj for function does not exist", zap.String("function", fsvc.Function.Name))
 		return false
 	}
 
 	currentDeploy, err := deploy.kubernetesClient.ExtensionsV1beta1().
 		Deployments(deployObj.Namespace).Get(deployObj.Name, metav1.GetOptions{})
 	if err != nil {
-		log.Printf("Error validating deployment for function %v: %v", fsvc.Function.Name, err)
+		deploy.logger.Error("error validating function deployment", zap.Error(err), zap.String("function", fsvc.Function.Name))
 		return false
 	}
 
@@ -560,7 +576,7 @@ func (deploy *NewDeploy) idleObjectReaper() {
 
 		envs, err := deploy.fissionClient.Environments(metav1.NamespaceAll).List(metav1.ListOptions{})
 		if err != nil {
-			log.Fatalf("Failed to get environment list: %v", err)
+			deploy.logger.Fatal("failed to get environment list", zap.Error(err))
 		}
 
 		envList := make(map[types.UID]struct{})
@@ -570,7 +586,7 @@ func (deploy *NewDeploy) idleObjectReaper() {
 
 		funcSvcs, err := deploy.fsCache.ListOld(deploy.idlePodReapTime)
 		if err != nil {
-			log.Printf("Error reaping idle pods: %v", err)
+			deploy.logger.Error("error reaping idle pods", zap.Error(err))
 			continue
 		}
 
@@ -582,8 +598,9 @@ func (deploy *NewDeploy) idleObjectReaper() {
 			// For function with the environment that no longer exists, executor
 			// scales down the deployment as usual and prints log to notify user.
 			if _, ok := envList[fsvc.Environment.Metadata.UID]; !ok {
-				log.Printf("Environment %v for function %v no longer exists",
-					fsvc.Environment.Metadata.Name, fsvc.Name)
+				deploy.logger.Error("function environment no longer exists",
+					zap.String("environment", fsvc.Environment.Metadata.Name),
+					zap.String("function", fsvc.Name))
 			}
 
 			fn, err := deploy.fissionClient.Functions(fsvc.Function.Namespace).Get(fsvc.Function.Name)
@@ -593,20 +610,20 @@ func (deploy *NewDeploy) idleObjectReaper() {
 				if k8sErrs.IsNotFound(err) && fsvc.Executor == fscache.NEWDEPLOY {
 					continue
 				}
-				log.Printf("Error getting function: %v", fsvc.Function.Name)
+				deploy.logger.Error("error getting function", zap.Error(err), zap.String("function", fsvc.Function.Name))
 				continue
 			}
 
 			deployObj := getDeploymentObj(fsvc.KubernetesObjects)
 			if deployObj == nil {
-				log.Printf("Error finding deployment for function %v: %v", fsvc.Function.Name, err)
+				deploy.logger.Error("error finding function deployment", zap.Error(err), zap.String("function", fsvc.Function.Name))
 				continue
 			}
 
 			currentDeploy, err := deploy.kubernetesClient.ExtensionsV1beta1().
 				Deployments(deployObj.Namespace).Get(deployObj.Name, metav1.GetOptions{})
 			if err != nil {
-				log.Printf("Error validating deployment for function %v: %v", fsvc.Function.Name, err)
+				deploy.logger.Error("error validating function deployment", zap.Error(err), zap.String("function", fsvc.Function.Name))
 				continue
 			}
 
@@ -617,9 +634,9 @@ func (deploy *NewDeploy) idleObjectReaper() {
 				continue
 			}
 
-			err = scaleDeployment(deploy.kubernetesClient, deployObj.Namespace, deployObj.Name, minScale)
+			err = deploy.scaleDeployment(deployObj.Namespace, deployObj.Name, minScale)
 			if err != nil {
-				log.Printf("Error scaling down deployment for function %v: %v", fsvc.Function.Name, err)
+				deploy.logger.Error("error scaling down function deployment", zap.Error(err), zap.String("function", fsvc.Function.Name))
 			}
 		}
 	}
@@ -635,9 +652,12 @@ func getDeploymentObj(kubeobjs []apiv1.ObjectReference) *apiv1.ObjectReference {
 	return nil
 }
 
-func scaleDeployment(client *kubernetes.Clientset, deplNS string, deplName string, replicas int32) error {
-	log.Printf("Scale deployment %v in namespace %v to replicas %v", deplName, deplNS, replicas)
-	_, err := client.ExtensionsV1beta1().Deployments(deplNS).UpdateScale(deplName, &v1beta1.Scale{
+func (deploy *NewDeploy) scaleDeployment(deplNS string, deplName string, replicas int32) error {
+	deploy.logger.Info("scaling deployment",
+		zap.String("deployment", deplName),
+		zap.String("namespace", deplNS),
+		zap.Int32("replicas", replicas))
+	_, err := deploy.kubernetesClient.ExtensionsV1beta1().Deployments(deplNS).UpdateScale(deplName, &v1beta1.Scale{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      deplName,
 			Namespace: deplNS,

executor/poolmgr/funcwatcher.go

@@ -19,7 +19,7 @@ package poolmgr
 import (
 	"time"
 
-	log "github.com/sirupsen/logrus"
+	"go.uber.org/zap"
 	apiv1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -72,11 +72,15 @@ func (gpm *GenericPoolManager) makeFuncController(fissionClient *crd.FissionClie
 				// setup rolebinding is tried, if it fails, we dont return. we just log an error and move on, because :
 				// 1. not all functions have secrets and/or configmaps, so things will work without this rolebinding in that case.
 				// 2. on the contrary, when the route is tried, the env fetcher logs will show a 403 forbidden message and same will be relayed to executor.
-				err := fission.SetupRoleBinding(kubernetesClient, fission.SecretConfigMapGetterRB, fn.Metadata.Namespace, fission.SecretConfigMapGetterCR, fission.ClusterRole, fission.FissionFetcherSA, envNs)
+				err := fission.SetupRoleBinding(gpm.logger, kubernetesClient, fission.SecretConfigMapGetterRB, fn.Metadata.Namespace, fission.SecretConfigMapGetterCR, fission.ClusterRole, fission.FissionFetcherSA, envNs)
 				if err != nil {
-					log.Printf("Error : %v creating %s RoleBinding", err, fission.SecretConfigMapGetterRB)
+					gpm.logger.Error("error creating rolebinding", zap.Error(err), zap.String("role_binding", fission.SecretConfigMapGetterRB))
 				} else {
-					log.Printf("Successfully set up rolebinding for fetcher SA: %s.%s, in func's ns for func : %s/%s", fission.FissionFetcherSA, envNs, fn.Metadata.Namespace, fn.Metadata.Name)
+					gpm.logger.Info("successfully set up rolebinding for fetcher service account for function",
+						zap.String("service_account", fission.FissionFetcherSA),
+						zap.String("service_account_namepsace", envNs),
+						zap.String("function_name", fn.Metadata.Name),
+						zap.String("function_namespace", fn.Metadata.Namespace))
 				}
 
 				if istioEnabled {
@@ -126,7 +130,11 @@ func (gpm *GenericPoolManager) makeFuncController(fissionClient *crd.FissionClie
 					// create function istio service if it does not exist
 					_, err = kubernetesClient.CoreV1().Services(envNs).Create(&svc)
 					if err != nil && !kerrors.IsAlreadyExists(err) {
-						log.Printf("Error creating function istio service: %v", err)
+						gpm.logger.Error("error creating istio service for function",
+							zap.Error(err),
+							zap.String("service_name", svcName),
+							zap.String("function_name", fn.Metadata.Name),
+							zap.Any("selectors", sel))
 					}
 				}
 			},
@@ -149,7 +157,11 @@ func (gpm *GenericPoolManager) makeFuncController(fissionClient *crd.FissionClie
 					// delete function istio service
 					err := kubernetesClient.CoreV1().Services(envNs).Delete(svcName, nil)
 					if err != nil && !kerrors.IsNotFound(err) {
-						log.Printf("Error deleting function istio service: %v", err)
+						gpm.logger.Error("error deleting istio service for function",
+							zap.Error(err),
+							zap.String("service_name", svcName),
+							zap.String("function_name", fn.Metadata.Name))
+
 					}
 				}
 			},
@@ -178,13 +190,18 @@ func (gpm *GenericPoolManager) makeFuncController(fissionClient *crd.FissionClie
 						envNs = newFunc.Spec.Environment.Namespace
 					}
 
-					err := fission.SetupRoleBinding(kubernetesClient, fission.SecretConfigMapGetterRB,
+					err := fission.SetupRoleBinding(gpm.logger, kubernetesClient, fission.SecretConfigMapGetterRB,
 						newFunc.Metadata.Namespace, fission.SecretConfigMapGetterCR, fission.ClusterRole,
 						fission.FissionFetcherSA, envNs)
+
 					if err != nil {
-						log.Printf("Error : %v creating GetSecretConfigMapRoleBinding", err)
+						gpm.logger.Error("error creating rolebinding", zap.Error(err), zap.String("role_binding", fission.SecretConfigMapGetterRB))
 					} else {
-						log.Printf("Set up rolebinding for fetcher SA in func's env ns : %s, in func's ns : %s, for func : %s", newFunc.Spec.Environment.Namespace, newFunc.Metadata.Namespace, newFunc.Metadata.Name)
+						gpm.logger.Info("successfully set up rolebinding for fetcher service account for function",
+							zap.String("service_account", fission.FissionFetcherSA),
+							zap.String("service_account_namepsace", envNs),
+							zap.String("function_name", newFunc.Metadata.Name),
+							zap.String("function_namespace", newFunc.Metadata.Namespace))
 					}
 				}
 			},

executor/poolmgr/gp.go

@@ -19,7 +19,6 @@ package poolmgr
 import (
 	"context"
 	"fmt"
-	"log"
 	"math/rand"
 	"net"
 	"os"
@@ -30,6 +29,7 @@ import (
 	"github.com/dchest/uniuri"
 	multierror "github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
+	"go.uber.org/zap"
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -46,6 +46,7 @@ import (
 
 type (
 	GenericPool struct {
+		logger                 *zap.Logger
 		env                    *crd.Environment
 		replicas               int32                         // num idle pods
 		deployment             *v1beta1.Deployment           // kubernetes deployment
@@ -83,6 +84,7 @@ type (
 )
 
 func MakeGenericPool(
+	logger *zap.Logger,
 	fissionClient *crd.FissionClient,
 	kubernetesClient *kubernetes.Clientset,
 	env *crd.Environment,
@@ -94,7 +96,9 @@ func MakeGenericPool(
 	enableIstio bool,
 	collectorEndpoint string) (*GenericPool, error) {
 
-	log.Printf("Creating pool for environment %v", env.Metadata)
+	gpLogger := logger.Named("generic_pool")
+
+	gpLogger.Info("creating pool", zap.Any("environment", env.Metadata))
 
 	fetcherImage := os.Getenv("FETCHER_IMAGE")
 	if len(fetcherImage) == 0 {
@@ -104,6 +108,7 @@ func MakeGenericPool(
 	// TODO: in general we need to provide the user a way to configure pools.  Initial
 	// replicas, autoscaling params, various timeouts, etc.
 	gp := &GenericPool{
+		logger:            gpLogger,
 		env:               env,
 		replicas:          initialReplicas, // TODO make this an env param instead?
 		requestChannel:    make(chan *choosePodRequest),
@@ -128,13 +133,12 @@ func MakeGenericPool(
 	gp.runtimeImagePullPolicy = fission.GetImagePullPolicy(os.Getenv("RUNTIME_IMAGE_PULL_POLICY"))
 	gp.fetcherImagePullPolicy = fission.GetImagePullPolicy(os.Getenv("FETCHER_IMAGE_PULL_POLICY"))
 
-	log.Printf("fetcher image: %v, pull policy: %v", gp.fetcherImage, gp.fetcherImagePullPolicy)
+	gpLogger.Info("fetcher", zap.String("image", gp.fetcherImage), zap.Any("pull_policy", gp.fetcherImagePullPolicy))
 
 	// create fetcher SA in this ns, if not already created
 	_, err := fission.SetupSA(gp.kubernetesClient, fission.FissionFetcherSA, gp.namespace)
 	if err != nil {
-		log.Printf("Error : %v creating fetcher SA in ns : %s", err, gp.namespace)
-		return nil, err
+		return nil, errors.Wrapf(err, "error creating fetcher service account in namespace %q", gp.namespace)
 	}
 
 	// Labels for generic deployment/RS/pods.
@@ -145,7 +149,7 @@ func MakeGenericPool(
 	if err != nil {
 		return nil, err
 	}
-	log.Printf("[%v] Deployment created", env.Metadata)
+	gpLogger.Info("deployment created", zap.Any("environment", env.Metadata))
 
 	go gp.choosePodService()
 
@@ -195,7 +199,7 @@ func (gp *GenericPool) _choosePod(newLabels map[string]string) (*apiv1.Pod, erro
 	for {
 		// Retries took too long, error out.
 		if time.Since(startTime) > gp.podReadyTimeout {
-			log.Printf("[%v] Erroring out, timed out", newLabels)
+			gp.logger.Error("timed out waiting for pod", zap.Any("labels", newLabels), zap.Duration("timeout", gp.podReadyTimeout))
 			return nil, errors.New("timeout: waited too long to get a ready pod")
 		}
 
@@ -220,7 +224,10 @@ func (gp *GenericPool) _choosePod(newLabels map[string]string) (*apiv1.Pod, erro
 			// add it to the list of ready pods
 			readyPods = append(readyPods, &pod)
 		}
-		log.Printf("[%v] found %v ready pods of %v total", newLabels, len(readyPods), len(podList.Items))
+		gp.logger.Info("found ready pods",
+			zap.Any("labels", newLabels),
+			zap.Int("ready_count", len(readyPods)),
+			zap.Int("total", len(podList.Items)))
 
 		// If there are no ready pods, wait and retry.
 		if len(readyPods) == 0 {
@@ -241,14 +248,14 @@ func (gp *GenericPool) _choosePod(newLabels map[string]string) (*apiv1.Pod, erro
 			// modified, this should fail; in that case just
 			// retry.
 			chosenPod.ObjectMeta.Labels = newLabels
-			log.Printf("relabeling pod: [%v]", chosenPod.ObjectMeta.Name)
+			gp.logger.Info("relabeling pod", zap.String("pod", chosenPod.ObjectMeta.Name))
 			_, err = gp.kubernetesClient.CoreV1().Pods(gp.namespace).Update(chosenPod)
 			if err != nil {
-				log.Printf("failed to relabel pod [%v]: %v", chosenPod.ObjectMeta.Name, err)
+				gp.logger.Error("failed to relabel pod", zap.Error(err), zap.String("pod", chosenPod.ObjectMeta.Name))
 				continue
 			}
 		}
-		log.Printf("Chosen pod: %v (in %v)", chosenPod.ObjectMeta.Name, time.Since(startTime))
+		gp.logger.Info("chose pod", zap.String("pod", chosenPod.ObjectMeta.Name), zap.Duration("elapsed_time", time.Since(startTime)))
 		return chosenPod, nil
 	}
 }
@@ -267,7 +274,7 @@ func (gp *GenericPool) scheduleDeletePod(name string) {
 		// The sleep allows debugging or collecting logs from the pod before it's
 		// cleaned up.  (We need a better solutions for both those things; log
 		// aggregation and storage will help.)
-		log.Printf("Error in pod '%v', scheduling cleanup", name)
+		gp.logger.Error("error in pod - scheduling cleanup", zap.String("pod", name))
 		// Ignore sleep here if istio feature is enabled, function pod
 		// will be deleted after 6 mins (terminationGracePeriodSeconds).
 		if !gp.useIstio {
@@ -319,7 +326,7 @@ func (gp *GenericPool) specializePod(ctx context.Context, pod *apiv1.Pod, metada
 
 	// tell fetcher to get the function.
 	fetcherUrl := gp.getSpecializeUrl(podIP)
-	log.Printf("[%v] calling fetcher to copy function with fetcher url: %v", metadata.Name, fetcherUrl)
+	gp.logger.Info("calling fetcher to copy function", zap.String("function", metadata.Name), zap.String("url", fetcherUrl))
 
 	fn, err := gp.fissionClient.
 		Functions(metadata.Namespace).
@@ -356,9 +363,9 @@ func (gp *GenericPool) specializePod(ctx context.Context, pod *apiv1.Pod, metada
 		},
 	}
 
-	log.Printf("[%v] specializing pod", metadata.Name)
+	gp.logger.Info("specializing pod", zap.String("function", metadata.Name))
 
-	err = fetcherClient.MakeClient(fetcherUrl).Specialize(ctx, &specializeReq)
+	err = fetcherClient.MakeClient(gp.logger, fetcherUrl).Specialize(ctx, &specializeReq)
 	if err != nil {
 		return err
 	}
@@ -550,7 +557,7 @@ func (gp *GenericPool) createPool() error {
 
 	depl, err := gp.kubernetesClient.ExtensionsV1beta1().Deployments(gp.namespace).Create(deployment)
 	if err != nil {
-		log.Printf("Error creating deployment for %s in kubernetes, err: %v", deployment.Name, err)
+		gp.logger.Error("error creating deployment in kubernetes", zap.Error(err), zap.String("deployment", deployment.Name))
 		return err
 	}
 	gp.deployment = depl
@@ -564,11 +571,9 @@ func (gp *GenericPool) waitForReadyPod() error {
 		depl, err := gp.kubernetesClient.ExtensionsV1beta1().Deployments(gp.namespace).Get(
 			gp.deployment.ObjectMeta.Name, metav1.GetOptions{})
 		if err != nil {
-			err = errors.Wrap(err, fmt.Sprintf(
-				"Error waiting for ready pod of deployment %v in namespace %v",
-				gp.deployment.ObjectMeta.Name, gp.namespace))
-			log.Print(err)
-			return err
+			e := "error waiting for ready pod for deployment"
+			gp.logger.Error(e, zap.String("deployment", gp.deployment.ObjectMeta.Name), zap.String("namespace", gp.namespace))
+			return fmt.Errorf("%s %q in namespace %q", e, gp.deployment.ObjectMeta.Name, gp.namespace)
 		}
 
 		gp.deployment = depl
@@ -582,7 +587,7 @@ func (gp *GenericPool) waitForReadyPod() error {
 					gp.deployment.Spec.Selector.MatchLabels).AsSelector().String(),
 			})
 			if err != nil {
-				log.Printf("Error getting pod list after timeout waiting for ready pod: %v", err)
+				gp.logger.Error("error getting pod list after timeout waiting for ready pod", zap.Error(err))
 			}
 
 			// Since even single pod is not ready, choosing the first pod to inspect is a good approximation. In future this can be done better
@@ -593,9 +598,8 @@ func (gp *GenericPool) waitForReadyPod() error {
 					multierr = multierror.Append(multierr, errors.New(fmt.Sprintf("%v: %v", cStatus.State.Waiting.Reason, cStatus.State.Waiting.Message)))
 				}
 			}
-			return errors.Errorf(
-				"Timeout: waited too long for pod of deployment %v in namespace %v to be ready, error: %v",
-				gp.deployment.ObjectMeta.Name, gp.namespace, multierr)
+			return errors.Wrapf(multierr, "Timeout: waited too long for pod of deployment %v in namespace %v to be ready",
+				gp.deployment.ObjectMeta.Name, gp.namespace)
 		}
 		time.Sleep(1000 * time.Millisecond)
 	}
@@ -623,7 +627,7 @@ func (gp *GenericPool) createSvc(name string, labels map[string]string) (*apiv1.
 }
 
 func (gp *GenericPool) GetFuncSvc(ctx context.Context, m *metav1.ObjectMeta) (*fscache.FuncSvc, error) {
-	log.Printf("[%v] Choosing pod from pool", m.Name)
+	gp.logger.Info("choosing pod from pool", zap.String("function", m.Name))
 	newLabels := gp.labelsForFunction(m)
 
 	if gp.useIstio {
@@ -677,7 +681,7 @@ func (gp *GenericPool) GetFuncSvc(ctx context.Context, m *metav1.ObjectMeta) (*f
 		gp.scheduleDeletePod(pod.ObjectMeta.Name)
 		return nil, err
 	}
-	log.Printf("Specialized pod: %v", pod.ObjectMeta.Name)
+	gp.logger.Info("specialized pod", zap.String("pod", pod.ObjectMeta.Name), zap.String("function", m.Name))
 
 	var svcHost string
 	if gp.useSvc && !gp.useIstio {
@@ -694,7 +698,7 @@ func (gp *GenericPool) GetFuncSvc(ctx context.Context, m *metav1.ObjectMeta) (*f
 		}
 		if svc.ObjectMeta.Name != svcName {
 			gp.scheduleDeletePod(pod.ObjectMeta.Name)
-			return nil, errors.New(fmt.Sprintf("sanity check failed for svc %v", svc.ObjectMeta.Name))
+			return nil, errors.Errorf("sanity check failed for svc %v", svc.ObjectMeta.Name)
 		}
 
 		// the fission router isn't in the same namespace, so return a
@@ -704,7 +708,7 @@ func (gp *GenericPool) GetFuncSvc(ctx context.Context, m *metav1.ObjectMeta) (*f
 		svc := fission.GetFunctionIstioServiceName(m.Name, m.Namespace)
 		svcHost = fmt.Sprintf("%v.%v:8888", svc, gp.namespace)
 	} else {
-		log.Printf("Using pod IP for specialized pod")
+		gp.logger.Info("using pod IP for specialized pod", zap.String("pod", pod.ObjectMeta.Name), zap.String("function", m.Name))
 		svcHost = fmt.Sprintf("%v:8888", pod.Status.PodIP)
 	}
 
@@ -746,7 +750,10 @@ func (gp *GenericPool) destroy() error {
 	err := gp.kubernetesClient.ExtensionsV1beta1().
 		Deployments(gp.namespace).Delete(gp.deployment.ObjectMeta.Name, &delOpt)
 	if err != nil {
-		log.Printf("Error destroying deployment: %v", err)
+		gp.logger.Error("error destroying deployment",
+			zap.Error(err),
+			zap.String("deployment_name", gp.deployment.ObjectMeta.Name),
+			zap.String("deployment_namespace", gp.namespace))
 		return err
 	}
 	return nil

executor/poolmgr/gpm.go

@@ -18,12 +18,12 @@ package poolmgr
 
 import (
 	"context"
-	"log"
 	"os"
 	"strconv"
 	"strings"
 	"time"
 
+	"go.uber.org/zap"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
@@ -45,6 +45,8 @@ const (
 
 type (
 	GenericPoolManager struct {
+		logger *zap.Logger
+
 		pools            map[string]*GenericPool
 		kubernetesClient *kubernetes.Clientset
 		namespace        string
@@ -78,18 +80,22 @@ type (
 )
 
 func MakeGenericPoolManager(
+	logger *zap.Logger,
 	fissionClient *crd.FissionClient,
 	kubernetesClient *kubernetes.Clientset,
 	functionNamespace string,
 	instanceId string) *GenericPoolManager {
 
+	gpmLogger := logger.Named("generic_pool_manager")
+
 	gpm := &GenericPoolManager{
+		logger:            gpmLogger,
 		pools:             make(map[string]*GenericPool),
 		kubernetesClient:  kubernetesClient,
 		namespace:         functionNamespace,
 		fissionClient:     fissionClient,
 		functionEnv:       cache.MakeCache(10*time.Second, 0),
-		fsCache:           fscache.MakeFunctionServiceCache(),
+		fsCache:           fscache.MakeFunctionServiceCache(gpmLogger),
 		instanceId:        instanceId,
 		requestChannel:    make(chan *request),
 		idlePodReapTime:   2 * time.Minute,
@@ -101,7 +107,7 @@ func MakeGenericPoolManager(
 	if len(os.Getenv("ENABLE_ISTIO")) > 0 {
 		istio, err := strconv.ParseBool(os.Getenv("ENABLE_ISTIO"))
 		if err != nil {
-			log.Println("Failed to parse ENABLE_ISTIO")
+			gpmLogger.Info("failed to parse ENABLE_ISTIO")
 		}
 		gpm.enableIstio = istio
 	}
@@ -142,7 +148,7 @@ func (gpm *GenericPoolManager) service() {
 					ns = req.env.Metadata.Namespace
 				}
 
-				pool, err = MakeGenericPool(
+				pool, err = MakeGenericPool(gpm.logger,
 					gpm.fissionClient, gpm.kubernetesClient, req.env, poolsize,
 					ns, gpm.namespace, gpm.fsCache, gpm.instanceId, gpm.enableIstio,
 					gpm.collectorEndpoint)
@@ -163,7 +169,7 @@ func (gpm *GenericPoolManager) service() {
 				if !ok || poolsize == 0 {
 					// Env no longer exists or pool size changed to zero
 
-					log.Printf("Destroying generic pool for environment %v", pool.env.Metadata)
+					gpm.logger.Info("eestroying generic pool", zap.Any("environment", pool.env.Metadata))
 					delete(gpm.pools, key)
 
 					// and delete the pool asynchronously.
@@ -195,7 +201,7 @@ func (gpm *GenericPoolManager) CleanupPools(envs []crd.Environment) {
 
 func (gpm *GenericPoolManager) GetFuncSvc(ctx context.Context, metadata *metav1.ObjectMeta) (*fscache.FuncSvc, error) {
 	// from Func -> get Env
-	log.Printf("[%v] getting environment for function", metadata.Name)
+	gpm.logger.Info("getting environment for function", zap.String("function", metadata.Name))
 	env, err := gpm.getFunctionEnv(metadata)
 	if err != nil {
 		return nil, err
@@ -207,7 +213,7 @@ func (gpm *GenericPoolManager) GetFuncSvc(ctx context.Context, metadata *metav1.
 	}
 	// from GenericPool -> get one function container
 	// (this also adds to the cache)
-	log.Printf("[%v] getting function service from pool", metadata.Name)
+	gpm.logger.Info("getting function service from pool", zap.String("function", metadata.Name))
 	return pool.GetFuncSvc(ctx, metadata)
 }
 
@@ -228,7 +234,7 @@ func (gpm *GenericPoolManager) getFunctionEnv(m *metav1.ObjectMeta) (*crd.Enviro
 	}
 
 	// Get env from metadata
-	log.Printf("[%v] getting env", m)
+	gpm.logger.Info("getting env", zap.Any("function", m))
 	env, err = gpm.fissionClient.Environments(f.Spec.Environment.Namespace).Get(f.Spec.Environment.Name)
 	if err != nil {
 		return nil, err
@@ -247,11 +253,11 @@ func (gpm *GenericPoolManager) eagerPoolCreator() {
 		envs, err := gpm.fissionClient.Environments(metav1.NamespaceAll).List(metav1.ListOptions{})
 		if err != nil {
 			if fission.IsNetworkError(err) {
-				log.Printf("Encountered network error, retrying: %v", err)
+				gpm.logger.Error("encountered network error, retrying", zap.Error(err))
 				time.Sleep(5 * time.Second)
 				continue
 			}
-			log.Fatalf("Failed to get environment list: %v", err)
+			gpm.logger.Fatal("failed to get environment list", zap.Error(err))
 		}
 
 		// Create pools for all envs.  TODO: we should make this a bit less eager, only
@@ -264,7 +270,7 @@ func (gpm *GenericPoolManager) eagerPoolCreator() {
 			if gpm.getEnvPoolsize(&env) > 0 {
 				_, err := gpm.GetPool(&envs.Items[i])
 				if err != nil {
-					log.Printf("eager-create pool failed: %v", err)
+					gpm.logger.Error("eager-create pool failed", zap.Error(err))
 				}
 			}
 		}
@@ -292,7 +298,7 @@ func (gpm *GenericPoolManager) IsValid(fsvc *fscache.FuncSvc) bool {
 		if obj.Kind == "pod" {
 			pod, err := gpm.kubernetesClient.CoreV1().Pods(obj.Namespace).Get(obj.Name, metav1.GetOptions{})
 			if err == nil && strings.Contains(fsvc.Address, pod.Status.PodIP) && fission.IsReadyPod(pod) {
-				log.Printf("Valid pod address : %s", fsvc.Address)
+				gpm.logger.Info("valid pod address", zap.String("address", fsvc.Address))
 				return true
 			}
 		}
@@ -309,7 +315,7 @@ func (gpm *GenericPoolManager) idleObjectReaper() {
 
 		envs, err := gpm.fissionClient.Environments(metav1.NamespaceAll).List(metav1.ListOptions{})
 		if err != nil {
-			log.Fatalf("Failed to get environment list: %v", err)
+			gpm.logger.Fatal("failed to get environment list", zap.Error(err))
 		}
 
 		envList := make(map[types.UID]struct{})
@@ -319,7 +325,7 @@ func (gpm *GenericPoolManager) idleObjectReaper() {
 
 		funcSvcs, err := gpm.fsCache.ListOld(gpm.idlePodReapTime)
 		if err != nil {
-			log.Printf("Error reaping idle pods: %v", err)
+			gpm.logger.Error("error reaping idle pods", zap.Error(err))
 			continue
 		}
 
@@ -331,8 +337,9 @@ func (gpm *GenericPoolManager) idleObjectReaper() {
 			// For function with the environment that no longer exists, executor
 			// cleanups the idle pod as usual and prints log to notify user.
 			if _, ok := envList[fsvc.Environment.Metadata.UID]; !ok {
-				log.Printf("Environment %v for function %v no longer exists",
-					fsvc.Environment.Metadata.Name, fsvc.Name)
+				gpm.logger.Info("function environment no longer exists",
+					zap.String("environment", fsvc.Environment.Metadata.Name),
+					zap.String("function", fsvc.Name))
 			}
 
 			if fsvc.Environment.Spec.AllowedFunctionsPerContainer == fission.AllowedFunctionsPerContainerInfinite {
@@ -341,7 +348,9 @@ func (gpm *GenericPoolManager) idleObjectReaper() {
 
 			deleted, err := gpm.fsCache.DeleteOld(fsvc, gpm.idlePodReapTime)
 			if err != nil {
-				log.Printf("Error deleting Kubernetes objects for fsvc '%v': %v", fsvc, err)
+				gpm.logger.Error("error deleting Kubernetes objects for function service",
+					zap.Error(err),
+					zap.Any("service", fsvc))
 			}
 
 			if !deleted {
@@ -349,7 +358,7 @@ func (gpm *GenericPoolManager) idleObjectReaper() {
 			}
 
 			for _, kubeobj := range fsvc.KubernetesObjects {
-				reaper.CleanupKubeObject(gpm.kubernetesClient, &kubeobj)
+				reaper.CleanupKubeObject(gpm.logger, gpm.kubernetesClient, &kubeobj)
 			}
 		}
 	}

executor/poolmgr/packagewatcher.go

@@ -19,7 +19,7 @@ package poolmgr
 import (
 	"time"
 
-	log "github.com/sirupsen/logrus"
+	"go.uber.org/zap"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/client-go/kubernetes"
@@ -39,7 +39,9 @@ func (gpm *GenericPoolManager) makePkgController(fissionClient *crd.FissionClien
 		k8sCache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
 				pkg := obj.(*crd.Package)
-				log.Printf("List watch for package reported a new package addition: %s.%s", pkg.Metadata.Name, pkg.Metadata.Namespace)
+				gpm.logger.Info("list watch for package reported a new package addition",
+					zap.String("package_name", pkg.Metadata.Name),
+					zap.String("package_namepsace", pkg.Metadata.Namespace))
 
 				// create or update role-binding for fetcher sa in env ns to be able to get the pkg contents from pkg namespace
 				envNs := fissionfnNamespace
@@ -49,13 +51,21 @@ func (gpm *GenericPoolManager) makePkgController(fissionClient *crd.FissionClien
 
 				// here, we return if we hit an error during rolebinding setup. this is because this rolebinding is mandatory for
 				// every function's package to be loaded into its env. without that, there's no point to move forward.
-				err := fission.SetupRoleBinding(kubernetesClient, fission.PackageGetterRB, pkg.Metadata.Namespace, fission.PackageGetterCR, fission.ClusterRole, fission.FissionFetcherSA, envNs)
+				err := fission.SetupRoleBinding(gpm.logger, kubernetesClient, fission.PackageGetterRB, pkg.Metadata.Namespace, fission.PackageGetterCR, fission.ClusterRole, fission.FissionFetcherSA, envNs)
 				if err != nil {
-					log.Printf("Error creating %s for package: %s.%s, err: %v", fission.PackageGetterRB, pkg.Metadata.Name, pkg.Metadata.Namespace, err)
+					gpm.logger.Error("error creating rolebinding for package",
+						zap.Error(err),
+						zap.String("role_binding", fission.PackageGetterRB),
+						zap.String("package_name", pkg.Metadata.Name),
+						zap.String("package_namespace", pkg.Metadata.Namespace))
 					return
 				}
 
-				log.Printf("Successfully set up rolebinding for fetcher SA: %s.%s, in packages's ns : %s, for pkg : %s", fission.FissionFetcherSA, envNs, pkg.Metadata.Namespace, pkg.Metadata.Name)
+				gpm.logger.Info("successfully set up rolebinding for fetcher service account",
+					zap.String("service_account", fission.FissionFetcherSA),
+					zap.String("service_account_namespace", envNs),
+					zap.String("package_name", pkg.Metadata.Name),
+					zap.String("package_namespace", pkg.Metadata.Namespace))
 			},
 
 			UpdateFunc: func(oldObj, newObj interface{}) {
@@ -75,14 +85,23 @@ func (gpm *GenericPoolManager) makePkgController(fissionClient *crd.FissionClien
 						envNs = newPkg.Spec.Environment.Namespace
 					}
 
-					err := fission.SetupRoleBinding(kubernetesClient, fission.PackageGetterRB,
+					err := fission.SetupRoleBinding(gpm.logger, kubernetesClient, fission.PackageGetterRB,
 						newPkg.Metadata.Namespace, fission.PackageGetterCR, fission.ClusterRole,
 						fission.FissionFetcherSA, envNs)
 					if err != nil {
-						log.Printf("Error : %v updating %s for package: %s.%s", err, fission.PackageGetterRB, newPkg.Metadata.Name, newPkg.Metadata.Namespace)
+						gpm.logger.Error("error updating rolebinding for package",
+							zap.Error(err),
+							zap.String("role_binding", fission.PackageGetterRB),
+							zap.String("package_name", newPkg.Metadata.Name),
+							zap.String("package_namespace", newPkg.Metadata.Namespace))
 						return
 					}
-					log.Printf("Updated rolebinding for fetcher SA: %s.%s, in packages's ns : %s, for pkg : %s", fission.FissionFetcherSA, envNs, newPkg.Metadata.Namespace, newPkg.Metadata.Name)
+
+					gpm.logger.Info("successfully updated rolebinding for fetcher service account",
+						zap.String("service_account", fission.FissionFetcherSA),
+						zap.String("service_account_namespace", envNs),
+						zap.String("package_name", newPkg.Metadata.Name),
+						zap.String("package_namespace", newPkg.Metadata.Namespace))
 				}
 			},
 		})

executor/reaper/reaper.go

@@ -17,11 +17,10 @@ limitations under the License.
 package reaper
 
 import (
-	"fmt"
-	"log"
 	"strings"
 	"time"
 
+	"go.uber.org/zap"
 	apiv1 "k8s.io/api/core/v1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -36,24 +35,24 @@ var (
 )
 
 // CleanupOldExecutorObjects cleans up resources created by old executor instances
-func CleanupOldExecutorObjects(kubernetesClient *kubernetes.Clientset, instanceId string) {
+func CleanupOldExecutorObjects(logger *zap.Logger, kubernetesClient *kubernetes.Clientset, instanceId string) {
 	go func() {
-		err := cleanup(kubernetesClient, instanceId)
+		err := cleanup(logger, kubernetesClient, instanceId)
 		if err != nil {
 			// TODO retry reaper; logged and ignored for now
-			log.Printf("Failed to reaper: %v", err)
+			logger.Error("Failed to cleanup old executor objects", zap.Error(err))
 		}
 	}()
 }
 
-func cleanup(client *kubernetes.Clientset, instanceId string) error {
+func cleanup(logger *zap.Logger, client *kubernetes.Clientset, instanceId string) error {
 
-	err := cleanupServices(client, instanceId)
+	err := cleanupServices(logger, client, instanceId)
 	if err != nil {
 		return err
 	}
 
-	err = cleanupHpa(client, instanceId)
+	err = cleanupHpa(logger, client, instanceId)
 	if err != nil {
 		return err
 	}
@@ -61,7 +60,7 @@ func cleanup(client *kubernetes.Clientset, instanceId string) error {
 	// Deployments are used for idle pools and can be cleaned up
 	// immediately.  (We should "adopt" these instead of creating
 	// a new pool.)
-	err = cleanupDeployments(client, instanceId)
+	err = cleanupDeployments(logger, client, instanceId)
 	if err != nil {
 		return err
 	}
@@ -73,7 +72,7 @@ func cleanup(client *kubernetes.Clientset, instanceId string) error {
 	// time.
 	time.Sleep(6 * time.Minute)
 
-	err = cleanupPods(client, instanceId)
+	err = cleanupPods(logger, client, instanceId)
 	if err != nil {
 		return err
 	}
@@ -82,31 +81,39 @@ func cleanup(client *kubernetes.Clientset, instanceId string) error {
 }
 
 // CleanupKubeObject deletes given kubernetes object
-func CleanupKubeObject(kubeClient *kubernetes.Clientset, kubeobj *apiv1.ObjectReference) {
+func CleanupKubeObject(logger *zap.Logger, kubeClient *kubernetes.Clientset, kubeobj *apiv1.ObjectReference) {
 	switch strings.ToLower(kubeobj.Kind) {
 	case "pod":
 		err := kubeClient.CoreV1().Pods(kubeobj.Namespace).Delete(kubeobj.Name, nil)
-		logErr(fmt.Sprintf("cleaning up pod %v ", kubeobj.Name), err)
+		if err != nil {
+			logger.Error("error cleaning up pod", zap.Error(err), zap.String("pod", kubeobj.Name))
+		}
 
 	case "service":
 		err := kubeClient.CoreV1().Services(kubeobj.Namespace).Delete(kubeobj.Name, nil)
-		logErr(fmt.Sprintf("cleaning up service %v ", kubeobj.Name), err)
+		if err != nil {
+			logger.Error("error cleaning up service", zap.Error(err), zap.String("service", kubeobj.Name))
+		}
 
 	case "deployment":
 		err := kubeClient.ExtensionsV1beta1().Deployments(kubeobj.Namespace).Delete(kubeobj.Name, &delOpt)
-		logErr(fmt.Sprintf("cleaning up deployment %v ", kubeobj.Name), err)
+		if err != nil {
+			logger.Error("error cleaning up deployment", zap.Error(err), zap.String("deployment", kubeobj.Name))
+		}
 
 	case "horizontalpodautoscaler":
 		err := kubeClient.AutoscalingV1().HorizontalPodAutoscalers(kubeobj.Namespace).Delete(kubeobj.Name, nil)
-		logErr(fmt.Sprintf("cleaning up horizontalpodautoscaler %v ", kubeobj.Name), err)
+		if err != nil {
+			logger.Error("error cleaning up horizontalpodautoscaler", zap.Error(err), zap.String("horizontalpodautoscaler", kubeobj.Name))
+		}
 
 	default:
-		log.Printf("There was an error identifying the object type: %v for obj: %v", kubeobj.Kind, kubeobj)
+		logger.Error("Could not identifying the object type to clean up", zap.String("type", kubeobj.Kind), zap.Any("object", kubeobj))
 
 	}
 }
 
-func cleanupDeployments(client *kubernetes.Clientset, instanceId string) error {
+func cleanupDeployments(logger *zap.Logger, client *kubernetes.Clientset, instanceId string) error {
 	deploymentList, err := client.ExtensionsV1beta1().Deployments(meta_v1.NamespaceAll).List(meta_v1.ListOptions{})
 	if err != nil {
 		return err
@@ -114,24 +121,34 @@ func cleanupDeployments(client *kubernetes.Clientset, instanceId string) error {
 	for _, dep := range deploymentList.Items {
 		id, ok := dep.ObjectMeta.Labels[fission.EXECUTOR_INSTANCEID_LABEL]
 		if ok && id != instanceId {
-			log.Printf("Cleaning up deployment %v", dep.ObjectMeta.Name)
+			logger.Info("cleaning up deployment", zap.String("deployment", dep.ObjectMeta.Name))
 			err := client.ExtensionsV1beta1().Deployments(dep.ObjectMeta.Namespace).Delete(dep.ObjectMeta.Name, &delOpt)
-			logErr("cleaning up deployment", err)
+			if err != nil {
+				logger.Error("error cleaning up deployment",
+					zap.Error(err),
+					zap.String("deployment_name", dep.ObjectMeta.Name),
+					zap.String("deployment_namespace", dep.ObjectMeta.Namespace))
+			}
 			// ignore err
 		}
 		// Backward compatibility with older label name
 		pid, pok := dep.ObjectMeta.Labels[fission.POOLMGR_INSTANCEID_LABEL]
 		if pok && pid != instanceId {
-			log.Printf("Cleaning up deployment %v", dep.ObjectMeta.Name)
+			logger.Info("cleaning up deployment", zap.String("deployment", dep.ObjectMeta.Name))
 			err := client.ExtensionsV1beta1().Deployments(dep.ObjectMeta.Namespace).Delete(dep.ObjectMeta.Name, &delOpt)
-			logErr("cleaning up deployment", err)
+			if err != nil {
+				logger.Error("error cleaning up deployment",
+					zap.Error(err),
+					zap.String("deployment_name", dep.ObjectMeta.Name),
+					zap.String("deployment_namespace", dep.ObjectMeta.Namespace))
+			}
 			// ignore err
 		}
 	}
 	return nil
 }
 
-func cleanupPods(client *kubernetes.Clientset, instanceId string) error {
+func cleanupPods(logger *zap.Logger, client *kubernetes.Clientset, instanceId string) error {
 	podList, err := client.CoreV1().Pods(meta_v1.NamespaceAll).List(meta_v1.ListOptions{})
 	if err != nil {
 		return err
@@ -139,17 +156,27 @@ func cleanupPods(client *kubernetes.Clientset, instanceId string) error {
 	for _, pod := range podList.Items {
 		id, ok := pod.ObjectMeta.Labels[fission.EXECUTOR_INSTANCEID_LABEL]
 		if ok && id != instanceId {
-			log.Printf("Cleaning up pod %v", pod.ObjectMeta.Name)
+			logger.Info("cleaning up pod", zap.String("pod", pod.ObjectMeta.Name))
 			err := client.CoreV1().Pods(pod.ObjectMeta.Namespace).Delete(pod.ObjectMeta.Name, nil)
-			logErr("cleaning up pod", err)
+			if err != nil {
+				logger.Error("error cleaning up pod",
+					zap.Error(err),
+					zap.String("pod_name", pod.ObjectMeta.Name),
+					zap.String("pod_namespace", pod.ObjectMeta.Namespace))
+			}
 			// ignore err
 		}
 		// Backward compatibility with older label name
 		pid, pok := pod.ObjectMeta.Labels[fission.POOLMGR_INSTANCEID_LABEL]
 		if pok && pid != instanceId {
-			log.Printf("Cleaning up pod %v", pod.ObjectMeta.Name)
+			logger.Info("cleaning up pod", zap.String("pod", pod.ObjectMeta.Name))
 			err := client.CoreV1().Pods(pod.ObjectMeta.Namespace).Delete(pod.ObjectMeta.Name, nil)
-			logErr("cleaning up pod", err)
+			if err != nil {
+				logger.Error("error cleaning up pod",
+					zap.Error(err),
+					zap.String("pod_name", pod.ObjectMeta.Name),
+					zap.String("pod_namespace", pod.ObjectMeta.Namespace))
+			}
 			// ignore err
 		}
 
@@ -157,7 +184,7 @@ func cleanupPods(client *kubernetes.Clientset, instanceId string) error {
 	return nil
 }
 
-func cleanupServices(client *kubernetes.Clientset, instanceId string) error {
+func cleanupServices(logger *zap.Logger, client *kubernetes.Clientset, instanceId string) error {
 	svcList, err := client.CoreV1().Services(meta_v1.NamespaceAll).List(meta_v1.ListOptions{})
 	if err != nil {
 		return err
@@ -165,16 +192,21 @@ func cleanupServices(client *kubernetes.Clientset, instanceId string) error {
 	for _, svc := range svcList.Items {
 		id, ok := svc.ObjectMeta.Labels[fission.EXECUTOR_INSTANCEID_LABEL]
 		if ok && id != instanceId {
-			log.Printf("Cleaning up svc %v", svc.ObjectMeta.Name)
+			logger.Info("cleaning up service", zap.String("service", svc.ObjectMeta.Name))
 			err := client.CoreV1().Services(svc.ObjectMeta.Namespace).Delete(svc.ObjectMeta.Name, nil)
-			logErr("cleaning up service", err)
+			if err != nil {
+				logger.Error("error cleaning up service",
+					zap.Error(err),
+					zap.String("service_name", svc.ObjectMeta.Name),
+					zap.String("service_namespace", svc.ObjectMeta.Namespace))
+			}
 			// ignore err
 		}
 	}
 	return nil
 }
 
-func cleanupHpa(client *kubernetes.Clientset, instanceId string) error {
+func cleanupHpa(logger *zap.Logger, client *kubernetes.Clientset, instanceId string) error {
 	hpaList, err := client.AutoscalingV1().HorizontalPodAutoscalers(meta_v1.NamespaceAll).List(meta_v1.ListOptions{})
 	if err != nil {
 		return err
@@ -183,9 +215,15 @@ func cleanupHpa(client *kubernetes.Clientset, instanceId string) error {
 	for _, hpa := range hpaList.Items {
 		id, ok := hpa.ObjectMeta.Labels[fission.EXECUTOR_INSTANCEID_LABEL]
 		if ok && id != instanceId {
-			log.Printf("Cleaning up HPA %v", hpa.ObjectMeta.Name)
+			logger.Info("cleaning up HPA", zap.String("hpa", hpa.ObjectMeta.Name))
 			err := client.AutoscalingV1().HorizontalPodAutoscalers(hpa.ObjectMeta.Namespace).Delete(hpa.ObjectMeta.Name, nil)
-			logErr("cleaning up HPA", err)
+			if err != nil {
+				logger.Error("error cleaning up HPA",
+					zap.Error(err),
+					zap.String("hpa_name", hpa.ObjectMeta.Name),
+					zap.String("hpa_namespace", hpa.ObjectMeta.Namespace))
+			}
+			// ignore err
 		}
 
 	}
@@ -193,22 +231,16 @@ func cleanupHpa(client *kubernetes.Clientset, instanceId string) error {
 
 }
 
-func logErr(msg string, err error) {
-	if err != nil {
-		log.Printf("Error %v: %v", msg, err)
-	}
-}
-
 // CleanupRoleBindings periodically lists rolebindings across all namespaces and removes Service Accounts from them or
 // deletes the rolebindings completely if there are no Service Accounts in a rolebinding object.
-func CleanupRoleBindings(client *kubernetes.Clientset, fissionClient *crd.FissionClient, functionNs, envBuilderNs string, cleanupRoleBindingInterval time.Duration) {
+func CleanupRoleBindings(logger *zap.Logger, client *kubernetes.Clientset, fissionClient *crd.FissionClient, functionNs, envBuilderNs string, cleanupRoleBindingInterval time.Duration) {
 	for {
-		log.Println("Starting cleanupRoleBindings cycle")
+		logger.Info("starting cleanupRoleBindings cycle")
 		// get all rolebindings ( just to be efficient, one call to kubernetes )
 		rbList, err := client.RbacV1beta1().RoleBindings(meta_v1.NamespaceAll).List(meta_v1.ListOptions{})
 		if err != nil {
 			// something wrong, but next iteration hopefully succeeds
-			log.Printf("Error listing rolebindings in all ns, err: %v", err)
+			logger.Error("error listing role bindings in all namespaces", zap.Error(err))
 			continue
 		}
 
@@ -228,7 +260,7 @@ func CleanupRoleBindings(client *kubernetes.Clientset, fissionClient *crd.Fissio
 			// we can list the functions once per role-binding.
 			funcList, err := fissionClient.Functions(roleBinding.Namespace).List(meta_v1.ListOptions{})
 			if err != nil {
-				log.Printf("Error fetching environment list in : %s", roleBinding.Namespace)
+				logger.Error("error fetching environment list in namespace", zap.Error(err), zap.String("namespace", roleBinding.Namespace))
 				continue
 			}
 
@@ -275,7 +307,7 @@ func CleanupRoleBindings(client *kubernetes.Clientset, fissionClient *crd.Fissio
 					// check if there is an env obj in saNs
 					envList, err := fissionClient.Environments(saNs).List(meta_v1.ListOptions{})
 					if err != nil {
-						log.Printf("Error fetching environment list in : %s", saNs)
+						logger.Error("error fetching environment list in service account namespace", zap.Error(err), zap.String("namespace", saNs))
 						continue
 					}
 
@@ -313,15 +345,21 @@ func CleanupRoleBindings(client *kubernetes.Clientset, fissionClient *crd.Fissio
 			// finally, make a call to RemoveSAFromRoleBindingWithRetries for all the service accounts that need to be removed
 			// for the role-binding in this iteration
 			if len(saToRemove) != 0 {
-				log.Printf("saToRemove : %v for rolebinding : %s.%s", saToRemove, roleBinding.Name, roleBinding.Namespace)
+				logger.Info("removing service accounts from role binding",
+					zap.Any("service_accounts", saToRemove),
+					zap.String("role_binding_name", roleBinding.Name),
+					zap.String("role_binding_namespace", roleBinding.Namespace))
 
 				// call this once in the end for each role-binding
-				err = fission.RemoveSAFromRoleBindingWithRetries(client, roleBinding.Name, roleBinding.Namespace, saToRemove)
+				err = fission.RemoveSAFromRoleBindingWithRetries(logger, client, roleBinding.Name, roleBinding.Namespace, saToRemove)
 				if err != nil {
 					// if there's an error, we just log it and proceed with the next role-binding, hoping that this role-binding
 					// will be processed in next iteration.
-					log.Printf("Error removing SA : %v from rolebinding : %s.%s, err: %v", saToRemove, roleBinding.Name,
-						roleBinding.Namespace, err)
+					logger.Info("error removing service account from role binding",
+						zap.Error(err),
+						zap.Any("service_accounts", saToRemove),
+						zap.String("role_binding_name", roleBinding.Name),
+						zap.String("role_binding_namespace", roleBinding.Namespace))
 				}
 			}
 		}