Skip to content

feat: add telemetry as part of mount options for blobfuse mounts #2248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+120 −4
Open

feat: add telemetry as part of mount options for blobfuse mounts #2248

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7999704
chore: add telemetry as part of mount options for blobfuse mounts
mittachaitu Nov 10, 2025
2b4e9c8
fix: add telemetry to the flag only if protocol is fuse2
mittachaitu Nov 17, 2025
8d9b9fd
fix: move the tag adding logic inside blobfuse-proxy code
mittachaitu Nov 19, 2025
5c45e5b
fix: update telemtery tag and add Unit test for telemetry parsing
mittachaitu Nov 26, 2025
618aac5
fix: fix test case linting issue
mittachaitu Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,4 @@ delete-metrics-svc:

.PHONY: blobfuse-proxy
blobfuse-proxy:
CGO_ENABLED=0 GOOS=linux GOARCH=$(ARCH) go build -mod vendor -ldflags="-s -w" -o _output/${ARCH}/blobfuse-proxy ./pkg/blobfuse-proxy
CGO_ENABLED=0 GOOS=linux GOARCH=$(ARCH) go build -mod vendor -ldflags="-s -w -X ${PKG}/pkg/blobfuse-proxy/server.driverVersion=${IMAGE_VERSION}" -o _output/${ARCH}/blobfuse-proxy ./pkg/blobfuse-proxy
4 changes: 0 additions & 4 deletions pkg/blob/nodeserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,6 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
}

// Get mountOptions that the volume will be formatted and mounted with
if protocol == Fuse2 {
// Adding telemetry tag to know that blob is been mounted through AKS
mountFlags = util.JoinMountOptions(mountFlags, []string{fmt.Sprintf("--telemetry=azpartner-aks/%s", d.Version)})
}
mountOptions := mountFlags
if ephemeralVol {
mountOptions = util.JoinMountOptions(mountOptions, strings.Split(ephemeralVolMountOptions, ","))
Expand Down
19 changes: 18 additions & 1 deletion pkg/blobfuse-proxy/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ import (
)

var (
mutex sync.Mutex
mutex sync.Mutex
driverVersion string
)

type BlobfuseVersion int
Expand Down Expand Up @@ -71,6 +72,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
args = "mount " + args
// add this arg for blobfuse2 to solve the issue:
// https://github.com/Azure/azure-storage-fuse/issues/1015
Expand All @@ -82,6 +84,21 @@ func (server *MountServer) MountAzureBlob(_ context.Context,
klog.V(2).Infof("append --disable-version-check to mount args")
args = args + " " + "--disable-version-check=true"
}
// Adding telemetry tag to know that blob is been mounted through AKS CSI Driver
if !strings.Contains(args, "--telemetry") {
klog.V(2).Infof("append --telemetry=%s to mount args", telemetryTag)
args = args + " " + "--telemetry=" + telemetryTag
} else {
// If telemetry flag is already present, check for aks tag if not present
Copy link
Member

@andyzhangx andyzhangx Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If telemetry flag is already present, we just respect original setting, I don't think we need to change the original value by force

Copy link
Author

@mittachaitu mittachaitu Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If user has some custom tag, then we can't use that for tracking blob usage through AKS. If we just use the original setting then whole purpose to track usage will be lost, so I appended our tag with user specified value.

Copy link
Member

@andyzhangx andyzhangx Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but what if customer only wants to keep the original telemetry? this PR would change the telemetry even user does not want to.

and this adds the telemetry as azpartner-aks in open source driver, which means if it runs on AWS k8s node, it would still have this azpartner-aks telemetry, so maybe use default userAgent as fmt.Sprintf("%s/%s", driverName, driverVersion) is more reasonable.

Copy link
Author

@mittachaitu mittachaitu Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated tag to blobpartner-csi/<driver-version>, Concern from blobfuse team is if we go with driver specific tag then it will become costly operation for blobfuse analytics team to parse these tags (since they are expecting all partners should use similar tag instead of each partner goes with different tag).

// then user might have their own telemetry tag append aks tag to it
if !strings.Contains(args, "azpartner-aks") {
splitedArgs := strings.Split(args, "--telemetry=")
if len(splitedArgs) == 2 {
args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1]
Comment on lines +100 to +102
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in variable name: 'splitedArgs' should be 'splitArgs' or 'splittedArgs'. 'Splited' is not a valid English word; the past tense of 'split' is 'split' or 'splitted' (though 'split' is more common).

Suggested change
splitedArgs := strings.Split(args, "--telemetry=")
if len(splitedArgs) == 2 {
args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1]
splitArgs := strings.Split(args, "--telemetry=")
if len(splitArgs) == 2 {
args = splitArgs[0] + " --telemetry=" + telemetryTag + "," + splitArgs[1]

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string manipulation logic for inserting the telemetry tag creates a potential issue with spacing. The code constructs splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1], which adds a space before --telemetry= but doesn't trim it from splitedArgs[0]. This could result in double spaces if splitedArgs[0] already ends with a space. Consider using strings.TrimSpace() on splitedArgs[0] before concatenation.

Suggested change
args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1]
args = strings.TrimSpace(splitedArgs[0]) + " --telemetry=" + telemetryTag + "," + splitedArgs[1]

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic doesn't handle the edge case where --telemetry= is provided with an empty value (e.g., --telemetry= --other-flag). In this case, splitedArgs[1] would start with a space and a different flag, resulting in malformed telemetry arguments like --telemetry=blobpartner-csi/version, --other-flag. Consider trimming whitespace from splitedArgs[1] and checking if it's empty before concatenating.

Suggested change
args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1]
telemetryValue := strings.TrimSpace(splitedArgs[1])
if telemetryValue == "" {
args = splitedArgs[0] + " --telemetry=" + telemetryTag
} else {
args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + telemetryValue
}

Copilot uses AI. Check for mistakes.
}
klog.V(2).Infof("updated --telemetry tag in mount args: %s", args)
Comment on lines +100 to +104
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The telemetry string parsing logic has a critical flaw. When splitting by "--telemetry=", if there are multiple occurrences of this substring in args (e.g., in a container name or account name), len(splitedArgs) could be greater than 2, but only the first occurrence is handled. This could result in malformed arguments. Consider using a more robust parsing method such as regular expressions or proper argument parsing to specifically target the --telemetry flag.

