From bfcecb56b0221dc330f7fe0d23341904ac5b0341 Mon Sep 17 00:00:00 2001 From: K0K0V0K <109747532+K0K0V0K@users.noreply.github.com> Date: Thu, 9 Nov 2023 10:14:14 +0100 Subject: [PATCH] HADOOP-18954. Filter NaN values from JMX json interface. (#6229). Reviewed-by: Ferenc Erdelyi Signed-off-by: He Xiaoqiao --- .../fs/CommonConfigurationKeysPublic.java | 8 +++ .../org/apache/hadoop/http/HttpServer2.java | 15 ++++- .../org/apache/hadoop/jmx/JMXJsonServlet.java | 51 ++++++++++----- .../hadoop/jmx/JMXJsonServletNaNFiltered.java | 49 ++++++++++++++ .../src/main/resources/core-default.xml | 12 ++++ .../apache/hadoop/jmx/TestJMXJsonServlet.java | 9 ++- .../jmx/TestJMXJsonServletNaNFiltered.java | 65 +++++++++++++++++++ .../metrics2/lib/TestMutableMetrics.java | 2 + 8 files changed, 190 insertions(+), 21 deletions(-) create mode 100644 hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServletNaNFiltered.java create mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServletNaNFiltered.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java index 006144e64ad15..24a3167b3db2d 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java @@ -1076,5 +1076,13 @@ public class CommonConfigurationKeysPublic { public static final String IPC_SERVER_METRICS_UPDATE_RUNNER_INTERVAL = "ipc.server.metrics.update.runner.interval"; public static final int IPC_SERVER_METRICS_UPDATE_RUNNER_INTERVAL_DEFAULT = 5000; + + /** + * @see + * + * core-default.xml + */ + public static final String JMX_NAN_FILTER = "hadoop.http.jmx.nan-filter.enabled"; + public static final boolean JMX_NAN_FILTER_DEFAULT = false; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index 515148e929817..d4bfa41b21c4c 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -56,6 +56,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.hadoop.classification.VisibleForTesting; +import org.apache.hadoop.jmx.JMXJsonServletNaNFiltered; import org.apache.hadoop.util.Preconditions; import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableMap; import com.sun.jersey.spi.container.servlet.ServletContainer; @@ -117,6 +118,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.JMX_NAN_FILTER; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.JMX_NAN_FILTER_DEFAULT; + /** * Create a Jetty embedded server to answer http requests. The primary goal is * to serve up status information for the server. There are three contexts: @@ -785,7 +789,7 @@ private void initializeWebServer(String name, String hostName, } } - addDefaultServlets(); + addDefaultServlets(conf); addPrometheusServlet(conf); addAsyncProfilerServlet(contexts, conf); } @@ -976,12 +980,17 @@ private void setContextAttributes(ServletContextHandler context, /** * Add default servlets. + * @param configuration the hadoop configuration */ - protected void addDefaultServlets() { + protected void addDefaultServlets(Configuration configuration) { // set up default servlets addServlet("stacks", "/stacks", StackServlet.class); addServlet("logLevel", "/logLevel", LogLevel.Servlet.class); - addServlet("jmx", "/jmx", JMXJsonServlet.class); + addServlet("jmx", "/jmx", + configuration.getBoolean(JMX_NAN_FILTER, JMX_NAN_FILTER_DEFAULT) + ? JMXJsonServletNaNFiltered.class + : JMXJsonServlet.class + ); addServlet("conf", "/conf", ConfServlet.class); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServlet.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServlet.java index 85f2d2828562d..f089db502783e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServlet.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServlet.java @@ -17,12 +17,12 @@ package org.apache.hadoop.jmx; -import com.fasterxml.jackson.core.JsonFactory; -import com.fasterxml.jackson.core.JsonGenerator; -import org.apache.hadoop.http.HttpServer2; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - +import java.io.IOException; +import java.io.PrintWriter; +import java.lang.management.ManagementFactory; +import java.lang.reflect.Array; +import java.util.Iterator; +import java.util.Set; import javax.management.AttributeNotFoundException; import javax.management.InstanceNotFoundException; import javax.management.IntrospectionException; @@ -42,12 +42,14 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.io.PrintWriter; -import java.lang.management.ManagementFactory; -import java.lang.reflect.Array; -import java.util.Iterator; -import java.util.Set; + +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonGenerator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.commons.lang3.NotImplementedException; +import org.apache.hadoop.http.HttpServer2; /* * This servlet is based off of the JMXProxyServlet from Tomcat 7.0.14. It has @@ -136,6 +138,7 @@ public class JMXJsonServlet extends HttpServlet { * Json Factory to create Json generators for write objects in json format */ protected transient JsonFactory jsonFactory; + /** * Initialize this servlet. */ @@ -386,10 +389,10 @@ private void writeAttribute(JsonGenerator jg, ObjectName oname, MBeanAttributeIn private void writeAttribute(JsonGenerator jg, String attName, Object value) throws IOException { jg.writeFieldName(attName); - writeObject(jg, value); + writeObject(jg, value, attName); } - private void writeObject(JsonGenerator jg, Object value) throws IOException { + private void writeObject(JsonGenerator jg, Object value, String attName) throws IOException { if(value == null) { jg.writeNull(); } else { @@ -399,9 +402,11 @@ private void writeObject(JsonGenerator jg, Object value) throws IOException { int len = Array.getLength(value); for (int j = 0; j < len; j++) { Object item = Array.get(value, j); - writeObject(jg, item); + writeObject(jg, item, attName); } jg.writeEndArray(); + } else if (extraCheck(value)) { + extraWrite(value, attName, jg); } else if(value instanceof Number) { Number n = (Number)value; jg.writeNumber(n.toString()); @@ -421,7 +426,7 @@ private void writeObject(JsonGenerator jg, Object value) throws IOException { TabularData tds = (TabularData)value; jg.writeStartArray(); for(Object entry : tds.values()) { - writeObject(jg, entry); + writeObject(jg, entry, attName); } jg.writeEndArray(); } else { @@ -429,4 +434,18 @@ private void writeObject(JsonGenerator jg, Object value) throws IOException { } } } + + /** + * In case you need to modify the logic, how java objects transforms to json, + * you can overwrite this method to return true in case special handling + * @param value the object what should be judged + * @return true, if it needs special transformation + */ + protected boolean extraCheck(Object value) { + return false; + } + + protected void extraWrite(Object value, String attName, JsonGenerator jg) throws IOException { + throw new NotImplementedException(); + } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServletNaNFiltered.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServletNaNFiltered.java new file mode 100644 index 0000000000000..40e3c1a168f9d --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServletNaNFiltered.java @@ -0,0 +1,49 @@ +/** + * 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.jmx; + +import java.io.IOException; +import java.util.Objects; + +import com.fasterxml.jackson.core.JsonGenerator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * For example in case of MutableGauge we are using numbers, + * but not implementing Number interface, + * so we skip class check here because we can not be sure NaN values are wrapped + * with classes which implements the Number interface + */ +public class JMXJsonServletNaNFiltered extends JMXJsonServlet { + + private static final Logger LOG = + LoggerFactory.getLogger(JMXJsonServletNaNFiltered.class); + + @Override + protected boolean extraCheck(Object value) { + return Objects.equals("NaN", Objects.toString(value).trim()); + } + + @Override + protected void extraWrite(Object value, String attName, JsonGenerator jg) throws IOException { + LOG.debug("The {} attribute with value: {} was identified as NaN " + + "and will be replaced with 0.0", attName, value); + jg.writeNumber(0.0); + } +} diff --git a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml index 31d980353bf26..c86fd8b98609b 100644 --- a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml +++ b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml @@ -65,6 +65,18 @@ + + hadoop.http.jmx.nan-filter.enabled + false + + The REST API of the JMX interface can return with NaN values + if the attribute represent a 0.0/0.0 value. + Some JSON parser by default can not parse json attributes like foo:NaN. + If this filter is enabled the NaN values will be converted to 0.0 values, + to make json parse less complicated. + + + diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServlet.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServlet.java index 035090ef65b72..ba7de6f437ee5 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServlet.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServlet.java @@ -62,10 +62,15 @@ public static void assertReFind(String re, String value) { result = readOutput(new URL(baseUrl, "/jmx?qry=java.lang:type=Memory")); assertReFind("\"name\"\\s*:\\s*\"java.lang:type=Memory\"", result); assertReFind("\"modelerType\"", result); - + + System.setProperty("THE_TEST_OF_THE_NAN_VALUES", String.valueOf(Float.NaN)); result = readOutput(new URL(baseUrl, "/jmx")); assertReFind("\"name\"\\s*:\\s*\"java.lang:type=Memory\"", result); - + assertReFind( + "\"key\"\\s*:\\s*\"THE_TEST_OF_THE_NAN_VALUES\"\\s*,\\s*\"value\"\\s*:\\s*\"NaN\"", + result + ); + // test to get an attribute of a mbean result = readOutput(new URL(baseUrl, "/jmx?get=java.lang:type=Memory::HeapMemoryUsage")); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServletNaNFiltered.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServletNaNFiltered.java new file mode 100644 index 0000000000000..52a52be80a35c --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/jmx/TestJMXJsonServletNaNFiltered.java @@ -0,0 +1,65 @@ +/** + * 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.jmx; + +import java.net.URL; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.http.HttpServer2; +import org.apache.hadoop.http.HttpServerFunctionalTest; + +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.JMX_NAN_FILTER; + +public class TestJMXJsonServletNaNFiltered extends HttpServerFunctionalTest { + private static HttpServer2 server; + private static URL baseUrl; + + @BeforeClass public static void setup() throws Exception { + Configuration configuration = new Configuration(); + configuration.setBoolean(JMX_NAN_FILTER, true); + server = createTestServer(configuration); + server.start(); + baseUrl = getServerURL(server); + } + + @AfterClass public static void cleanup() throws Exception { + server.stop(); + } + + public static void assertReFind(String re, String value) { + Pattern p = Pattern.compile(re); + Matcher m = p.matcher(value); + assertTrue("'"+p+"' does not match "+value, m.find()); + } + + @Test public void testQuery() throws Exception { + System.setProperty("THE_TEST_OF_THE_NAN_VALUES", String.valueOf(Float.NaN)); + String result = readOutput(new URL(baseUrl, "/jmx")); + assertReFind("\"name\"\\s*:\\s*\"java.lang:type=Memory\"", result); + assertReFind( + "\"key\"\\s*:\\s*\"THE_TEST_OF_THE_NAN_VALUES\"\\s*,\\s*\"value\"\\s*:\\s*0.0", + result + ); + } +} diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMutableMetrics.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMutableMetrics.java index 2cc3d706f0c20..1ebc0cbdbf23d 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMutableMetrics.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMutableMetrics.java @@ -634,5 +634,7 @@ public void testMutableGaugeFloat() { assertEquals(3.2f, mgf.value(), 0.0); mgf.incr(); assertEquals(4.2f, mgf.value(), 0.0); + mgf.set(Float.NaN); + assertEquals(Float.NaN, mgf.value(), 0.0); } }