fix: update telemtery tag and add Unit test for telemetry parsing

Signed-off-by: mittachaitu <[email protected]>

Author: mittachaitu

pkg/blobfuse-proxy/server/server.go

@@ -38,6 +38,9 @@ var (
 	driverVersion string
 )
 
+// telemetryTagPrefix is used to identify the mounts done via blobcsi driver
+const telemetryTagPrefix = "blobpartner-csi/"
+
 type BlobfuseVersion int
 
 const (
@@ -48,12 +51,14 @@ const (
 type MountServer struct {
 	blobfuseVersion BlobfuseVersion
 	mount_azure_blob.UnimplementedMountServiceServer
+	exec func(name string, arg ...string) *exec.Cmd
 }
 
 // NewMountServer returns a new Mountserver
 func NewMountServiceServer() *MountServer {
 	mountServer := &MountServer{}
 	mountServer.blobfuseVersion = getBlobfuseVersion()
+	mountServer.exec = exec.Command
 	return mountServer
 }
 
@@ -72,7 +77,7 @@ func (server *MountServer) MountAzureBlob(_ context.Context,
 	var cmd *exec.Cmd
 	var result mount_azure_blob.MountAzureBlobResponse
 	if protocol == blob.Fuse2 || server.blobfuseVersion == BlobfuseV2 {
-		telemetryTag := "azpartner-aks/" + driverVersion
+		telemetryTag := telemetryTagPrefix + driverVersion
 		args = "mount " + args
 		// add this arg for blobfuse2 to solve the issue:
 		// https://github.com/Azure/azure-storage-fuse/issues/1015
@@ -91,7 +96,7 @@ func (server *MountServer) MountAzureBlob(_ context.Context,
 		} else {
 			// If telemetry flag is already present, check for aks tag if not present
 			// then user might have their own telemetry tag append aks tag to it
-			if !strings.Contains(args, "azpartner-aks") {
+			if !strings.Contains(args, telemetryTagPrefix) {
 				splitedArgs := strings.Split(args, "--telemetry=")
 				if len(splitedArgs) == 2 {
 					args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1]
@@ -101,11 +106,11 @@ func (server *MountServer) MountAzureBlob(_ context.Context,
 		}
 		args = util.TrimDuplicatedSpace(args)
 		klog.V(2).Infof("mount with v2, protocol: %s, args: %s", protocol, args)
-		cmd = exec.Command("blobfuse2", strings.Split(args, " ")...)
+		cmd = server.exec("blobfuse2", strings.Split(args, " ")...)
 	} else {
 		args = util.TrimDuplicatedSpace(args)
 		klog.V(2).Infof("mount with v1, protocol: %s, args: %s", protocol, args)
-		cmd = exec.Command("blobfuse", strings.Split(args, " ")...)
+		cmd = server.exec("blobfuse", strings.Split(args, " ")...)
 	}
 
 	cmd.Env = append(os.Environ(), authEnv...)

pkg/blobfuse-proxy/server/server_test.go

@@ -18,6 +18,8 @@ package server
 
 import (
 	"context"
+	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -64,3 +66,95 @@ func TestServerMountAzureBlob(t *testing.T) {
 		})
 	}
 }
+
+// fakeExecCommand is used to mock exec.Command for testing, it returns list of args
+func fakeExecCommandEchoArgs(command string, args ...string) *exec.Cmd {
+	return exec.Command("echo", append([]string{"-n"}, args...)...)
+}
+
+func TestServerMountAzureBlob_Telemetry(t *testing.T) {
+	driverVersion = "fake-version"
+	t.Parallel()
+	testCases := []struct {
+		name                  string
+		args                  string
+		code                  codes.Code
+		mountServer           MountServer
+		areValidTelemetryArgs func(cmdArgs string) bool
+	}{
+		{
+			name: "mount_with_telemetry_tag_blobfusev2",
+			args: "--account-name=testaccount --container-name=testcontainer --telemetry=volume1-app1 --tmp-path=/tmp/blobfuse-tmp",
+			mountServer: MountServer{
+				blobfuseVersion: BlobfuseV2,
+				exec:            fakeExecCommandEchoArgs,
+			},
+			code: codes.OK,
+			areValidTelemetryArgs: func(cmdArgs string) bool {
+				expectedTelemetryArg := "--telemetry=" + telemetryTagPrefix + driverVersion + ",volume1-app1"
+				return strings.Contains(cmdArgs, expectedTelemetryArg)
+			},
+		},
+		{
+			name: "mount_without_telemetry_tag_blobfusev2",
+			args: "--account-name=testaccount --container-name=testcontainer --tmp-path=/tmp/blobfuse-tmp",
+			mountServer: MountServer{
+				blobfuseVersion: BlobfuseV2,
+				exec:            fakeExecCommandEchoArgs,
+			},
+			code: codes.OK,
+			areValidTelemetryArgs: func(cmdArgs string) bool {
+				expectedTelemetryArg := "--telemetry=" + telemetryTagPrefix + driverVersion
+				return strings.Contains(cmdArgs, expectedTelemetryArg)
+			},
+		},
+		{
+			name: "mount_with_same_telemetry_tag_blobfusev2",
+			args: "--account-name=testaccount --container-name=testcontainer --telemetry=" + telemetryTagPrefix + driverVersion,
+			mountServer: MountServer{
+				blobfuseVersion: BlobfuseV2,
+				exec:            fakeExecCommandEchoArgs,
+			},
+			code: codes.OK,
+			areValidTelemetryArgs: func(cmdArgs string) bool {
+				// Argument order should remain unchanged
+				return strings.Contains(cmdArgs, "--account-name=testaccount --container-name=testcontainer --telemetry="+telemetryTagPrefix+driverVersion)
+			},
+		},
+		{
+			name: "mount_with_blobfusev1",
+			args: "--account-name=testaccount --container-name=testcontainer --tmp-path=/tmp/blobfuse-tmp",
+			mountServer: MountServer{
+				blobfuseVersion: BlobfuseV1,
+				exec:            fakeExecCommandEchoArgs,
+			},
+			code: codes.OK,
+			areValidTelemetryArgs: func(cmdArgs string) bool {
+				// No telemetry arg should be added for blobfuse v1
+				return !strings.Contains(cmdArgs, "--telemetry=") && cmdArgs == "--account-name=testaccount --container-name=testcontainer --tmp-path=/tmp/blobfuse-tmp"
+			},
+		},
+	}
+
+	for i := range testCases {
+		tc := testCases[i]
+
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			req := mount_azure_blob.MountAzureBlobRequest{
+				MountArgs: tc.args,
+				AuthEnv:   []string{},
+			}
+			res, err := tc.mountServer.MountAzureBlob(context.Background(), &req)
+			if tc.code == codes.OK {
+				require.NoError(t, err)
+				require.NotNil(t, res)
+				require.True(t, tc.areValidTelemetryArgs(res.Output), "telemetry args are not valid in command args: %s", res.Output)
+			} else {
+				require.Error(t, err)
+				require.NotNil(t, res)
+			}
+		})
+	}
+}