anmolanmol1234 commented on code in PR #8137:
URL: https://github.com/apache/hadoop/pull/8137#discussion_r2650743802
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java:
##########
@@ -876,6 +894,308 @@ public void testIsNonEmptyDirectory() throws IOException {
false, 1, true);
}
+ /**
+ * Test to verify that in case metric account is not set,
+ * metric collection is enabled with default metric format
+ * and account url.
+ *
+ * @throws Exception in case of any failure
+ */
+ @Test
+ public void testMetricAccountFallback() throws Exception {
+ Configuration configuration = getRawConfiguration();
+ configuration.setBoolean(
+ AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, false);
+ configuration.setBoolean(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, false);
+ configuration.unset(FS_AZURE_METRIC_ACCOUNT_NAME);
+ configuration.unset(FS_AZURE_METRIC_ACCOUNT_KEY);
+ configuration.unset(FS_AZURE_METRIC_FORMAT);
+ configuration.setBoolean(FS_AZURE_ALWAYS_USE_HTTPS, true);
+ final AzureBlobFileSystem fs = getFileSystem(configuration);
+ Assertions.assertThat(
+ fs.getAbfsStore().getAbfsConfiguration().getMetricFormat())
+ .describedAs(
+ "In case metric format is not set, metric format should "
+ + "be defaulted to internal metric format")
+ .isEqualTo(MetricFormat.INTERNAL_METRIC_FORMAT);
+
+ Assertions.assertThat(
+ fs.getAbfsStore().getClient().isMetricCollectionEnabled())
+ .describedAs(
+ "Metric collection should be enabled even if metric account is not
set")
+ .isTrue();
+
+ Assertions.assertThat(
+ fs.getAbfsStore().getClient().getAbfsCounters().toString())
+ .describedAs(
+ "AbfsCounters should not contain backoff related metrics "
+ + "as no metric is collected for backoff")
+ .doesNotContain("#BO:");
+
+ Assertions.assertThat(
+ fs.getAbfsStore().getClient().getAbfsCounters().toString())
+ .describedAs(
+ "AbfsCounters should not contain read footer related metrics "
+ + "as no metric is collected for read footer")
+ .doesNotContain("#FO:");
+
+ final URIBuilder uriBuilder = new URIBuilder();
+ uriBuilder.setScheme(FileSystemUriSchemes.HTTPS_SCHEME);
+ uriBuilder.setHost(fs.getUri().getHost());
+ uriBuilder.setPath(FORWARD_SLASH);
+ Assertions.assertThat(fs.getAbfsStore().getClient().getMetricsUrl())
+ .describedAs(
+ "In case metric account is not set, account url should be used")
+ .isEqualTo(
+ UriUtils.changeUrlFromBlobToDfs(uriBuilder.build().toURL()));
+ }
+
+ /**
+ * Test to verify that in case metric format is set to empty,
+ * metric collection is disabled.
+ *
+ * @throws Exception in case of any failure
+ */
+ @Test
+ public void testMetricCollectionWithDifferentMetricFormat() throws Exception
{
+ Configuration configuration = getRawConfiguration();
+ // Setting this configuration just to ensure there is only one call during
filesystem initialization
+ configuration.setBoolean(
+ AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, true);
+ configuration.setBoolean(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, false);
+ configuration.unset(FS_AZURE_METRIC_FORMAT);
+ configuration.setEnum(FS_AZURE_METRIC_FORMAT,
+ INTERNAL_BACKOFF_METRIC_FORMAT);
+ final AzureBlobFileSystem fs = getFileSystem(configuration);
+ int totalCalls = 1; // Filesystem initialization call
+ Assertions.assertThat(
+ fs.getAbfsStore().getClient().isMetricCollectionEnabled())
+ .describedAs("Metric collection should be enabled by default")
+ .isTrue();
+
+ Assertions.assertThat(
+ fs.getAbfsStore().getAbfsConfiguration().getMetricFormat())
+ .describedAs("Metric format should be as set in configuration")
+ .isEqualTo(INTERNAL_BACKOFF_METRIC_FORMAT);
+
+ Assertions.assertThat(
+ fs.getAbfsStore().getClient().getAbfsCounters().toString())
+ .describedAs(
+ "AbfsCounters should only contains backoff related metrics when "
+ + "metric format is internal backoff metric format")
+ .contains("#BO:");
+
+ Assertions.assertThat(
+ fs.getAbfsStore().getClient().getAbfsCounters().toString())
+ .describedAs(
+ "AbfsCounters should not contains read footer related metrics when
"
+ + "metric format is internal backoff metric format")
+ .doesNotContain("#FO:");
+
+ Assertions.assertThat(fs.getAbfsStore()
+ .getClient()
+ .getAbfsCounters()
+ .getAbfsBackoffMetrics()
+ .getMetricValue(
+ AbfsBackoffMetricsEnum.TOTAL_NUMBER_OF_REQUESTS))
+ .describedAs(
+ "Total number of requests should be 1 for filesystem
initialization")
+ .isEqualTo(totalCalls);
+
+
+ if (fs.getAbfsStore().getClient() instanceof AbfsDfsClient) {
+ intercept(FileNotFoundException.class,
+ "The specified path does not exist.",
+ () -> fs.listStatus(path("/testPath")));
+ totalCalls += 1; // listStatus call
+ } else {
+ intercept(FileNotFoundException.class,
+ "The specified blob does not exist.",
+ () -> fs.listStatus(path("/testPath")));
+ totalCalls += 2; // listStatus call makes 2 calls to the service
+ }
+
+ Assertions.assertThat(fs.getAbfsStore()
+ .getClient()
+ .getAbfsCounters()
+ .getAbfsBackoffMetrics()
+ .getMetricValue(
+ AbfsBackoffMetricsEnum.TOTAL_NUMBER_OF_REQUESTS))
+ .describedAs(
+ "Total number of requests should be 2 after listStatus")
+ .isEqualTo(totalCalls);
+ }
+
+ /**
+ * Test to verify that clientRequestId contains backoff metrics
+ * when metric format is set to internal backoff metric format.
+ *
+ * @throws Exception in case of any failure
+ */
+ @Test
+ public void testGetMetricsCallMethod() throws Exception {
+ // File system init will make few calls to the service.
+ // Backoff metrics will be collected for those calls.
+ AzureBlobFileSystem fs = getFileSystem();
+ TracingContext tracingContext = new TracingContext(
+ fs.getAbfsStore().getAbfsConfiguration().getClientCorrelationId(),
+ "test-filesystem-id", FSOperationType.TEST_OP, true,
+ TracingHeaderFormat.AGGREGATED_METRICS_FORMAT, null,
+ fs.getAbfsStore().getClient().getAbfsCounters().toString());
+
+ AbfsHttpOperation abfsHttpOperation = getAbfsClient(
+ fs.getAbfsStore()).getAbfsRestOperation(
+ AbfsRestOperationType.GetFileSystemProperties,
+ HTTP_METHOD_HEAD,
+ fs.getAbfsStore().getClient().getMetricsUrl(),
+ getTestRequestHeaders(fs.getAbfsStore().getClient()))
+ .createHttpOperation();
+ tracingContext.constructHeader(abfsHttpOperation, null,
+ EXPONENTIAL_RETRY_POLICY_ABBREVIATION);
+ assertThat(abfsHttpOperation.getClientRequestId())
+ .describedAs("ClientRequestId should be contains Backoff metrics")
Review Comment:
nit: should be containing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
