From bf9106c812c146098d3e277df10535c8ee5093bb Mon Sep 17 00:00:00 2001
From: Neil <wxzn23@foxmail.com>
Date: Fri, 1 Oct 2021 15:35:14 +0800
Subject: [PATCH] HDFS-16242. JournalMetrics should add JournalId MetricTag.
 (#3494)

Contributed by Max Xie
---
 .../apache/hadoop/test/MetricsAsserts.java    | 25 +++++++++++++++++++
 .../hdfs/qjournal/server/JournalMetrics.java  |  6 +++++
 .../hdfs/qjournal/server/TestJournalNode.java | 23 +++++++++++++++--
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MetricsAsserts.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MetricsAsserts.java
index eb8d9389947..53b0e8b38a2 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MetricsAsserts.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MetricsAsserts.java
@@ -401,4 +401,29 @@ public class MetricsAsserts {
           geq(0l));
     }
   }
+
+  /**
+   * Assert a tag of metric as expected.
+   * @param name  of the metric tag
+   * @param expected  value of the metric tag
+   * @param rb  the record builder mock used to getMetrics
+   */
+  public static void assertTag(String name, String expected,
+      MetricsRecordBuilder rb) {
+    Assert.assertEquals("Bad Tag for metric " + name,
+        expected, getStringTag(name, rb));
+  }
+
+  /**
+   * get the value tag for the metric.
+   * @param name  of the metric tag
+   * @param rb value of the metric tag
+   * @return the value tag for the metric
+   */
+  public static String getStringTag(String name, MetricsRecordBuilder rb) {
+    ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
+    verify(rb).tag(eqName(info(name, "")), captor.capture());
+    checkCaptured(captor, name);
+    return captor.getValue();
+  }
 }
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalMetrics.java
index 3499da98dca..574ca00350a 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalMetrics.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalMetrics.java
@@ -20,6 +20,7 @@ package org.apache.hadoop.hdfs.qjournal.server;
 import java.io.IOException;
 
 import org.apache.hadoop.metrics2.annotation.Metric;
+import org.apache.hadoop.metrics2.annotation.Metric.Type;
 import org.apache.hadoop.metrics2.annotation.Metrics;
 import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
 import org.apache.hadoop.metrics2.lib.MetricsRegistry;
@@ -99,6 +100,11 @@ class JournalMetrics {
     return "Journal-" + journal.getJournalId();
   }
 
+  @Metric(value={"JournalId", "Current JournalId"}, type=Type.TAG)
+  public String getJournalId() {
+    return journal.getJournalId();
+  }
+
   @Metric("Current writer's epoch")
   public long getLastWriterEpoch() {
     try {
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java
index 6e117b7687a..a8d4442bdff 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java
@@ -97,7 +97,8 @@ public class TestJournalNode {
       conf.set(DFSConfigKeys.DFS_JOURNALNODE_EDITS_DIR_KEY,
           editsDir.getAbsolutePath());
     } else if (testName.getMethodName().equals(
-        "testJournalDefaultDirForOneNameSpace")) {
+        "testJournalDefaultDirForOneNameSpace") ||
+        testName.getMethodName().equals("testJournalMetricTags")) {
       FileUtil.fullyDelete(new File(DFSConfigKeys
           .DFS_JOURNALNODE_EDITS_DIR_DEFAULT));
       setFederationConf();
@@ -151,7 +152,8 @@ public class TestJournalNode {
         testName.getMethodName().equals(
             "testJournalCommonDirAcrossNameSpace") ||
         testName.getMethodName().equals(
-            "testJournalDefaultDirForOneNameSpace")) {
+            "testJournalDefaultDirForOneNameSpace") ||
+        testName.getMethodName().equals("testJournalMetricTags")) {
       Collection<String> nameServiceIds = DFSUtilClient.getNameServiceIds(conf);
       for(String nsId: nameServiceIds) {
         journalId = "test-journalid-" + nsId;
@@ -240,6 +242,23 @@ public class TestJournalNode {
         File.separator + jid);
     assertEquals(editsDir.toString(), journalStorage.getRoot().toString());
   }
+
+  @Test(timeout=100000)
+  public void testJournalMetricTags() {
+    setupStaticHostResolution(2, "journalnode");
+    String jid = "test-journalid-ns1";
+    Journal nsJournal = jn.getJournal(jid);
+    MetricsRecordBuilder metrics = MetricsAsserts.getMetrics(
+        nsJournal.getMetrics().getName());
+    MetricsAsserts.assertTag("JournalId", jid, metrics);
+
+    jid = "test-journalid-ns2";
+    nsJournal = jn.getJournal(jid);
+    metrics = MetricsAsserts.getMetrics(
+        nsJournal.getMetrics().getName());
+    MetricsAsserts.assertTag("JournalId", jid, metrics);
+  }
+
   @Test(timeout=100000)
   public void testJournal() throws Exception {
     MetricsRecordBuilder metrics = MetricsAsserts.getMetrics(