Set SampleRate to 0 when LogConfig is disabled for BackendService

kubernetes · Apr 4, 2023 · c97ce60 · c97ce60
1 parent 670f177
commit c97ce60
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 7 deletions.
diff --git a/pkg/backends/features/logging.go b/pkg/backends/features/logging.go
@@ -32,7 +32,7 @@ func EnsureLogging(sp utils.ServicePort, be *composite.BackendService) bool {
 		return false
 	}
 	svcKey := fmt.Sprintf("%s/%s", sp.ID.Service.Namespace, sp.ID.Service.Name)
-	if be.LogConfig != nil && !be.LogConfig.Enable && !sp.BackendConfig.Spec.Logging.Enable {
+	if be.LogConfig != nil && !be.LogConfig.Enable && !sp.BackendConfig.Spec.Logging.Enable && be.LogConfig.SampleRate == 0.0 {
 		klog.V(3).Infof("Logging continues to stay disabled for service %s (port %d), skipping update", svcKey, sp.Port)
 		return false
 	}
@@ -44,8 +44,7 @@ func EnsureLogging(sp utils.ServicePort, be *composite.BackendService) bool {
 		}
 	}
 	ensureBackendServiceLogConfig(sp, be)
-	if existingLogConfig == nil || existingLogConfig.Enable != be.LogConfig.Enable ||
-		existingLogConfig.SampleRate != be.LogConfig.SampleRate {
+	if existingLogConfig == nil || existingLogConfig.Enable != be.LogConfig.Enable || existingLogConfig.SampleRate != be.LogConfig.SampleRate {
 		klog.V(2).Infof("Updated Logging settings for service %s (port %d) to (Enable: %t, SampleRate: %f)", svcKey, sp.Port, be.LogConfig.Enable, be.LogConfig.SampleRate)
 		return true
 	}
@@ -61,8 +60,9 @@ func ensureBackendServiceLogConfig(sp utils.ServicePort, be *composite.BackendSe
 		be.LogConfig = &composite.BackendServiceLogConfig{}
 	}
 	be.LogConfig.Enable = sp.BackendConfig.Spec.Logging.Enable
-	// Ignore sample rate if logging is not enabled.
+	// SampleRate should be set to 0.0 if logging is not enabled.
 	if !sp.BackendConfig.Spec.Logging.Enable {
+		be.LogConfig.SampleRate = 0.0
 		return
 	}
 	// Existing sample rate is retained if not specified.

diff --git a/pkg/backends/features/logging_test.go b/pkg/backends/features/logging_test.go
@@ -102,11 +102,31 @@ func TestEnsureLogging(t *testing.T) {
 			be: &composite.BackendService{
 				LogConfig: &composite.BackendServiceLogConfig{
 					Enable:     false,
-					SampleRate: 1.0,
+					SampleRate: 0.0,
 				},
 			},
 			expectUpdate: false,
 		},
+		{
+			desc: "logging remains disabled but has incorrect sample rate, update needed",
+			sp: utils.ServicePort{
+				BackendConfig: &backendconfigv1.BackendConfig{
+					Spec: backendconfigv1.BackendConfigSpec{
+						Logging: &backendconfigv1.LogConfig{
+							Enable:     false,
+							SampleRate: testutils.Float64ToPtr(0.4),
+						},
+					},
+				},
+			},
+			be: &composite.BackendService{
+				LogConfig: &composite.BackendServiceLogConfig{
+					Enable:     false,
+					SampleRate: 0.7,
+				},
+			},
+			expectUpdate: true,
+		},
 		{
 			desc: "no existing settings, update needed",
 			sp: utils.ServicePort{
@@ -272,7 +292,7 @@ func TestEnsureBackendServiceLogConfig(t *testing.T) {
 			},
 		},
 		{
-			desc: "logging stays disabled, sample rate retained",
+			desc: "logging is disabled, sample rate defaults to 0.0",
 			sp: utils.ServicePort{
 				BackendConfig: &backendconfigv1.BackendConfig{
 					Spec: backendconfigv1.BackendConfigSpec{
@@ -284,13 +304,34 @@ func TestEnsureBackendServiceLogConfig(t *testing.T) {
 				},
 			},
 			logConfig: &composite.BackendServiceLogConfig{
-				Enable:     false,
+				Enable:     true,
 				SampleRate: 0.4,
 			},
 			expectLogConfig: &composite.BackendServiceLogConfig{
+				Enable:     false,
+				SampleRate: 0.0,
+			},
+		},
+		{
+			desc: "logging stays disabled, sample rate defaults to 0.0",
+			sp: utils.ServicePort{
+				BackendConfig: &backendconfigv1.BackendConfig{
+					Spec: backendconfigv1.BackendConfigSpec{
+						Logging: &backendconfigv1.LogConfig{
+							Enable:     false,
+							SampleRate: testutils.Float64ToPtr(0.5),
+						},
+					},
+				},
+			},
+			logConfig: &composite.BackendServiceLogConfig{
 				Enable:     false,
 				SampleRate: 0.4,
 			},
+			expectLogConfig: &composite.BackendServiceLogConfig{
+				Enable:     false,
+				SampleRate: 0.0,
+			},
 		},
 		{
 			desc: "logging enabled, sample rate retained",