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

mittachaitu · 2025-11-13T17:54:29Z

What type of PR is this?
/ kind bug

What this PR does / why we need it:
This PR adds telemetry=azpartner-aks/ as mount options for blobfuse mounts.
This is required to track, usage of Azure Blob Storage through AKS.

Which issue(s) this PR fixes:

Fixes #

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Special notes for your reviewer:

Need to check whether this tag is reflecting at blob storage metrics.

Release note:

none

This commit adds telemetry=azpartner-aks/<blob-driver-version> as mount options for blobfuse mounts Signed-off-by: [email protected] <[email protected]>

k8s-ci-robot · 2025-11-13T17:54:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mittachaitu
Once this PR has been reviewed and has the lgtm label, please assign feiskyer for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2025-11-13T17:54:37Z

Welcome @mittachaitu!

It looks like this is your first PR to kubernetes-sigs/blob-csi-driver 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/blob-csi-driver has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-11-13T17:54:38Z

Hi @mittachaitu. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

pkg/blob/nodeserver.go

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

andyzhangx

/ok-to-test

andyzhangx · 2025-11-17T07:20:10Z

pkg/blob/nodeserver.go

 	// 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)})


follow the PR example here: #2024 since protocol may be empty here, and also check whether this option is already specified

btw, I think just use --telemetry=aks/%s is more straightfoward

Andy, I have moved the code to under pkg/blobfuse-proxy/server.go and if user specifies the option through StorageClass then we are appending above to telemetry option

ex: In below example user has provided telemetry as part of mount option...

mountOptions: - -o allow_other - --file-cache-timeout-in-seconds=120 - --use-attr-cache=true - --cancel-list-on-mount-seconds=10 # prevent billing charges on mounting - -o attr_timeout=120 - -o entry_timeout=120 - -o negative_timeout=120 - --telemetry=tire1

Then mount args to blobfuse2 will be (--telemetry=azpartner-aks/v1.28.0,normaltest)

server.go:99] updated --telemetry tag in mount args: mount /var/lib/kubelet/plugins/kubernetes.io/csi/blob.csi.azure.com/d6b00230de0d40736dda3f197b49a4919bfb9b7512f4466d881183c6a2baa974/globalmount -o allow_other --file-cache-timeout-in-seconds=120 --use-attr-cache=true --cancel-list-on-mount-seconds=10 -o attr_timeout=120 -o entry_timeout=120 -o negative_timeout=120 --log-level=LOG_DEBUG --cache-size-mb=1000 --telemetry=azpartner-aks/v1.28.0,normaltest --pre-mount-validate=true --use-https=true --empty-dir-check=false --tmp-path=/mnt/<RG>#<Storage Account>#<Contianer>#<Blob>#default# --container-name=traineddata --ignore-open-flags=true --disable-version-check=true

Note: If there is no --telemetry in mount option then --telemetry=azpartner-aks/v1.28.0.

btw, I think just use --telemetry=aks/%s is more straightfoward

The tag has been suggested by @vibhansa-msft, so not sure if there is any monitoring from blobfuse side

This commit moves the logic to add tag to blobfuse-proxy service. If user specifies --telemetry=tire1 as mount option then add azpartner-aks/v1.28.0 value as comma separated value to user provided version (--telemetry=azpartner-aks/v1.28.0,tire1) Signed-off-by: [email protected] <[email protected]>

andyzhangx · 2025-11-19T14:57:49Z

pkg/blobfuse-proxy/server/server.go

+			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


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

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.

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.

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).

pkg/blobfuse-proxy/server/server.go

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

mittachaitu · 2025-11-26T12:59:11Z

/retest-required

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

Copilot

Pull request overview

This pull request adds telemetry tracking support for Azure Blob Storage usage through AKS by implementing a telemetry tag prefix in blobfuse mount operations. The implementation adds a version-aware telemetry tag (blobpartner-csi/<version>) to all blobfuse v2 mount operations to enable tracking and monitoring of blob storage access patterns.

Key changes:

Telemetry tag infrastructure with version injection via build-time ldflags
Logic to prepend AKS CSI driver telemetry tags to user-provided telemetry values
Comprehensive test coverage for various telemetry scenarios including existing tags, missing tags, and different blobfuse versions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
pkg/blobfuse-proxy/server/server.go	Implements telemetry tag injection logic for blobfuse v2 mounts with conditional handling for existing telemetry flags
pkg/blobfuse-proxy/server/server_test.go	Adds comprehensive test suite covering telemetry tag injection scenarios with mock command execution
Makefile	Updates build configuration to inject driver version into binary via ldflags for telemetry tagging

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-27T13:28:27Z

