Disable GCSFuse metrics exporting by default.

pkg/csi_driver/utils.go

@@ -188,7 +188,7 @@ func parseVolumeAttributes(fuseMountOptions []string, volumeContext map[string]s
 		fuseMountOptions = joinMountOptions(fuseMountOptions, strings.Split(mountOptions, ","))
 	}
 	skipCSIBucketAccessCheck := false
-	disableMetricsCollection := false
+	disableMetricsCollection := true
 	for volumeAttribute, mountOption := range volumeAttributesToMountOptionsMapping {
 		value, ok := volumeContext[volumeAttribute]
 		if !ok {

pkg/csi_driver/utils_test.go

@@ -70,12 +70,12 @@ func TestParseVolumeAttributes(t *testing.T) {
 	t.Run("parsing volume attributes into mount options", func(t *testing.T) {
 		t.Parallel()
 		testCases := []struct {
-			name                             string
-			volumeContext                    map[string]string
-			expectedMountOptions             []string
-			expectedSkipBucketAccessCheck    bool
-			expectedDisableMetricsCollection bool
-			expectedErr                      bool
+			name                            string
+			volumeContext                   map[string]string
+			expectedMountOptions            []string
+			expectedSkipBucketAccessCheck   bool
+			expectedEnableMetricsCollection bool
+			expectedErr                     bool
 		}{
 			{
 				name:                 "should return correct fileCacheCapacity 1",
@@ -379,40 +379,43 @@ func TestParseVolumeAttributes(t *testing.T) {
 				expectedErr:   true,
 			},
 			{
-				name:                             "value set to true for VolumeContextKeyDisableMetrics",
-				volumeContext:                    map[string]string{VolumeContextKeyDisableMetrics: util.TrueStr},
-				expectedMountOptions:             []string{volumeAttributesToMountOptionsMapping[VolumeContextKeyDisableMetrics] + util.TrueStr},
-				expectedDisableMetricsCollection: true,
+				name:                            "value set to true for VolumeContextKeyDisableMetrics",
+				volumeContext:                   map[string]string{VolumeContextKeyDisableMetrics: util.TrueStr},
+				expectedMountOptions:            []string{volumeAttributesToMountOptionsMapping[VolumeContextKeyDisableMetrics] + util.TrueStr},
+				expectedEnableMetricsCollection: false,
 			},
 			{
-				name:                             "value set to false for VolumeContextKeyDisableMetrics",
-				volumeContext:                    map[string]string{VolumeContextKeyDisableMetrics: util.FalseStr},
-				expectedMountOptions:             []string{volumeAttributesToMountOptionsMapping[VolumeContextKeyDisableMetrics] + util.FalseStr},
-				expectedDisableMetricsCollection: false,
+				name:                            "value set to false for VolumeContextKeyDisableMetrics",
+				volumeContext:                   map[string]string{VolumeContextKeyDisableMetrics: util.FalseStr},
+				expectedMountOptions:            []string{volumeAttributesToMountOptionsMapping[VolumeContextKeyDisableMetrics] + util.FalseStr},
+				expectedEnableMetricsCollection: true,
 			},
 		}
 
 		for _, tc := range testCases {
-			t.Logf("test case: %s", tc.name)
-			output, skipCSIBucketAccessCheck, disableMetricsCollection, err := parseVolumeAttributes([]string{}, tc.volumeContext)
-			if (err != nil) != tc.expectedErr {
-				t.Errorf("Got error %v, but expected error %v", err, tc.expectedErr)
-			}
+			t.Run(tc.name, func(t *testing.T) {
+				t.Logf("test case: %s", tc.name)
+				output, skipCSIBucketAccessCheck, disableMetricsCollection, err := parseVolumeAttributes([]string{}, tc.volumeContext)
+				if (err != nil) != tc.expectedErr {
+					t.Errorf("Got error %v, but expected error %v", err, tc.expectedErr)
+				}
+				enableMetricsCollection := !disableMetricsCollection
 
-			if tc.expectedErr {
-				continue
-			}
-			if tc.expectedSkipBucketAccessCheck != skipCSIBucketAccessCheck {
-				t.Errorf("Got skipBucketAccessCheck %v, but expected %v", skipCSIBucketAccessCheck, tc.expectedSkipBucketAccessCheck)
-			}
-			if tc.expectedDisableMetricsCollection != disableMetricsCollection {
-				t.Errorf("Got disableMetricsCollection %v, but expected %v", disableMetricsCollection, tc.expectedDisableMetricsCollection)
-			}
+				if tc.expectedErr {
+					return
+				}
+				if tc.expectedSkipBucketAccessCheck != skipCSIBucketAccessCheck {
+					t.Errorf("Got skipBucketAccessCheck %v, but expected %v", skipCSIBucketAccessCheck, tc.expectedSkipBucketAccessCheck)
+				}
+				if tc.expectedEnableMetricsCollection != enableMetricsCollection {
+					t.Errorf("Got disableMetricsCollection %v, but expected %v", enableMetricsCollection, tc.expectedEnableMetricsCollection)
+				}
 
-			less := func(a, b string) bool { return a > b }
-			if diff := cmp.Diff(output, tc.expectedMountOptions, cmpopts.SortSlices(less)); diff != "" {
-				t.Errorf("unexpected options args (-got, +want)\n%s", diff)
-			}
+				less := func(a, b string) bool { return a > b }
+				if diff := cmp.Diff(output, tc.expectedMountOptions, cmpopts.SortSlices(less)); diff != "" {
+					t.Errorf("unexpected options args (-got, +want)\n%s", diff)
+				}
+			})
 		}
 	})
 }

pkg/sidecar_mounter/sidecar_mounter_config.go

@@ -150,16 +150,13 @@ func NewMountConfig(sp string) *MountConfig {
 
 func (mc *MountConfig) prepareMountArgs() {
 	flagMap := map[string]string{
-		"app-name":        GCSFuseAppName,
-		"temp-dir":        mc.BufferDir + TempDir,
-		"config-file":     mc.ConfigFile,
-		"foreground":      "",
-		"uid":             "0",
-		"gid":             "0",
-		"prometheus-port": strconv.Itoa(prometheusPort),
+		"app-name":    GCSFuseAppName,
+		"temp-dir":    mc.BufferDir + TempDir,
+		"config-file": mc.ConfigFile,
+		"foreground":  "",
+		"uid":         "0",
+		"gid":         "0",
 	}
-	// Use a new port each gcsfuse instance that we start.
-	prometheusPort++
 
 	configFileFlagMap := map[string]string{
 		"logging:file-path": "/dev/fd/1", // redirect the output to cmd stdout
@@ -174,8 +171,10 @@ func (mc *MountConfig) prepareMountArgs() {
 			i := strings.LastIndex(arg, ":")
 			f, v := arg[:i], arg[i+1:]
 
-			if f == util.DisableMetricsForGKE && v == util.TrueStr {
-				flagMap["prometheus-port"] = "0"
+			if f == util.DisableMetricsForGKE && v == util.FalseStr {
+				flagMap["prometheus-port"] = strconv.Itoa(prometheusPort)
+				// Use a new port each gcsfuse instance that we start.
+				prometheusPort++
 
 				continue
 			}

pkg/sidecar_mounter/sidecar_mounter_config_test.go

@@ -220,7 +220,7 @@ func TestPrepareMountArgs(t *testing.T) {
 				BufferDir:  "test-buffer-dir",
 				CacheDir:   "test-cache-dir",
 				ConfigFile: "test-config-file",
-				Options:    []string{util.DisableMetricsForGKE + ":true"},
+				Options:    []string{util.DisableMetricsForGKE + ":false"},
 			},
 			expectedArgs: map[string]string{
 				"app-name":        GCSFuseAppName,
@@ -237,30 +237,31 @@ func TestPrepareMountArgs(t *testing.T) {
 
 	prometheusPort := 8080
 	for _, tc := range testCases {
-		t.Logf("test case: %s", tc.name)
-
-		found := false
-		for _, o := range tc.mc.Options {
-			if o == util.DisableMetricsForGKE+":true" {
-				found = true
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			found := false
+			for _, o := range tc.mc.Options {
+				if o == util.DisableMetricsForGKE+":false" {
+					found = true
 
-				break
+					break
+				}
 			}
-		}
 
-		if !found {
-			tc.expectedArgs["prometheus-port"] = strconv.Itoa(prometheusPort)
-			prometheusPort++
-		}
+			if found {
+				tc.expectedArgs["prometheus-port"] = strconv.Itoa(prometheusPort)
+				prometheusPort++
+			}
 
-		tc.mc.prepareMountArgs()
-		if !reflect.DeepEqual(tc.mc.FlagMap, tc.expectedArgs) {
-			t.Errorf("Got args %v, but expected %v", tc.mc.FlagMap, tc.expectedArgs)
-		}
+			tc.mc.prepareMountArgs()
+			if !reflect.DeepEqual(tc.mc.FlagMap, tc.expectedArgs) {
+				t.Errorf("Got args %v, but expected %v", tc.mc.FlagMap, tc.expectedArgs)
+			}
 
-		if !reflect.DeepEqual(tc.mc.ConfigFileFlagMap, tc.expectedConfigMapArgs) {
-			t.Errorf("Got config file args %v, but expected %v", tc.mc.ConfigFileFlagMap, tc.expectedConfigMapArgs)
-		}
+			if !reflect.DeepEqual(tc.mc.ConfigFileFlagMap, tc.expectedConfigMapArgs) {
+				t.Errorf("Got config file args %v, but expected %v", tc.mc.ConfigFileFlagMap, tc.expectedConfigMapArgs)
+			}
+		})
 	}
 }
 

test/e2e/specs/specs.go

@@ -63,7 +63,9 @@ const (
 	SubfolderInBucketPrefix                                    = "gcsfuse-csi-subfolder-in-bucket"
 	MultipleBucketsPrefix                                      = "gcsfuse-csi-multiple-buckets"
 	EnableFileCacheForceNewBucketPrefix                        = "gcsfuse-csi-enable-file-cache-force-new-bucket"
+	EnableFileCacheForceNewBucketAndMetricsPrefix              = "gcsfuse-csi-enable-file-cache-force-new-bucket-and-metrics"
 	EnableFileCachePrefix                                      = "gcsfuse-csi-enable-file-cache"
+	EnableFileCacheAndMetricsPrefix                            = "gcsfuse-csi-enable-file-cache-and-metrics"
 	EnableFileCacheWithLargeCapacityPrefix                     = "gcsfuse-csi-enable-file-cache-large-capacity"
 	ImplicitDirsPath                                           = "implicit-dir"
 	InvalidVolume                                              = "<invalid-name>"

test/e2e/specs/testdriver.go

@@ -57,6 +57,7 @@ type gcsVolume struct {
 	shared                  bool
 	readOnly                bool
 	skipBucketAccessCheck   bool
+	enableMetrics           bool
 }
 
 // InitGCSFuseCSITestDriver returns GCSFuseCSITestDriver that implements TestDriver interface.
@@ -151,7 +152,7 @@ func (n *GCSFuseCSITestDriver) CreateVolume(ctx context.Context, config *storage
 			bucketName = uuid.NewString()
 		case InvalidVolumePrefix, SkipCSIBucketAccessCheckAndInvalidVolumePrefix:
 			bucketName = InvalidVolume
-		case ForceNewBucketPrefix, EnableFileCacheForceNewBucketPrefix:
+		case ForceNewBucketPrefix, EnableFileCacheForceNewBucketPrefix, EnableFileCacheForceNewBucketAndMetricsPrefix:
 			bucketName = n.createBucket(ctx, config.Framework.Namespace.Name)
 		case MultipleBucketsPrefix:
 			isMultipleBucketsPrefix = true
@@ -206,6 +207,9 @@ func (n *GCSFuseCSITestDriver) CreateVolume(ctx context.Context, config *storage
 			mountOptions += ",only-dir=" + dirPath
 		case EnableFileCachePrefix, EnableFileCacheForceNewBucketPrefix:
 			v.fileCacheCapacity = "100Mi"
+		case EnableFileCacheAndMetricsPrefix, EnableFileCacheForceNewBucketAndMetricsPrefix:
+			v.fileCacheCapacity = "100Mi"
+			v.enableMetrics = true
 		case EnableFileCacheWithLargeCapacityPrefix:
 			v.fileCacheCapacity = "2Gi"
 		case SkipCSIBucketAccessCheckPrefix, SkipCSIBucketAccessCheckAndFakeVolumePrefix, SkipCSIBucketAccessCheckAndInvalidVolumePrefix:
@@ -229,7 +233,7 @@ func (n *GCSFuseCSITestDriver) CreateVolume(ctx context.Context, config *storage
 		}
 
 		switch config.Prefix {
-		case "", EnableFileCachePrefix, EnableFileCacheWithLargeCapacityPrefix:
+		case "", EnableFileCachePrefix, EnableFileCacheWithLargeCapacityPrefix, EnableFileCacheAndMetricsPrefix:
 			// Use config.Prefix to pass the bucket names back to the test suite.
 			config.Prefix = bucketName
 		}
@@ -262,6 +266,10 @@ func (n *GCSFuseCSITestDriver) GetPersistentVolumeSource(readOnly bool, _ string
 		va[driver.VolumeContextKeySkipCSIBucketAccessCheck] = util.TrueStr
 	}
 
+	if gv.enableMetrics {
+		va[driver.VolumeContextKeyDisableMetrics] = util.FalseStr
+	}
+
 	return &corev1.PersistentVolumeSource{
 		CSI: &corev1.CSIPersistentVolumeSource{
 			Driver:           n.driverInfo.Name,
@@ -289,6 +297,10 @@ func (n *GCSFuseCSITestDriver) GetVolume(config *storageframework.PerTestConfig,
 		va[driver.VolumeContextKeySkipCSIBucketAccessCheck] = util.TrueStr
 	}
 
+	if gv.enableMetrics {
+		va[driver.VolumeContextKeyDisableMetrics] = util.FalseStr
+	}
+
 	return va, gv.shared, gv.readOnly
 }
 

test/e2e/testsuites/metrics.go

@@ -257,7 +257,7 @@ func (t *gcsFuseCSIMetricsTestSuite) DefineTests(driver storageframework.TestDri
 	//       |
 	//   [bucket1]
 	ginkgo.It("should emit metrics", func() {
-		init(1, specs.EnableFileCachePrefix)
+		init(1, specs.EnableFileCacheAndMetricsPrefix)
 		defer cleanup()
 
 		ginkgo.By("Configuring the pod")
@@ -281,7 +281,7 @@ func (t *gcsFuseCSIMetricsTestSuite) DefineTests(driver storageframework.TestDri
 	//       |          |
 	//   [bucket1]  [bucket2]
 	ginkgo.It("should emit metrics from multiple volumes", func() {
-		init(2, specs.EnableFileCacheForceNewBucketPrefix)
+		init(2, specs.EnableFileCacheForceNewBucketAndMetricsPrefix)
 		defer cleanup()
 
 		ginkgo.By("Configuring the pod")