ndimiduk commented on code in PR #4691:
URL: https://github.com/apache/hbase/pull/4691#discussion_r953603702
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/prometheus/PrometheusHadoopServlet.java:
##
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.http.prometheus;
+
+import com.google.errorprone.annotations.RestrictedApi;
+import java.io.IOException;
+import java.io.Writer;
+import java.util.Collection;
+import java.util.regex.Pattern;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.metrics2.AbstractMetric;
+import org.apache.hadoop.metrics2.MetricType;
+import org.apache.hadoop.metrics2.MetricsRecord;
+import org.apache.hadoop.metrics2.MetricsTag;
+import org.apache.hadoop.metrics2.impl.MetricsExportHelper;
+import org.apache.yetus.audience.InterfaceAudience;
+
+@InterfaceAudience.Private
+public class PrometheusHadoopServlet extends HttpServlet {
Review Comment:
Why is this called a "Hadoop" servlet?
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/prometheus/PrometheusHadoopServlet.java:
##
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.http.prometheus;
+
+import com.google.errorprone.annotations.RestrictedApi;
+import java.io.IOException;
+import java.io.Writer;
+import java.util.Collection;
+import java.util.regex.Pattern;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.metrics2.AbstractMetric;
+import org.apache.hadoop.metrics2.MetricType;
+import org.apache.hadoop.metrics2.MetricsRecord;
+import org.apache.hadoop.metrics2.MetricsTag;
+import org.apache.hadoop.metrics2.impl.MetricsExportHelper;
+import org.apache.yetus.audience.InterfaceAudience;
+
+@InterfaceAudience.Private
+public class PrometheusHadoopServlet extends HttpServlet {
Review Comment:
Oh no, why was this implemented as a raw servlet :'(
We have Jersey. There is absolutely no reason to implement a servlet by hand.
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java:
##
@@ -784,6 +787,22 @@ protected void addDefaultServlets(ContextHandlerCollection
contexts, Configurati
LOG.info("ASYNC_PROFILER_HOME environment variable and
async.profiler.home system property "
+ "not specified. Disabling /prof endpoint.");
}
+
+/* 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) {
+ Class clz = Class.forName(servletConfig.getClazz());
+ addPrivilegedServlet(servletConfig.getName(),
servletConfig.getPathSpec(),
+clz.asSubclass(HttpServlet.class));
+}
+ } catch (Exception e) {
+/* shouldn't be fatal, so warn the user about it */
Review Comment:
I'm surprised that having metrics be optional was accepted by review. Before
this change, it was impossible to run an HBase processes
