HADOOP-18954. Filter NaN values from JMX json interface. (#6229).

Reviewed-by: Ferenc Erdelyi
Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>

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
+   * <a href="{@docRoot}/../hadoop-project-dist/hadoop-common/core-default.xml">
+   * core-default.xml</a>
+   */
+  public static final String JMX_NAN_FILTER = "hadoop.http.jmx.nan-filter.enabled";
+  public static final boolean JMX_NAN_FILTER_DEFAULT = false;
 }
 

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);
   }
 

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 java.io.IOException;
-import com.fasterxml.jackson.core.JsonGenerator;
+import java.io.PrintWriter;
-import org.apache.hadoop.http.HttpServer2;
+import java.lang.management.ManagementFactory;
-import org.slf4j.Logger;
+import java.lang.reflect.Array;
-import org.slf4j.LoggerFactory;
+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 com.fasterxml.jackson.core.JsonFactory;
-import java.lang.management.ManagementFactory;
+import com.fasterxml.jackson.core.JsonGenerator;
-import java.lang.reflect.Array;
+import org.slf4j.Logger;
-import java.util.Iterator;
+import org.slf4j.LoggerFactory;
-import java.util.Set;
+
+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();
+  }
 }

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);
+  }
+}

hadoop-common-project/hadoop-common/src/main/resources/core-default.xml

@@ -65,6 +65,18 @@
   </description>
 </property>
 
+<property>
+  <name>hadoop.http.jmx.nan-filter.enabled</name>
+  <value>false</value>
+  <description>
+    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.
+  </description>
+</property>
+
 <!--- security properties -->
 
 <property>

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"));

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
+    );
+  }
+}

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);
   }
 }