[GitHub] [hbase] mmpataki commented on a change in pull request #1814: HBASE-20904 Prometheus /metrics http endpoint for monitoring
mmpataki commented on a change in pull request #1814:
URL: https://github.com/apache/hbase/pull/1814#discussion_r444276936
##
File path:
hbase-http/src/main/java/org/apache/hadoop/hbase/http/prometheus/PrometheusHadoop2Servlet.java
##
@@ -0,0 +1,115 @@
+/*
+ * 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 java.io.IOException;
+import java.io.Writer;
+import java.util.Collection;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+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;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+
+@InterfaceAudience.Private
+public class PrometheusHadoop2Servlet extends HttpServlet {
+
+ private Logger LOG = LoggerFactory.getLogger(PrometheusHadoop2Servlet.class);
+
+ /**
+ * These 2 structures will make sure that there is only one thread which
+ * reads the metricRecords and produces a string which can be directly
consumed
+ * to write on socket / any writer.
+ */
+ private final AtomicInteger amIProducer = new AtomicInteger(0);
+ private final Object waitingRoom = new Object();
+
+ /* reference to generated metrics buffer */
+ private final AtomicReference pMetricsBuffer = new
AtomicReference<>();
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+throws IOException {
+writeMetrics(resp.getWriter());
+ }
+
+ @VisibleForTesting
+ void writeMetrics(Writer writer) throws IOException {
+if (amIProducer.compareAndSet(0, 1)) {
+ try {
+renderMetrics(MetricsExportHelper.export());
+ } finally {
+amIProducer.set(0);
+synchronized (waitingRoom) {
+ waitingRoom.notifyAll();
+}
+ }
+} else {
+ synchronized (waitingRoom) {
+try {
+ waitingRoom.wait(5000L);
Review comment:
It's not worth waiting for more than X seconds to get the metrics. (X
should be 1s) if we can't produce in 1s we should/can return the metrics which
were produced by earlier producer. (1s old metrics are acceptable?)
It's guaranteed that the old metrics will be there when we come out of this
wait.
Once the thread comes out of wait, it tries to get back the lock which is
unnecessary is there way to avoid it?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hbase] mmpataki commented on a change in pull request #1814: HBASE-20904 Prometheus /metrics http endpoint for monitoring
mmpataki commented on a change in pull request #1814:
URL: https://github.com/apache/hbase/pull/1814#discussion_r443990900
##
File path:
hbase-http/src/main/java/org/apache/hadoop/hbase/http/prom/PrometheusMetricsSink.java
##
@@ -0,0 +1,103 @@
+/**
+ * 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.prom;
+
+import org.apache.commons.configuration2.SubsetConfiguration;
+import org.apache.hadoop.metrics2.AbstractMetric;
+import org.apache.hadoop.metrics2.MetricType;
+import org.apache.hadoop.metrics2.MetricsRecord;
+import org.apache.hadoop.metrics2.MetricsSink;
+import org.apache.hadoop.metrics2.MetricsTag;
+
+import java.io.IOException;
+import java.io.Writer;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * Metrics sink for prometheus exporter.
+ *
+ * Stores the metric data in-memory and return with it on request.
+ */
+@InterfaceAudience.Private
+public class PrometheusMetricsSink implements MetricsSink {
+
+ /**
+ * Cached output lines for each metrics.
+ */
+ private Map metricLines = new HashMap<>();
Review comment:
Not using this anymore.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hbase] mmpataki commented on a change in pull request #1814: HBASE-20904 Prometheus /metrics http endpoint for monitoring
mmpataki commented on a change in pull request #1814:
URL: https://github.com/apache/hbase/pull/1814#discussion_r443991072
##
File path:
hbase-http/src/main/java/org/apache/hadoop/hbase/http/prom/PrometheusMetricsSink.java
##
@@ -0,0 +1,103 @@
+/**
+ * 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.prom;
+
+import org.apache.commons.configuration2.SubsetConfiguration;
+import org.apache.hadoop.metrics2.AbstractMetric;
+import org.apache.hadoop.metrics2.MetricType;
+import org.apache.hadoop.metrics2.MetricsRecord;
+import org.apache.hadoop.metrics2.MetricsSink;
+import org.apache.hadoop.metrics2.MetricsTag;
+
+import java.io.IOException;
+import java.io.Writer;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * Metrics sink for prometheus exporter.
+ *
+ * Stores the metric data in-memory and return with it on request.
+ */
+@InterfaceAudience.Private
+public class PrometheusMetricsSink implements MetricsSink {
Review comment:
Not using this anymore because HADOOP-17081
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hbase] mmpataki commented on a change in pull request #1814: HBASE-20904 Prometheus /metrics http endpoint for monitoring
mmpataki commented on a change in pull request #1814: URL: https://github.com/apache/hbase/pull/1814#discussion_r443990799 ## File path: hbase-http/src/main/java/org/apache/hadoop/hbase/http/prom/PrometheusMetricsSink.java ## @@ -0,0 +1,103 @@ +/** + * 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.prom; + +import org.apache.commons.configuration2.SubsetConfiguration; +import org.apache.hadoop.metrics2.AbstractMetric; +import org.apache.hadoop.metrics2.MetricType; +import org.apache.hadoop.metrics2.MetricsRecord; +import org.apache.hadoop.metrics2.MetricsSink; +import org.apache.hadoop.metrics2.MetricsTag; + +import java.io.IOException; +import java.io.Writer; +import java.util.HashMap; +import java.util.Map; +import java.util.regex.Pattern; + +import org.apache.yetus.audience.InterfaceAudience; + +/** + * Metrics sink for prometheus exporter. + * + * Stores the metric data in-memory and return with it on request. Review comment: It emits all the metrics available in the metrics system 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] mmpataki commented on a change in pull request #1814: HBASE-20904 Prometheus /metrics http endpoint for monitoring
mmpataki commented on a change in pull request #1814: URL: https://github.com/apache/hbase/pull/1814#discussion_r436029458 ## File path: hbase-common/src/main/resources/hbase-default.xml ## @@ -1727,6 +1727,16 @@ possible configurations would overwhelm and obscure the important. ThreadPool. + +hbase.http.enable.prometheus.servlets Review comment: @busbey thank you for the review. I believe, I should have started with a discussion about this feature on how to implement it. How is it done in the hbase project? Do we use mailing lists or jira for discussing on implementations? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] mmpataki commented on a change in pull request #1814: HBASE-20904 Prometheus /metrics http endpoint for monitoring
mmpataki commented on a change in pull request #1814: URL: https://github.com/apache/hbase/pull/1814#discussion_r436029458 ## File path: hbase-common/src/main/resources/hbase-default.xml ## @@ -1727,6 +1727,16 @@ possible configurations would overwhelm and obscure the important. ThreadPool. + +hbase.http.enable.prometheus.servlets Review comment: @busbey thank you for the review. I believe, I should have started with a discussion about this feature. How is it done in the hbase project? Do we use mailing lists or jira for discussing on implementations? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] mmpataki commented on a change in pull request #1814: HBASE-20904 Prometheus /metrics http endpoint for monitoring
mmpataki commented on a change in pull request #1814: URL: https://github.com/apache/hbase/pull/1814#discussion_r432997789 ## File path: hbase-common/src/main/resources/hbase-default.xml ## @@ -1727,6 +1727,15 @@ possible configurations would overwhelm and obscure the important. ThreadPool. + +hbase.http.enable.prometheus.servlets +false + + Enable prometheus servlets /prom and /prom2 for prometheus based monitoring. + /prom is based on new HBase metrics API and all metrics are not exported for now. + /prom2 is based on the old hadoop2 metrics API and has all the metrics. Review comment: @saintstack > Why not /prom and /prom-old ? Why have the old at all (maybe you want to backport this)? When /prom and /prom2, w/o close reading, users will think they need to read from /prom2 because 2 is a later number than no number? > > Also, can you say more on old vs new metrics. I'm not clear. Would be good to also clean up the description in here so clear to the casual reader. > > Thakns for adding the flag. That makes sense, I will change it to /prom-old or similar and fix the documentation of the flag. (Answers to other questions are in the next block) > Is 'new metrics' our use of 'hadoop metrics', a move we made years ago? What versions in release do the old way of metrics? Thanks. HBASE-9774 brought in hbase-native metrics collection for the coprocessors and a decision was made to to use this API to record all the other metrics as well (hbase-metrics-api / README.txt). I am referring to this as "new API'. Now all the work to consume this new API isn't done so we still have to depend on the "old", "hadoop2" based metric collection. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
