HADOOP-19575. ABFS: [FNSOverBlob] Add Distinct String In User Agent to Get Telemetry for FNS-Blob #7713
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| @Test | ||
| // Test to verify that the user agent string for FNS-Blob accounts | ||
| public void verifyUserAgentForFNSBlob() throws Exception { |
Contributor
There was a problem hiding this comment.
add similar test for dfs and validate that no extra string is added.
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
Test Results============================================================
|
anujmodi2021
approved these changes
May 29, 2025
Contributor
anujmodi2021
left a comment
anujmodi2021
left a comment
There was a problem hiding this comment.
+1
Thanks for the patch. LGTM
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
| @Test | ||
| // Test to verify that the user agent string for non-FNS-Blob accounts | ||
| // does not contain the FNS identifier. | ||
| public void verifyUserAgentForDFS() throws Exception { |
Contributor
There was a problem hiding this comment.
assumeHnsDisabled() as well
Contributor
Author
There was a problem hiding this comment.
For both HNS-DFS and FNS-DFS we would want to check that the string isnt added right
| public static final Logger LOG = LoggerFactory.getLogger(AbfsClient.class); | ||
| public static final String HUNDRED_CONTINUE_USER_AGENT = SINGLE_WHITE_SPACE + HUNDRED_CONTINUE + SEMICOLON; | ||
| public static final String ABFS_CLIENT_TIMER_THREAD_NAME = "abfs-timer-client"; | ||
| public static final String FNS_BLOB_USER_AGENT_IDENTIFIER = "FNS"; |
Contributor
There was a problem hiding this comment.
Any reason for just facing FNS as value for FNS Blob user agent? FNS user agent can be there for FNS DFS as well, right?
Contributor
Author
There was a problem hiding this comment.
We are adding the string only for FNS-Blob cases (since we wanted to check their usage only)
Initially we added "FNS-Blob" as string but then decided to shorten it to "FNS" since the endpoint we can check from the URL itself
| @Test | ||
| // Test to verify the unique identifier in user agent string for FNS-Blob accounts | ||
| public void verifyUserAgentForFNSBlob() throws Exception { | ||
| Assume.assumeTrue(JDK_HTTP_URL_CONNECTION == httpOperationType); |
Contributor
There was a problem hiding this comment.
Why this assume for connection type?
Contributor
Author
There was a problem hiding this comment.
Right, the user-agent string checks here should be network library agnostic. Made the changes.
|
🎊 +1 overall
This message was automatically generated. |
| import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.Permissions; | ||
| import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants; | ||
| import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ApiVersion; | ||
| import org.apache.hadoop.fs.azurebfs.constants.AbfsServiceType; |
Contributor
There was a problem hiding this comment.
Instead of importing the entire AbfsServiceType class, we can use static import of BLOB enum.
| sb.append(abfsConfiguration.getClusterType()); | ||
|
|
||
| // Add a unique identifier in FNS-Blob user agent string | ||
| if (!getIsNamespaceEnabled() && abfsConfiguration.getFsConfiguredServiceType() == AbfsServiceType.BLOB){ |
Contributor
There was a problem hiding this comment.
With static import, we just need to give BLOB, instead of AbfsServiceType.BLOB
| // Add a unique identifier in FNS-Blob user agent string | ||
| if (!getIsNamespaceEnabled() && abfsConfiguration.getFsConfiguredServiceType() == AbfsServiceType.BLOB){ | ||
| sb.append(SEMICOLON).append(SINGLE_WHITE_SPACE); | ||
| sb.append(FNS_BLOB_USER_AGENT_IDENTIFIER); |
Contributor
There was a problem hiding this comment.
sb.append(SEMICOLON)
.append(SINGLE_WHITE_SPACE)
.append(FNS_BLOB_USER_AGENT_IDENTIFIER);
| // Test to verify the unique identifier in user agent string for FNS-Blob accounts | ||
| public void verifyUserAgentForFNSBlob() throws Exception { | ||
| assumeHnsDisabled(); | ||
| Assume.assumeTrue(getAbfsServiceType() == AbfsServiceType.BLOB); |
Contributor
There was a problem hiding this comment.
Please use assumeBlobServiceType()
| public void verifyUserAgentForFNSBlob() throws Exception { | ||
| assumeHnsDisabled(); | ||
| Assume.assumeTrue(getAbfsServiceType() == AbfsServiceType.BLOB); | ||
| final AzureBlobFileSystem fs = getFileSystem(getRawConfiguration()); |
Contributor
There was a problem hiding this comment.
Just getFileSystem() is also enough incase there is no other config you want to add in the raw configuration.
Contributor
Author
There was a problem hiding this comment.
Makes sense, taken
| // does not contain the FNS identifier. | ||
| public void verifyUserAgentForDFS() throws Exception { | ||
| Assume.assumeTrue(getAbfsServiceType() == AbfsServiceType.DFS); | ||
| final AzureBlobFileSystem fs = getFileSystem(getRawConfiguration()); |
|
🎊 +1 overall
This message was automatically generated. |
anmolanmol1234
approved these changes
Jun 30, 2025
manika137
added a commit
to ABFSDriver/AbfsHadoop
that referenced
this pull request
Jul 8, 2025
…o Get Telemetry for FNS-Blob (apache#7713) Contributed by Manika Joshi.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
JIRA: https://issues.apache.org/jira/browse/HADOOP-19575
Adding a unique identifier in the user agent for FNS-Blob case for telemetry purposes.
How was this patch tested?
Test suite was run- the results of which are added in the comments below