Merge pull request #32 from evolvedbinary/6.x.x/hotfix/monex-monitoring-and-profiling

[6.x.x] Fix issues with Monitoring and Profiling as used by tools like Monex

Author: adamretter

exist-core/pom.xml

@@ -914,7 +914,7 @@
                                 <include>src/test/java/org/exist/xquery/ForwardReferenceTest.java</include>
                                 <include>src/main/java/org/exist/xquery/FunctionFactory.java</include>
                                 <include>src/main/java/org/exist/xquery/Optimizer.java</include>
-                                <include>src/main/java/org/exist/xquery/PerformanceStatsImpl.java</include>
+                                <include>src/main/java/org/exist/xquery/PerformanceStats.java</include>
                                 <include>src/main/java/org/exist/xquery/TryCatchExpression.java</include>
                                 <include>src/main/java/org/exist/xquery/UserDefinedFunction.java</include>
                                 <include>src/main/java/org/exist/xquery/XPathUtil.java</include>
@@ -1256,7 +1256,7 @@
                                 <exclude>src/main/java/org/exist/xquery/Materializable.java</exclude>
                                 <exclude>src/main/java/org/exist/xquery/NameTest.java</exclude>
                                 <exclude>src/main/java/org/exist/xquery/Optimizer.java</exclude>
-                                <exclude>src/main/java/org/exist/xquery/PerformanceStatsImpl.java</exclude>
+                                <exclude>src/main/java/org/exist/xquery/PerformanceStats.java</exclude>
                                 <exclude>src/main/java/org/exist/xquery/TryCatchExpression.java</exclude>
                                 <exclude>src/main/java/org/exist/xquery/UserDefinedFunction.java</exclude>
                                 <exclude>src/test/java/org/exist/xquery/WatchdogTest.java</exclude>

exist-core/src/main/java/org/exist/management/client/JMXServlet.java

@@ -56,6 +56,9 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.attribute.PosixFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
 import java.util.HashSet;
 import java.util.Properties;
 import java.util.Set;
@@ -116,7 +119,7 @@ public class JMXServlet extends HttpServlet {
     }
 
     private JMXtoXML client;
-    private final Set<String> localhostAddresses = new HashSet<>();
+    private final Set<String> serverAddresses = new HashSet<>();
 
     private Path dataDir;
     private Path tokenFile;
