KOGITO-8276: Improve the logging adding missing levels (#201)

* KOGITO-8276: Improve the logging adding missing levels

* Add support for -zap-log-level CLI flag

* Fix function invocation.

* Updates following peer review.

* Updates following peer review x2. Remove use of logger argument.

* Remove klog.InitFlags(nil) in Logger wrappers.

* Run make generate-all

* Another refactor removing wrappers and contexts.

* More updates.

* Enable Info logging by default.

* Add missing klog dependency.

* Fix erroneous logging statements.

* Move to structured logging.

* Remove references to k8s.io/klog[v1]

* Fix build

* Fix go.sum

* Update container-builder/builder/kubernetes/builder.go

Co-authored-by: Filippe Spolti <[email protected]>

* Update container-builder/builder/kubernetes/builder.go

Co-authored-by: Filippe Spolti <[email protected]>

* Update utils/kubernetes/annotations.go

Co-authored-by: Filippe Spolti <[email protected]>

* Update controllers/builder/containerbuilder.go

Co-authored-by: Filippe Spolti <[email protected]>

* Update container-builder/client/fastmapper.go

* Update container-builder/common/registry_docker.go

Co-authored-by: Massimiliano Dessì - (Fast Chauffeur) <[email protected]>

* Update controllers/platform/platformutils.go

Co-authored-by: Massimiliano Dessì - (Fast Chauffeur) <[email protected]>

* Update controllers/platform/platformutils.go

Co-authored-by: Massimiliano Dessì - (Fast Chauffeur) <[email protected]>

* Update controllers/platform/platformutils.go

Co-authored-by: Massimiliano Dessì - (Fast Chauffeur) <[email protected]>

---------

Co-authored-by: Michael Anstis <[email protected]>
Co-authored-by: Filippe Spolti <[email protected]>
Co-authored-by: Massimiliano Dessì - (Fast Chauffeur) <[email protected]>

api/const.go

@@ -0,0 +1,22 @@
+/*
+ * Copyright 2023 Red Hat, Inc. and/or its affiliates.
+ *
+ * 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 api
+
+const (
+	// ComponentName just a name to identify this package/component/application
+	ComponentName = "sonataflow"
+)

api/go.mod

@@ -70,7 +70,7 @@ require (
 	k8s.io/apiextensions-apiserver v0.27.2 // indirect
 	k8s.io/client-go v0.27.2 // indirect
 	k8s.io/component-base v0.27.2 // indirect
-	k8s.io/klog/v2 v2.90.1 // indirect
+	k8s.io/klog/v2 v2.100.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect
 	k8s.io/utils v0.0.0-20230313181309-38a27ef9d749 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

api/go.sum

@@ -271,8 +271,7 @@ k8s.io/client-go v0.27.2 h1:vDLSeuYvCHKeoQRhCXjxXO45nHVv2Ip4Fe0MfioMrhE=
 k8s.io/client-go v0.27.2/go.mod h1:tY0gVmUsHrAmjzHX9zs7eCjxcBsf8IiNe7KQ52biTcQ=
 k8s.io/component-base v0.27.2 h1:neju+7s/r5O4x4/txeUONNTS9r1HsPbyoPBAtHsDCpo=
 k8s.io/component-base v0.27.2/go.mod h1:5UPk7EjfgrfgRIuDBFtsEFAe4DAvP3U+M8RTzoSJkpo=
-k8s.io/klog/v2 v2.90.1 h1:m4bYOKall2MmOiRaR1J+We67Do7vm9KiQVlT96lnHUw=
-k8s.io/klog/v2 v2.90.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
+k8s.io/klog/v2 v2.100.1 h1:7WCHKK6K8fNhTqfBhISHQ97KrnJNFZMcQvKp7gP/tmg=
 k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f h1:2kWPakN3i/k81b0gvD5C5FJ2kxm1WrQFanWchyKuqGg=
 k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f/go.mod h1:byini6yhqGC14c3ebc/QwanvYwhuMWF6yz2F8uwW8eg=
 k8s.io/utils v0.0.0-20230313181309-38a27ef9d749 h1:xMMXJlJbsU8w3V5N2FLDQ8YgU8s1EoULdbQBcAeNJkY=

config/manager/manager.yaml

@@ -33,6 +33,7 @@ spec:
         - /usr/local/bin/manager
         args:
         - --leader-elect
+        - '-v=2'
         image: controller:latest
         name: manager
         securityContext:

container-builder/util/const.go → container-builder/api/const.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 Red Hat, Inc. and/or its affiliates.
+ * Copyright 2023 Red Hat, Inc. and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package util
+package api
 
 const (
 	// ComponentName just a name to identify this package/component/application

container-builder/builder/kaniko_docker_integration_test.go

@@ -22,11 +22,12 @@ import (
 	"testing"
 	"time"
 
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
+	"k8s.io/klog/v2"
 
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/common"
+	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
 )
 
 func TestKanikoTestSuite(t *testing.T) {
@@ -39,7 +40,7 @@ func (suite *KanikoDockerTestSuite) TestKanikoBuild() {
 	//registry, err, repos := checkEmptyDockerRegistry(suite)
 	mydir, err := os.Getwd()
 	if err != nil {
-		logrus.Error(err)
+		klog.V(log.E).ErrorS(err, "error getting working directory.")
 	}
 	dockefileDir := mydir + "/../examples/dockerfiles"
 	assert.Nil(suite.T(), suite.Docker.PullImage("gcr.io/kaniko-project/executor:latest"), "Pull image failed")
@@ -52,11 +53,11 @@ func (suite *KanikoDockerTestSuite) TestKanikoBuild() {
 		RegistryFinalImageName: imageName,
 		ReadBuildOutput:        false,
 	}
-	logrus.Infof("Start Kaniko build")
+	klog.V(log.I).InfoS("Start Kaniko build")
 	start := time.Now()
 	imageID, error := KanikoBuild(suite.Docker.Connection, config)
 	timeElapsed := time.Since(start)
-	logrus.Infof("The Kaniko build took %s", timeElapsed)
+	klog.V(log.I).InfoS("The Kaniko build took", "duration", timeElapsed)
 	assert.Nil(suite.T(), error, "ContainerBuild failed")
 	assert.NotNil(suite.T(), imageID, error, "ContainerBuild failed")
 	//@TODO investigate when the code will be in the mono repo
@@ -74,7 +75,7 @@ func checkEmptyDockerRegistry(suite *KanikoDockerTestSuite) (common.RegistryCont
 	assert.Truef(suite.T(), suite.RegistryID != "", "Registry not started")
 	registry, err := common.GetRegistryContainer()
 	if err != nil {
-		logrus.Error("registry not found")
+		klog.V(log.E).ErrorS(err, "registry not found")
 	}
 	repos, _ := registry.GetRepositories()
 	assert.True(suite.T(), len(repos) == 0)

container-builder/builder/kaniko_integration_test_suite.go

@@ -20,11 +20,12 @@ package builder
 import (
 	"time"
 
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
+	"k8s.io/klog/v2"
 
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/common"
+	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
 )
 
 type KanikoDockerTestSuite struct {
@@ -46,7 +47,7 @@ func (suite *KanikoDockerTestSuite) SetupSuite() {
 
 	pullErr := suite.Docker.PullImage(EXECUTOR_IMAGE)
 	if pullErr != nil {
-		logrus.Infof("Pull Kaniko executor Error:%s", pullErr)
+		klog.V(log.E).ErrorS(pullErr, "Pull Kaniko executor")
 	}
 	time.Sleep(4 * time.Second) // Needed on CI
 }
@@ -59,14 +60,14 @@ func (suite *KanikoDockerTestSuite) TearDownSuite() {
 		suite.LocalRegistry.StopRegistry()
 	}
 	purged, err := suite.Docker.PurgeContainer("", common.REGISTRY_IMG)
-	logrus.Infof("Purged containers %t", purged)
+	klog.V(log.I).InfoS("Purged containers", "containers", purged)
 	if err != nil {
-		logrus.Infof("Purged registry err %t", err)
+		klog.V(log.E).ErrorS(err, "Purged registry")
 	}
 
 	purgedBuild, err := suite.Docker.PurgeContainer("", EXECUTOR_IMAGE)
 	if err != nil {
-		logrus.Infof("Purged container err %t", err)
+		klog.V(log.E).ErrorS(err, "Purged container")
 	}
-	logrus.Infof("Purged container build %t", purgedBuild)
+	klog.V(log.I).InfoS("Purged container build", "container", purgedBuild)
 }

container-builder/builder/kaniko_vanilla.go

@@ -19,11 +19,14 @@ import (
 	"context"
 	"os"
 
+	"k8s.io/klog/v2"
+
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/stdcopy"
-	"github.com/sirupsen/logrus"
+
+	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
 )
 
 type KanikoVanillaConfig struct {
@@ -62,28 +65,25 @@ func KanikoBuild(connection *client.Client, config KanikoVanillaConfig) (string,
 	}, hostConfig, nil, nil, config.ContainerName)
 
 	if err != nil {
-		logrus.Error(err)
+		klog.V(log.E).ErrorS(err, "error during KanikoBuild, ContainerCreate")
 	}
 
 	if err := connection.ContainerStart(ctx, resp.ID, types.ContainerStartOptions{}); err != nil {
-		logrus.Error(err)
-	}
-	if err != nil {
-		logrus.Error(err)
+		klog.V(log.E).ErrorS(err, "error during KanikoBuild, ContainerStart")
 	}
 
 	statusCh, errCh := connection.ContainerWait(ctx, resp.ID, container.WaitConditionNotRunning)
 	select {
 	case err := <-errCh:
 		if err != nil {
-			logrus.Error(err)
+			klog.V(log.E).ErrorS(err, "error during KanikoBuild, ContainerWait")
 		}
 	case <-statusCh:
 	}
 
 	out, err := connection.ContainerLogs(ctx, resp.ID, types.ContainerLogsOptions{ShowStdout: true})
 	if err != nil {
-		logrus.Error(err)
+		klog.V(log.E).ErrorS(err, "error during KanikoBuild, ContainerLogs")
 	}
 	if config.ReadBuildOutput {
 		stdcopy.StdCopy(os.Stdout, os.Stderr, out)

container-builder/builder/kubernetes/action.go

@@ -19,11 +19,9 @@ import (
 
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/api"
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/client"
-	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
 )
 
 type Action interface {
-	log.Injectable
 	client.Injectable
 	// Name returns user-friendly name for the action
 	Name() string
@@ -37,15 +35,10 @@ type Action interface {
 
 type baseAction struct {
 	client client.Client
-	L      log.Logger
 }
 
 // TODO: implement our client wrapper
 
 func (action *baseAction) InjectClient(client client.Client) {
 	action.client = client
 }
-
-func (action *baseAction) InjectLogger(log log.Logger) {
-	action.L = log
-}

container-builder/builder/kubernetes/builder.go

@@ -18,11 +18,12 @@ import (
 	"context"
 	"fmt"
 
+	"k8s.io/klog/v2"
+
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/api"
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/client"
-	"github.com/kiegroup/kogito-serverless-operator/container-builder/util"
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
 )
 
@@ -49,7 +50,6 @@ type containerBuildContext struct {
 }
 
 type builder struct {
-	L       log.Logger
 	Context containerBuildContext
 }
 
@@ -94,7 +94,6 @@ type schedulerHandler interface {
 
 func FromBuild(build *api.ContainerBuild) ContainerBuilder {
 	return &builder{
-		L: log.WithName(util.ComponentName),
 		Context: containerBuildContext{
 			ContainerBuild: build,
 			C:              context.TODO(),
@@ -174,20 +173,19 @@ func (b *builder) Reconcile() (*api.ContainerBuild, error) {
 	target := b.Context.ContainerBuild.DeepCopy()
 
 	for _, a := range actions {
-		a.InjectLogger(b.L)
 		a.InjectClient(b.Context.Client)
 
 		if a.CanHandle(target) {
-			b.L.Infof("Invoking action %s", a.Name())
+			klog.V(log.I).InfoS("Invoking action", "buildAction", a.Name())
 			newTarget, err := a.Handle(b.Context.C, target)
 			if err != nil {
-				b.L.Errorf(err, "Failed to invoke action %s", a.Name())
+				klog.V(log.E).ErrorS(err, "Failed to invoke action", "buildAction", a.Name())
 				return nil, err
 			}
 
 			if newTarget != nil {
 				if newTarget.Status.Phase != target.Status.Phase {
-					b.L.Info(
+					klog.V(log.I).InfoS(
 						"state transition",
 						"phase-from", target.Status.Phase,
 						"phase-to", newTarget.Status.Phase,

container-builder/builder/kubernetes/builder_kaniko.go

@@ -20,8 +20,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/api"
-	"github.com/kiegroup/kogito-serverless-operator/container-builder/util"
-	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
 )
 
 type kanikoScheduler struct {
@@ -60,7 +58,6 @@ func (k kanikoSchedulerHandler) CreateScheduler(info ContainerBuilderInfo, build
 	sched := &kanikoScheduler{
 		&scheduler{
 			builder: builder{
-				L:       log.WithName(util.ComponentName),
 				Context: buildCtx,
 			},
 			Resources: make([]resource, 0),

container-builder/builder/kubernetes/recovery.go

@@ -20,6 +20,10 @@ import (
 	"context"
 	"time"
 
+	"k8s.io/klog/v2"
+
+	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
+
 	"github.com/jpillora/backoff"
 
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/api"
@@ -86,9 +90,9 @@ func (action *errorRecoveryAction) Handle(ctx context.Context, build *api.Contai
 	build.Status.Failure.Recovery.Attempt++
 	build.Status.Failure.Recovery.AttemptTime = metav1.Now()
 
-	action.L.Infof("Recovery attempt (%d/%d)",
-		build.Status.Failure.Recovery.Attempt,
-		build.Status.Failure.Recovery.AttemptMax,
+	klog.V(log.I).InfoS("Recovery attempt",
+		"attempt", build.Status.Failure.Recovery.Attempt,
+		"attemptMax", build.Status.Failure.Recovery.AttemptMax,
 	)
 
 	return build, nil

container-builder/cleaner/docker_integration_test.go

@@ -22,11 +22,12 @@ import (
 	"testing"
 	"time"
 
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
+	"k8s.io/klog/v2"
 
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/common"
+	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
 )
 
 func TestDockerIntegrationTestSuite(t *testing.T) {
@@ -43,21 +44,21 @@ func (suite *DockerTestSuite) TestImagesOperationsOnDockerRegistryForTest() {
 
 	pullErr := suite.Docker.PullImage(common.TEST_IMG + ":" + common.LATEST_TAG)
 	if pullErr != nil {
-		logrus.Infof("Pull Error:%s", pullErr)
+		klog.V(log.E).ErrorS(pullErr, "Pull Error")
 	}
 	assert.Nil(suite.T(), pullErr, "Pull image failed")
 	time.Sleep(2 * time.Second) // Needed on CI
 	assert.True(suite.T(), suite.LocalRegistry.IsImagePresent(common.TEST_IMG), "Test image not found in the registry after the pull")
 	tagErr := suite.Docker.TagImage(common.TEST_IMG, common.TEST_IMG_LOCAL_TAG)
 	if tagErr != nil {
-		logrus.Infof("Tag Error:%s", tagErr)
+		klog.V(log.E).ErrorS(tagErr, "Tag Error")
 	}
 
 	assert.Nil(suite.T(), tagErr, "Tag image failed")
 	time.Sleep(2 * time.Second) // Needed on CI
 	pushErr := suite.Docker.PushImage(common.TEST_IMG_LOCAL_TAG, common.REGISTRY_CONTAINER_URL_FROM_DOCKER_SOCKET, "", "")
 	if pushErr != nil {
-		logrus.Infof("Push Error:%s", pushErr)
+		klog.V(log.E).ErrorS(pushErr, "Push Error")
 	}
 
 	assert.Nil(suite.T(), pushErr, "Push image in the Docker container failed")

container-builder/cleaner/docker_integration_test_suite.go

@@ -18,11 +18,12 @@
 package cleaner
 
 import (
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
+	"k8s.io/klog/v2"
 
 	"github.com/kiegroup/kogito-serverless-operator/container-builder/common"
+	"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
 )
 
 type DockerTestSuite struct {
@@ -52,7 +53,7 @@ func (suite *DockerTestSuite) TearDownSuite() {
 	}
 	purged, err := suite.Docker.PurgeContainer("", common.REGISTRY_IMG)
 	if err != nil {
-		logrus.Errorf("Error during purged container in TearDown Suite %t", err)
+		klog.V(log.E).ErrorS(err, "Error during purged container in TearDown Suite.")
 	}
-	logrus.Infof("Purged containers %t", purged)
+	klog.V(log.I).InfoS("Purged container", "containers", purged)
 }