From 1c5b5ffcf5de8edeb28372f3c828dc0478e700ae Mon Sep 17 00:00:00 2001
From: Bryan Beaudreault <bbeaudreault@apache.org>
Date: Thu, 25 Jan 2024 09:30:47 -0500
Subject: [PATCH] HBASE-28315 Remove noisy WARN from trying to construct
 MetricsServlet (#5651)

Signed-off-by: Duo Zhang <zhangduo@apache.org>
---
 .../apache/hadoop/hbase/http/HttpServer.java  | 19 ++++++++++++-------
 .../hadoop/hbase/http/ServletConfig.java      | 16 +++++++++++++---
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java
index 6bbbc5608a0d..d5af8df1c7fd 100644
--- a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java
+++ b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java
@@ -171,7 +171,9 @@ public class HttpServer implements FilterContainer {
       .put("jmx",
         new ServletConfig("jmx", "/jmx", "org.apache.hadoop.hbase.http.jmx.JMXJsonServlet"))
       .put("metrics",
-        new ServletConfig("metrics", "/metrics", "org.apache.hadoop.metrics.MetricsServlet"))
+        // MetricsServlet is deprecated in hadoop 2.8 and removed in 3.0. We shouldn't expect it,
+        // so pass false so that we don't create a noisy warn during instantiation.
+        new ServletConfig("metrics", "/metrics", "org.apache.hadoop.metrics.MetricsServlet", false))
       .put("prometheus", new ServletConfig("prometheus", "/prometheus",
         "org.apache.hadoop.hbase.http.prometheus.PrometheusHadoopServlet"))
       .build();
@@ -846,16 +848,19 @@ protected void addDefaultServlets(ContextHandlerCollection contexts, Configurati
     /* register metrics servlets */
     String[] enabledServlets = conf.getStrings(METRIC_SERVLETS_CONF_KEY, METRICS_SERVLETS_DEFAULT);
     for (String enabledServlet : enabledServlets) {
-      try {
-        ServletConfig servletConfig = METRIC_SERVLETS.get(enabledServlet);
-        if (servletConfig != null) {
+      ServletConfig servletConfig = METRIC_SERVLETS.get(enabledServlet);
+      if (servletConfig != null) {
+        try {
           Class<?> clz = Class.forName(servletConfig.getClazz());
           addPrivilegedServlet(servletConfig.getName(), servletConfig.getPathSpec(),
             clz.asSubclass(HttpServlet.class));
+        } catch (Exception e) {
+          if (servletConfig.isExpected()) {
+            // metrics are not critical to read/write, so an exception here shouldn't be fatal
+            // if the class was expected we should warn though
+            LOG.warn("Couldn't register the servlet " + enabledServlet, e);
+          }
         }
-      } catch (Exception e) {
-        /* shouldn't be fatal, so warn the user about it */
-        LOG.warn("Couldn't register the servlet " + enabledServlet, e);
       }
     }
   }
diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/ServletConfig.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/ServletConfig.java
index befe60957605..366dbfd9f228 100644
--- a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/ServletConfig.java
+++ b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/ServletConfig.java
@@ -23,14 +23,20 @@
 
 @InterfaceAudience.Private
 class ServletConfig {
-  private String name;
-  private String pathSpec;
-  private String clazz;
+  private final String name;
+  private final String pathSpec;
+  private final String clazz;
+  private final boolean expected;
 
   public ServletConfig(String name, String pathSpec, String clazz) {
+    this(name, pathSpec, clazz, true);
+  }
+
+  public ServletConfig(String name, String pathSpec, String clazz, boolean expected) {
     this.name = name;
     this.pathSpec = pathSpec;
     this.clazz = clazz;
+    this.expected = expected;
   }
 
   public String getName() {
@@ -44,4 +50,8 @@ public String getPathSpec() {
   public String getClazz() {
     return clazz;
   }
+
+  public boolean isExpected() {
+    return expected;
+  }
 }