@@ -128,11 +131,15 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
         // Verify if request is from localhost or if user has specific servlet/container managed role.
         if (isFromLocalHost(request)) {
             // Localhost is always authorized to access
-            LOG.debug("Local access granted");
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Local access granted");
+            }
 
         } else if (hasSecretToken(request, getToken())) {
             // Correct token is provided
-            LOG.debug("Correct token provided by {}", request.getRemoteHost());
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Correct token provided by {}", request.getRemoteHost());
+            }
 
         } else {
             // Check if user is already authorized, e.g. via MONEX allow user too
@@ -218,7 +225,7 @@ public void init(ServletConfig config) throws ServletException {
         client.connect();
 
         // Register all known localhost addresses
-        registerLocalHostAddresses();
+        registerServerAddresses();
 
         // Get directory for token file
         final String jmxDataDir = client.getDataDir();
@@ -232,34 +239,47 @@ public void init(ServletConfig config) throws ServletException {
         }
 
         // Setup token and tokenfile
-        obtainTokenFileReference();
-
-        LOG.info("JMXservlet token: {}", getToken());
+        if (tokenFile == null) {
+            tokenFile = dataDir.resolve(TOKEN_FILE);
+            LOG.info("Token file:  {}", tokenFile.toAbsolutePath().toAbsolutePath());
+        }
 
+        // NOTE(AR) make sure to create the token in init when the servlet is loaded at startup so that it is present for Monex
+        final String token = getToken();
+        LOG.info("JMXServlet token: {}", token);
     }
 
     /**
-     * Register all known IP-addresses for localhost.
+     * Register all known IP-addresses for server.
      */
-    void registerLocalHostAddresses() {
-        // The external IP address of the server
+    void registerServerAddresses() {
+        // The IPv4 address of the loopback interface of the server - 127.0.0.1 on Windows/Linux/macOS, or 127.0.1.1 on Debian/Ubuntu
+        try {
+            serverAddresses.add(InetAddress.getLocalHost().getHostAddress());
+        } catch (final UnknownHostException ex) {
+            LOG.warn("Unable to get loopback IP address for localhost: {}", ex.getMessage());
+        }
+
+        // Any additional IPv4 and IPv6 addresses associated with the loopback interface of the server
         try {
-            localhostAddresses.add(InetAddress.getLocalHost().getHostAddress());
-        } catch (UnknownHostException ex) {
-            LOG.warn("Unable to get HostAddress for localhost: {}", ex.getMessage());
+            for (final InetAddress loopBackAddress : InetAddress.getAllByName("localhost")) {
+                serverAddresses.add(loopBackAddress.getHostAddress());
+            }
+        } catch (final UnknownHostException ex) {
+            LOG.warn("Unable to retrieve additional loopback IP addresses for localhost: {}", ex.getMessage());
         }
 
-        // The configured Localhost addresses
+        // Any IPv4 and IPv6 addresses associated with other interfaces in the server
         try {
-            for (InetAddress address : InetAddress.getAllByName("localhost")) {
-                localhostAddresses.add(address.getHostAddress());
+            for (final InetAddress hostAddress : InetAddress.getAllByName(InetAddress.getLocalHost().getHostName())) {
+                serverAddresses.add(hostAddress.getHostAddress());
             }
-        } catch (UnknownHostException ex) {
-            LOG.warn("Unable to retrieve ipaddresses for localhost: {}", ex.getMessage());
+        } catch (final UnknownHostException ex) {
+            LOG.warn("Unable to retrieve additional interface IP addresses for localhost: {}", ex.getMessage());
         }
 
-        if (localhostAddresses.isEmpty()) {
-            LOG.error("Unable to determine addresses for localhost, jmx servlet might be disfunctional.");
+        if (serverAddresses.isEmpty()) {
+            LOG.error("Unable to determine IP addresses for localhost, JMXServlet might be dysfunctional.");
         }
     }
 
@@ -269,8 +289,13 @@ void registerLocalHostAddresses() {
      * @param request The HTTP request
      * @return TRUE if request is from LOCALHOST otherwise FALSE
      */
-    boolean isFromLocalHost(HttpServletRequest request) {
-        return localhostAddresses.contains(request.getRemoteAddr());
+    boolean isFromLocalHost(final HttpServletRequest request) {
+        String remoteAddr = request.getRemoteAddr();
+        if (remoteAddr.charAt(0) == '[') {
+            // Handle IPv6 addresses that are wrapped in []
+            remoteAddr = remoteAddr.substring(1, remoteAddr.length() - 1);
+        }
+        return serverAddresses.contains(remoteAddr);
     }
 
     /**
@@ -291,17 +316,6 @@ boolean hasSecretToken(HttpServletRequest request, String token) {
         return false;
     }
 
-    /**
-     * Obtain reference to token file
-     */
-    private void obtainTokenFileReference() {
-
-        if (tokenFile == null) {
-            tokenFile = dataDir.resolve(TOKEN_FILE);
-            LOG.info("Token file:  {}", tokenFile.toAbsolutePath().toAbsolutePath());
-        }
-    }
-
     /**
      * Get token from file, create if not existent. Data is read for each call so the file can be updated run-time.
      *
@@ -327,6 +341,13 @@ private String getToken() {
         // Create and write when needed
         if (!Files.exists(tokenFile) || token == null) {
 
+            final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString("rw-r-----");
+            try {
+                tokenFile = Files.createFile(tokenFile, PosixFilePermissions.asFileAttribute(permissions));
+            } catch (final Throwable t) {
+                LOG.warn("Unable to restrict permissions on: " + tokenFile);
+            }
+
             // Create random token
             token = UUIDGenerator.getUUIDversion4();
 

exist-core/src/main/java/org/exist/xquery/PerformanceStats.java

@@ -1,4 +1,28 @@
 /*
+ * Elemental
+ * Copyright (C) 2024, Evolved Binary Ltd
+ *
+ * [email protected]
+ * https://www.evolvedbinary.com | https://www.elemental.xyz
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; version 2.1.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * NOTE: Parts of this file contain code from 'The eXist-db Authors'.
+ *       The original license header is included below.
+ *
+ * =====================================================================
+ *
  * eXist-db Open Source Native XML Database
  * Copyright (C) 2001 The eXist-db Authors
  *
@@ -19,7 +43,6 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
  */
-
 package org.exist.xquery;
 
 import org.apache.commons.io.output.StringBuilderWriter;
@@ -317,9 +340,10 @@ private FunctionStats[] sort() {
         return stats;
     }
 
-    public synchronized void toXML(MemTreeBuilder builder) {
+    public synchronized void toXML(final MemTreeBuilder builder) {
         final AttributesImpl attrs = new AttributesImpl();
-        builder.startElement(new QName("calls", XML_NAMESPACE, XML_PREFIX), null);
+        attrs.addAttribute("", "tracing-enabled", "tracing-enabled", "CDATA", Boolean.toString(isEnabled()));
+        builder.startElement(new QName("calls", XML_NAMESPACE, XML_PREFIX), attrs);
         for (final QueryStats stats : queries.values()) {
             attrs.clear();
             attrs.addAttribute("", "source", "source", "CDATA", stats.source);
@@ -333,20 +357,19 @@ public synchronized void toXML(MemTreeBuilder builder) {
             attrs.addAttribute("", "name", "name", "CDATA", stats.qname.getStringValue());
             attrs.addAttribute("", "elapsed", "elapsed", "CDATA", Double.toString(stats.executionTime / 1000.0));
             attrs.addAttribute("", "calls", "calls", "CDATA", Integer.toString(stats.callCount));
-            if (stats.source != null)
-                {attrs.addAttribute("", "source", "source", "CDATA", stats.source);}
+            if (stats.source != null) {
+                attrs.addAttribute("", "source", "source", "CDATA", stats.source);
+            }
             builder.startElement(new QName("function", XML_NAMESPACE, XML_PREFIX), attrs);
             builder.endElement();
         }
         for (final IndexStats stats: indexStats.values()) {
             attrs.clear();
             attrs.addAttribute("", "type", "type", "CDATA", stats.indexType);
-            attrs.addAttribute("", "source", "source", "CDATA", stats.source + " [" + stats.line + ":" +
-                stats.column + "]");
+            attrs.addAttribute("", "source", "source", "CDATA", stats.source + " [" + stats.line + ":" + stats.column + "]");
             attrs.addAttribute("", "elapsed", "elapsed", "CDATA", Double.toString(stats.executionTime / 1000.0));
             attrs.addAttribute("", "calls", "calls", "CDATA", Integer.toString(stats.usageCount));
-            attrs.addAttribute("", "optimization", "optimization", "CDATA",
-                Integer.toString(stats.mode));
+            attrs.addAttribute("", "optimization", "optimization", "CDATA", Integer.toString(stats.mode));
             builder.startElement(new QName("index", XML_NAMESPACE, XML_PREFIX), attrs);
             builder.endElement();
         }

exist-jetty-config/src/main/resources/webapp/WEB-INF/web.xml

@@ -176,6 +176,11 @@
     <servlet>
         <servlet-name>JMXServlet</servlet-name>
         <servlet-class>org.exist.management.client.JMXServlet</servlet-class>
+        <!-- NOTE(AR) required for Monex to correctly retrieve the JMX Token
+            as the JMXServlet has to create it before Monex makes its first request
+            for the status page
+        -->
+        <load-on-startup>3</load-on-startup>
     </servlet>
 
     <!--