Suggested change
splitedArgs := strings.Split(args, "--telemetry=")
if len(splitedArgs) == 2 {
args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1]
}
klog.V(2).Infof("updated --telemetry tag in mount args: %s", args)
// Split args into slice for robust parsing
argSlice := strings.Fields(args)
telemetryIdx := -1
for i, arg := range argSlice {
if strings.HasPrefix(arg, "--telemetry=") {
telemetryIdx = i
break
}
}
if telemetryIdx != -1 {
// Update the telemetry tag, appending our tag if not present
telemetryVal := strings.TrimPrefix(argSlice[telemetryIdx], "--telemetry=")
// Avoid duplicate tags
if !strings.Contains(telemetryVal, telemetryTagPrefix) {
argSlice[telemetryIdx] = "--telemetry=" + telemetryTag + "," + telemetryVal
args = strings.Join(argSlice, " ")
klog.V(2).Infof("updated --telemetry tag in mount args: %s", args)
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +103 to +104
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code silently does nothing when len(splitedArgs) != 2 (line 101 condition). This can occur if the string "--telemetry=" appears multiple times or is part of another parameter value. Consider adding logging (e.g., klog.Warningf) to indicate when the telemetry tag cannot be updated, so operators can identify potential issues with their mount arguments.

Suggested change
}
klog.V(2).Infof("updated --telemetry tag in mount args: %s", args)
klog.V(2).Infof("updated --telemetry tag in mount args: %s", args)
} else {
klog.Warningf("could not update --telemetry tag in mount args: unexpected format or multiple --telemetry= flags found in args: %s", args)
}

Copilot uses AI. Check for mistakes.
}
}
args = util.TrimDuplicatedSpace(args)
klog.V(2).Infof("mount with v2, protocol: %s, args: %s", protocol, args)
cmd = exec.Command("blobfuse2", strings.Split(args, " ")...)
Expand Down
Loading