pkg/blobfuse-proxy/server/server.go

 )

+// telemetryTagPrefix is used to identify the mounts done via blobcsi driver
+const telemetryTagPrefix = "blobpartner-csi/"


The telemetry tag prefix is inconsistent with the PR description. The PR description states the telemetry tag should be "azpartner-aks/", but the implementation uses "blobpartner-csi/". This mismatch needs to be corrected to match the intended telemetry format.

Suggested change

const telemetryTagPrefix = "blobpartner-csi/"

const telemetryTagPrefix = "azpartner-aks/"

Copilot · 2025-11-27T13:28:28Z

pkg/blobfuse-proxy/server/server.go

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


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 · 2025-11-27T13:28:28Z

pkg/blobfuse-proxy/server/server.go

+				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)


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 · 2025-11-27T13:28:28Z

pkg/blobfuse-proxy/server/server.go

+			if !strings.Contains(args, telemetryTagPrefix) {
+				splitedArgs := strings.Split(args, "--telemetry=")
+				if len(splitedArgs) == 2 {
+					args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1]


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 · 2025-11-27T13:28:29Z

pkg/blobfuse-proxy/server/server_test.go

+			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"
+			},
+		},


Missing test case: The telemetry string manipulation logic in server.go lines 100-103 doesn't have a test case for when len(splitedArgs) != 2 (e.g., when "--telemetry=" appears multiple times in args or not at all in the else branch). This edge case should be tested to ensure the code handles malformed input gracefully without modifying args or causing unexpected behavior.

Copilot · 2025-11-27T13:28:29Z

pkg/blobfuse-proxy/server/server.go

+				}
+				klog.V(2).Infof("updated --telemetry tag in mount args: %s", args)


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 · 2025-11-27T13:28:29Z

pkg/blobfuse-proxy/server/server.go

+			if !strings.Contains(args, telemetryTagPrefix) {
+				splitedArgs := strings.Split(args, "--telemetry=")
+				if len(splitedArgs) == 2 {
+					args = splitedArgs[0] + " --telemetry=" + telemetryTag + "," + splitedArgs[1]


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

}

chore: add telemetry as part of mount options for blobfuse mounts

7999704

This commit adds telemetry=azpartner-aks/<blob-driver-version> as mount options for blobfuse mounts Signed-off-by: [email protected] <[email protected]>

k8s-ci-robot requested a review from andyzhangx November 13, 2025 17:54

k8s-ci-robot requested a review from cvvz November 13, 2025 17:54

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 13, 2025

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 13, 2025

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 13, 2025

andyzhangx reviewed Nov 14, 2025

View reviewed changes

pkg/blob/nodeserver.go Outdated Show resolved Hide resolved

fix: add telemetry to the flag only if protocol is fuse2

2b4e9c8

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

andyzhangx reviewed Nov 17, 2025

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 17, 2025

andyzhangx reviewed Nov 17, 2025

View reviewed changes

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 19, 2025

andyzhangx reviewed Nov 19, 2025

View reviewed changes

andyzhangx reviewed Nov 20, 2025

View reviewed changes

pkg/blobfuse-proxy/server/server.go Show resolved Hide resolved

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

5c45e5b

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

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 26, 2025

fix: fix test case linting issue

618aac5

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

andyzhangx changed the title ~~chore: add telemetry as part of mount options for blobfuse mounts~~ feat: add telemetry as part of mount options for blobfuse mounts Nov 27, 2025

andyzhangx requested a review from Copilot November 27, 2025 13:18

Copilot started reviewing on behalf of andyzhangx November 27, 2025 13:18 View session

Copilot finished reviewing on behalf of andyzhangx November 27, 2025 13:21

Copilot AI reviewed Nov 27, 2025

View reviewed changes

	const telemetryTagPrefix = "blobpartner-csi/"
	const telemetryTagPrefix = "azpartner-aks/"

-				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)
+					}
+				}

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

		}
		klog.V(2).Infof("updated --telemetry tag in mount args: %s", args)

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

Are you sure you want to change the base?

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

Conversation

mittachaitu commented Nov 13, 2025

Uh oh!

k8s-ci-robot commented Nov 13, 2025

Uh oh!

k8s-ci-robot commented Nov 13, 2025

Uh oh!

k8s-ci-robot commented Nov 13, 2025

Uh oh!

Uh oh!

andyzhangx left a comment

Choose a reason for hiding this comment

Uh oh!

andyzhangx Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

andyzhangx Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

mittachaitu Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

andyzhangx Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

mittachaitu Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

andyzhangx Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

mittachaitu Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mittachaitu commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants