MESH-6122: Remove redundant logging statements across the codebase (#…

…866)

* MESH-6122: Remove redundant logging statements across the codebase

* MESH-6122: Add error logging for failed sidecar updates in serviceentry.go

admiral/pkg/clusters/admiralIgnoreIdentityStateChecker.go

@@ -87,7 +87,6 @@ func (checker *ignoreIdentityStateChecker) shouldRunOnIndependentGoRoutine() boo
 
 func (checker *ignoreIdentityStateChecker) runStateCheck(ctx context.Context) {
 	period := time.Duration(checker.admiralConfig.IgnoreIdentityList.StateCheckerPeriodInSeconds) * time.Second
-	log.Infof("op=ignoreIdentityStateChecker message=starting ticker to fetch ignore list every %v seconds", period)
 	ticker := time.NewTicker(period)
 	defer ticker.Stop()
 	state, err := checker.getState()
@@ -99,7 +98,6 @@ func (checker *ignoreIdentityStateChecker) runStateCheck(ctx context.Context) {
 	for {
 		select {
 		case <-ctx.Done():
-			log.Infof("op=ignoreIdentityStateChecker message=context done stopping ticker")
 			return
 		case <-ticker.C:
 			state, err := checker.getState()

admiral/pkg/clusters/admiralReadWriteLeaseStateChecker.go

@@ -110,7 +110,6 @@ func ExecuteStateCheck(ctx context.Context, dynamoDBConfig DynamoDBConfig, dynam
 		dynamodbClient.updatedReadWriteLease(readWriteLease, dynamoDBConfig.TableName)
 		//Not updating read-write mode until we confirm this pod has the lease
 	} else if skipLeaseCheckPodName == readWriteLease.LeaseOwner {
-		log.Info("DynamoDR: Lease held by skip lease check pod. Setting Admiral to read only mode")
 		commonUtil.CurrentAdmiralState.ReadOnly = ReadOnlyEnabled
 		commonUtil.CurrentAdmiralState.IsStateInitialized = StateInitialized
 	} else if remoteRegistry != nil && podIdentifier == readWriteLease.LeaseOwner && IsCacheWarmupTime(remoteRegistry) {

admiral/pkg/clusters/admiralStateChecker.go

@@ -21,12 +21,9 @@ Utility function to start Admiral DR checks.
 DR checks can be run either on the main go routine or a new go routine
 */
 func RunAdmiralStateCheck(ctx context.Context, stateChecker string, asc AdmiralStateChecker) {
-	log.Infof("Starting %s state checker", stateChecker)
 	if asc.shouldRunOnIndependentGoRoutine() {
-		log.Infof("Starting %s state checker on a new Go Routine", stateChecker)
 		go asc.runStateCheck(ctx)
 	} else {
-		log.Infof("Starting %s state checker on existing Go Routine", stateChecker)
 		asc.runStateCheck(ctx)
 	}
 }

admiral/pkg/clusters/clientconnectionconfig_handler.go

@@ -9,7 +9,6 @@ import (
 	v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1alpha1"
 	"github.com/istio-ecosystem/admiral/admiral/pkg/controller/admiral"
 	"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
-	log "github.com/sirupsen/logrus"
 )
 
 type ClientConnectionConfigHandler struct {
@@ -52,10 +51,6 @@ func (c *clientConnectionSettingsCache) Put(clientConnectionSettings *v1.ClientC
 	var clientConnectionSettingsIdentity = common.GetClientConnectionConfigIdentity(clientConnectionSettings)
 	var clientConnectionSettingsEnv = common.GetClientConnectionConfigEnv(clientConnectionSettings)
 
-	log.Infof(
-		"adding clientConnectionSettings with name %v to clientConnectionSettingsCache. LabelMatch=%v env=%v",
-		clientConnectionSettings.Name, clientConnectionSettingsIdentity, clientConnectionSettingsEnv)
-
 	key := common.ConstructKeyWithEnvAndIdentity(clientConnectionSettingsEnv, clientConnectionSettingsIdentity)
 	c.identityCache[key] = clientConnectionSettings
 	return nil
@@ -66,7 +61,6 @@ func (c *clientConnectionSettingsCache) Delete(identity string, environment stri
 	defer c.mutex.Unlock()
 	key := common.ConstructKeyWithEnvAndIdentity(environment, identity)
 	if _, ok := c.identityCache[key]; ok {
-		log.Infof("deleting clientConnectionSettings with key=%s from clientConnectionSettingsCache", key)
 		delete(c.identityCache, key)
 		return nil
 	}
@@ -75,8 +69,6 @@ func (c *clientConnectionSettingsCache) Delete(identity string, environment stri
 
 func (c *ClientConnectionConfigHandler) Added(ctx context.Context,
 	clientConnectionSettings *v1.ClientConnectionConfig) error {
-	log.Infof(
-		LogFormat, common.Add, common.ClientConnectionConfig, clientConnectionSettings.Name, c.ClusterID, "received")
 	err := HandleEventForClientConnectionConfig(
 		ctx, admiral.Add, clientConnectionSettings, c.RemoteRegistry, c.ClusterID, modifyServiceEntryForNewServiceOrPod)
 	if err != nil {
@@ -88,8 +80,6 @@ func (c *ClientConnectionConfigHandler) Added(ctx context.Context,
 
 func (c *ClientConnectionConfigHandler) Updated(
 	ctx context.Context, clientConnectionSettings *v1.ClientConnectionConfig) error {
-	log.Infof(
-		LogFormat, common.Update, common.ClientConnectionConfig, clientConnectionSettings.Name, c.ClusterID, common.ReceivedStatus)
 	err := HandleEventForClientConnectionConfig(
 		ctx, admiral.Update, clientConnectionSettings, c.RemoteRegistry, c.ClusterID, modifyServiceEntryForNewServiceOrPod)
 	if err != nil {
@@ -101,8 +91,6 @@ func (c *ClientConnectionConfigHandler) Updated(
 
 func (c *ClientConnectionConfigHandler) Deleted(
 	ctx context.Context, clientConnectionSettings *v1.ClientConnectionConfig) error {
-	log.Infof(
-		LogFormat, common.Delete, common.ClientConnectionConfig, clientConnectionSettings.Name, c.ClusterID, common.ReceivedStatus)
 	err := HandleEventForClientConnectionConfig(
 		ctx, admiral.Update, clientConnectionSettings, c.RemoteRegistry, c.ClusterID, modifyServiceEntryForNewServiceOrPod)
 	if err != nil {

admiral/pkg/clusters/clientdiscovery_handler.go

@@ -24,14 +24,11 @@ func (cdh *ClientDiscoveryHandler) Added(ctx context.Context, obj *common.K8sObj
 
 func HandleEventForClientDiscovery(ctx context.Context, event admiral.EventType, obj *common.K8sObject,
 	remoteRegistry *RemoteRegistry, clusterName string) error {
-	log.Infof(LogFormat, event, obj.Type, obj.Name, clusterName, common.ReceivedStatus)
 	globalIdentifier := common.GetGlobalIdentifier(obj.Annotations, obj.Labels)
 	originalIdentifier := common.GetOriginalIdentifier(obj.Annotations, obj.Labels)
 	if len(globalIdentifier) == 0 {
-		log.Infof(LogFormat, event, obj.Type, obj.Name, clusterName, "Skipped as '"+common.GetWorkloadIdentifier()+" was not found', namespace="+obj.Namespace)
 		return nil
 	}
-	ctxLogger := common.GetCtxLogger(ctx, globalIdentifier, "")
 
 	ctx = context.WithValue(ctx, "clusterName", clusterName)
 	ctx = context.WithValue(ctx, "eventResourceType", obj.Type)
@@ -54,26 +51,22 @@ func HandleEventForClientDiscovery(ctx context.Context, event admiral.EventType,
 	}
 
 	if commonUtil.IsAdmiralReadOnly() {
-		ctxLogger.Infof(common.CtxLogFormat, event, "", "", "", "processing skipped as Admiral is in Read-only mode")
 		return nil
 	}
 
 	if IsCacheWarmupTime(remoteRegistry) {
-		ctxLogger.Infof(common.CtxLogFormat, event, "", "", "", "processing skipped during cache warm up state")
 		return fmt.Errorf(common.CtxLogFormat, event, obj.Name, obj.Namespace, clusterName, "processing skipped during cache warm up state for env="+" identity="+globalIdentifier)
 	}
 
 	//if we have a deployment/rollout in this namespace skip processing to save some cycles
 	if DeploymentOrRolloutExistsInNamespace(remoteRegistry, globalIdentifier, clusterName, obj.Namespace) {
-		log.Infof(LogFormatAdv, "Process", obj.Type, obj.Name, obj.Namespace, clusterName, "Skipping client discovery as Deployment/Rollout already present in namespace for client="+globalIdentifier)
 		return nil
 	}
 
 	//write SEs required for this client
 	depRecord := remoteRegistry.DependencyController.Cache.Get(globalIdentifier)
 
 	if depRecord == nil {
-		log.Warnf(LogFormatAdv, "Process", obj.Type, obj.Name, obj.Namespace, clusterName, "Skipping client discovery as no dependency record found for client="+globalIdentifier)
 		return nil
 	}
 	err := remoteRegistry.DependencyController.DepHandler.Added(ctx, depRecord)

admiral/pkg/clusters/dependency_handler.go

@@ -47,7 +47,6 @@ func (dh *DependencyHandler) HandleDependencyRecord(ctx context.Context, obj *v1
 	remoteRegistry *RemoteRegistry, eventType admiral.EventType) error {
 	sourceIdentity := obj.Spec.Source
 	if len(sourceIdentity) == 0 {
-		log.Infof(LogFormat, string(eventType), common.DependencyResourceType, obj.Name, "", "No identity found namespace="+obj.Namespace)
 		return nil
 	}
 
@@ -117,7 +116,6 @@ func (d *ProcessDestinationService) Process(ctx context.Context, dependency *v1.
 		// the rollout/deployment event hasn't gone through yet.
 		// This can be ignored, and not be added back to the dependency controller queue
 		// because it will be processed by the rollout/deployment controller
-		ctxLogger.Infof(LogFormat, string(eventType), common.DependencyResourceType, dependency.Name, "", fmt.Sprintf("identity: %s, does not have any clusters. Skipping calling modifySE", dependency.Spec.Source))
 		return nil
 	}
 

admiral/pkg/clusters/deployment_handler.go

@@ -40,14 +40,10 @@ type HandleEventForDeploymentFunc func(
 func HandleEventForDeployment(ctx context.Context, event admiral.EventType, obj *k8sAppsV1.Deployment,
 	remoteRegistry *RemoteRegistry, clusterName string) error {
 
-	log.Infof(LogFormat, event, common.DeploymentResourceType, obj.Name, clusterName, common.ReceivedStatus)
 	globalIdentifier := common.GetDeploymentGlobalIdentifier(obj)
-	log.Infof(LogFormat, event, common.DeploymentResourceType, obj.Name, clusterName, "globalIdentifier is "+globalIdentifier)
 	originalIdentifier := common.GetDeploymentOriginalIdentifier(obj)
-	log.Infof(LogFormat, event, common.DeploymentResourceType, obj.Name, clusterName, "originalIdentifier is "+originalIdentifier)
 
 	if len(globalIdentifier) == 0 {
-		log.Infof(LogFormat, event, common.DeploymentResourceType, obj.Name, clusterName, "Skipped as '"+common.GetWorkloadIdentifier()+" was not found', namespace="+obj.Namespace)
 		return nil
 	}
 
@@ -66,7 +62,6 @@ func HandleEventForDeployment(ctx context.Context, event admiral.EventType, obj
 			}
 			if remoteRegistry.AdmiralCache.PartitionIdentityCache != nil && len(common.GetDeploymentIdentityPartition(obj)) > 0 {
 				remoteRegistry.AdmiralCache.PartitionIdentityCache.Put(globalIdentifier, originalIdentifier)
-				log.Infof(LogFormat, event, common.DeploymentResourceType, obj.Name, clusterName, "PartitionIdentityCachePut "+globalIdentifier+" for "+originalIdentifier)
 			}
 		}
 	}

admiral/pkg/clusters/destinationrule_handler.go

@@ -106,19 +106,6 @@ func getDestinationRule(se *networkingV1Alpha3.ServiceEntry, locality string, gt
 		dr.TrafficPolicy.LoadBalancer = loadBalancerSettings
 	}
 
-	if dr.TrafficPolicy.LoadBalancer.LocalityLbSetting != nil {
-		if dr.TrafficPolicy.LoadBalancer.LocalityLbSetting.Distribute != nil {
-			ctxLogger.Infof(common.CtxLogFormat,
-				"getDestinationRule", dr.Host, "", "", "Running in Active-Passive Mode")
-		} else {
-			ctxLogger.Infof(common.CtxLogFormat,
-				"getDestinationRule", dr.Host, "", "", "Running in Active-Active Mode")
-		}
-	} else {
-		ctxLogger.Infof(common.CtxLogFormat,
-			"getDestinationRule", dr.Host, "", "", "Running in Active-Active Mode")
-	}
-
 	derivedOutlierDetection := getOutlierDetection(se, locality, gtpTrafficPolicy, outlierDetection, common.DisableDefaultAutomaticFailover())
 	if derivedOutlierDetection != nil {
 		dr.TrafficPolicy.OutlierDetection = derivedOutlierDetection
@@ -246,11 +233,8 @@ func getOutlierDetection(
 	outlierDetectionCrd *v1.OutlierDetection,
 	disableDefaultAutomaticFailover bool) *networkingV1Alpha3.OutlierDetection {
 	if disableDefaultAutomaticFailover {
-		log.Infoln("default automatic failover is disabled. outlier detection " +
-			"will be configured only if OutlierDetection OR GTP resource is present")
 		if (outlierDetectionCrd == nil || (outlierDetectionCrd.Spec.OutlierConfig == nil)) &&
 			(gtpTrafficPolicy == nil || gtpTrafficPolicy.OutlierDetection == nil) {
-			log.Infoln("Neither outlier not GTP configured, will not set outlier configuration")
 			return &networkingV1Alpha3.OutlierDetection{
 				ConsecutiveGatewayErrors: &wrappers.UInt32Value{Value: 0},
 				Consecutive_5XxErrors:    &wrappers.UInt32Value{Value: 0},
@@ -272,12 +256,10 @@ func getOutlierDetection(
 	outlierDetection := getOutlierDetectionSkeleton(disableDefaultAutomaticFailover)
 	//Give priority to outlier detection crd than GTP. Eventually support for outlier detection via GTP will be stopped.
 	if outlierDetectionCrd != nil && outlierDetectionCrd.Spec.OutlierConfig != nil {
-		log.Infof("Using outlier detection config from Admiral Outlier Detection CRD. Hosts - %s", se.Hosts)
 		outlierDetection.ConsecutiveGatewayErrors = &wrappers.UInt32Value{Value: outlierDetectionCrd.Spec.OutlierConfig.ConsecutiveGatewayErrors}
 		outlierDetection.Interval = &duration.Duration{Seconds: outlierDetectionCrd.Spec.OutlierConfig.Interval}
 		outlierDetection.BaseEjectionTime = &duration.Duration{Seconds: outlierDetectionCrd.Spec.OutlierConfig.BaseEjectionTime}
 	} else if gtpTrafficPolicy != nil && gtpTrafficPolicy.OutlierDetection != nil {
-		log.Infof("Using outlier detection config from Admiral Global Traffic Policy CRD. Hosts - %s", se.Hosts)
 		setDefaultValuesOfOutlierDetection(outlierDetection)
 		if gtpTrafficPolicy.OutlierDetection.BaseEjectionTime > 0 {
 			outlierDetection.BaseEjectionTime = &duration.Duration{
@@ -333,14 +315,9 @@ func setDefaultValuesOfOutlierDetection(outlierDetection *networkingV1Alpha3.Out
 
 func (dh *DestinationRuleHandler) Added(ctx context.Context, obj *v1alpha3.DestinationRule) error {
 	if commonUtil.IsAdmiralReadOnly() {
-		log.Infof(LogFormat, "Add", "DestinationRule", obj.Name, dh.ClusterID, "Admiral is in read-only mode. Skipping resource from namespace="+obj.Namespace)
 		return nil
 	}
 	if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) {
-		log.Infof(LogFormat, "Add", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace="+obj.Namespace)
-		if len(obj.Annotations) > 0 && obj.Annotations[common.AdmiralIgnoreAnnotation] == "true" {
-			log.Infof(LogFormat, "admiralIoIgnoreAnnotationCheck", "DestinationRule", obj.Name, dh.ClusterID, "Value=true namespace="+obj.Namespace)
-		}
 		return nil
 	}
 	txId := common.FetchTxIdOrGenNew(ctx)
@@ -354,14 +331,9 @@ func (dh *DestinationRuleHandler) Added(ctx context.Context, obj *v1alpha3.Desti
 
 func (dh *DestinationRuleHandler) Updated(ctx context.Context, obj *v1alpha3.DestinationRule) error {
 	if commonUtil.IsAdmiralReadOnly() {
-		log.Infof(LogFormat, "Update", "DestinationRule", obj.Name, dh.ClusterID, "Admiral is in read-only mode. Skipping resource from namespace="+obj.Namespace)
 		return nil
 	}
 	if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) {
-		log.Infof(LogFormat, "Update", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace="+obj.Namespace)
-		if len(obj.Annotations) > 0 && obj.Annotations[common.AdmiralIgnoreAnnotation] == "true" {
-			log.Infof(LogFormat, "admiralIoIgnoreAnnotationCheck", "DestinationRule", obj.Name, dh.ClusterID, "Value=true namespace="+obj.Namespace)
-		}
 		return nil
 	}
 	txId := common.FetchTxIdOrGenNew(ctx)
@@ -375,14 +347,9 @@ func (dh *DestinationRuleHandler) Updated(ctx context.Context, obj *v1alpha3.Des
 
 func (dh *DestinationRuleHandler) Deleted(ctx context.Context, obj *v1alpha3.DestinationRule) error {
 	if commonUtil.IsAdmiralReadOnly() {
-		log.Infof(LogFormat, "Delete", "DestinationRule", obj.Name, dh.ClusterID, "Admiral is in read-only mode. Skipping resource from namespace="+obj.Namespace)
 		return nil
 	}
 	if IgnoreIstioResource(obj.Spec.ExportTo, obj.Annotations, obj.Namespace) {
-		log.Infof(LogFormat, "Delete", "DestinationRule", obj.Name, dh.ClusterID, "Skipping resource from namespace="+obj.Namespace)
-		if len(obj.Annotations) > 0 && obj.Annotations[common.AdmiralIgnoreAnnotation] == "true" {
-			log.Infof(LogFormat, "admiralIoIgnoreAnnotationCheck", "DestinationRule", obj.Name, dh.ClusterID, "Value=true namespace="+obj.Namespace)
-		}
 		return nil
 	}
 	txId := common.FetchTxIdOrGenNew(ctx)
@@ -406,7 +373,6 @@ func handleDestinationRuleEvent(ctxLogger *log.Entry, ctx context.Context, obj *
 	)
 
 	if len(dependentClusters) > 0 {
-		log.Infof(LogFormat, "Event", resourceType, obj.Name, clusterId, "Processing")
 		util.MapCopy(allDependentClusters, dependentClusters)
 		allDependentClusters[clusterId] = clusterId
 		for _, dependentCluster := range allDependentClusters {

admiral/pkg/clusters/dynamoDB.go

@@ -203,7 +203,6 @@ func (client *DynamoClient) getWorkloadDataItemByIdentityAndEnv(env, identity, t
 	}
 
 	if items == nil {
-		log.Infof("workload items came as nil for given env %s and identity %s in table %s", env, identity, tableName)
 		return workloadDataItems, nil
 	}
 

admiral/pkg/clusters/envoyfilter.go

@@ -69,8 +69,6 @@ func createOrUpdateEnvoyFilter(ctx context.Context, rc *RemoteController, routin
 		envoyFilterName := fmt.Sprintf("%s-dr-%s-%s-%s", strings.ToLower(routingPolicy.Spec.Plugin), routingPolicyNameSha, dependentIdentitySha, version)
 		envoyfilterSpec := constructEnvoyFilterStruct(routingPolicy, GetWorkloadSelectorLabels(workloadIdentityKey), version, envoyFilterName)
 
-		log.Infof(LogFormat, eventType, envoyFilter, envoyFilterName, rc.ClusterID, "version +"+version)
-
 		envoyfilter := &networking.EnvoyFilter{
 			TypeMeta: metaV1.TypeMeta{
 				Kind:       "EnvoyFilter",
@@ -99,11 +97,9 @@ func createOrUpdateEnvoyFilter(ctx context.Context, rc *RemoteController, routin
 				EnvoyFilters(filterNamespace).Get(ctx, envoyFilterName, metaV1.GetOptions{})
 
 			if k8sErrors.IsNotFound(err1) {
-				log.Infof(LogFormat, eventType, envoyFilter, envoyFilterName, rc.ClusterID, "creating the envoy filter")
 				filter, err2 = rc.RoutingPolicyController.IstioClient.NetworkingV1alpha3().
 					EnvoyFilters(filterNamespace).Create(ctx, envoyfilter, metaV1.CreateOptions{})
 			} else if err1 == nil {
-				log.Infof(LogFormat, eventType, envoyFilter, envoyFilterName, rc.ClusterID, "updating existing envoy filter")
 				envoyfilter.ResourceVersion = filter.ResourceVersion
 				filter, err2 = rc.RoutingPolicyController.IstioClient.NetworkingV1alpha3().
 					EnvoyFilters(filterNamespace).Update(ctx, envoyfilter, metaV1.UpdateOptions{})
@@ -154,8 +150,6 @@ func constructEnvoyFilterStruct(routingPolicy *v1.RoutingPolicy, workloadSelecto
 	envoyFilterStringConfig += getHosts(routingPolicy) + "\n"
 	envoyFilterStringConfig += getPlugin(routingPolicy)
 
-	log.Infof("msg=%s type=routingpolicy name=%s", "adding config", routingPolicy.Name)
-
 	configuration := structPb.Struct{
 		Fields: map[string]*structPb.Value{
 			"@type": {Kind: &structPb.Value_StringValue{StringValue: "type.googleapis.com/google.protobuf.StringValue"}},