ENG-2246: More UI improvements

Summary: - Auto curl and fetch latest info from current master leader on all pages - Moved a lot of common utilities into a separate nav link - Added iconic navigation Test Plan: Tested on my local machine Reviewers: kannan, mihnea Reviewed By: mihnea Subscribers: eng, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D3415
ryan-ally · Nov 9, 2017 · 06c7271 · 06c7271
1 parent ee44168
commit 06c7271
Show file tree

Hide file tree

Showing 11 changed files with 186 additions and 73 deletions.
diff --git a/src/yb/master/master-path-handlers.cc b/src/yb/master/master-path-handlers.cc
@@ -76,52 +76,63 @@ MasterPathHandlers::~MasterPathHandlers() {
 void MasterPathHandlers::CallIfLeaderOrPrintRedirect(
     const Webserver::WebRequest& req, stringstream* output,
     const Webserver::PathHandlerCallback& callback) {
+  string redirect;
   // Lock the CatalogManager in a self-contained block, to prevent double-locking on callbacks.
   {
     CatalogManager::ScopedLeaderSharedLock l(master_->catalog_manager());
-    if (!l.first_failed_status().ok()) {
-      *output << "<h1>This is not the Master Leader!</h1>\n";
 
-      do {
-        vector<ServerEntryPB> masters;
-        Status s = master_->ListMasters(&masters);
-        if (!s.ok()) {
-          break;
-        }
-
-        string redirect;
-        for (const ServerEntryPB& master : masters) {
-          if (master.has_error()) {
-            // This will leave redirect empty and thus fail accordingly.
-            break;
-          }
-
-          if (master.role() == consensus::RaftPeerPB::LEADER) {
-            // URI already starts with a /, so none is needed between $1 and $2.
-            redirect = Substitute(
-                "<a class=\"alert-link\" href=\"http://$0:$1$2$3\">Leader</a>",
-                master.registration().http_addresses(0).host(),
-                master.registration().http_addresses(0).port(), req.redirect_uri,
-                req.query_string.empty() ? "" : "?" + req.query_string);
-          }
-        }
+    // If we are the master leader, handle the request.
+    if (l.first_failed_status().ok()) {
+      callback(req, output);
+      return;
+    }
 
-        if (redirect.empty()) {
-          break;
-        }
+    // List all the masters.
+    vector<ServerEntryPB> masters;
+    Status s = master_->ListMasters(&masters);
+    if (!s.ok()) {
+      s = s.CloneAndPrepend("Unable to list Masters");
+      LOG(WARNING) << s.ToString();
+      *output << "<h2>" << s.ToString() << "</h2>\n";
+      return;
+    }
 
-        *output << "<h3><div class=\"alert alert-warning\">"
-                << "Please click  " << redirect << " to get redirected to the Master Leader!"
-                << "</div></h3>";
+    // Prepare the query for the master leader.
 
+    for (const ServerEntryPB &master : masters) {
+      if (master.has_error()) {
+        *output << "<h2>" << "Error listing all masters." << "</h2>\n";
         return;
-      } while (0);
+      }
 
-      *output << "Cannot get Leader information to help you redirect...\n";
-      return;
+      if (master.role() == consensus::RaftPeerPB::LEADER) {
+        // URI already starts with a /, so none is needed between $1 and $2.
+        redirect = Substitute("http://$0:$1$2$3",
+                              master.registration().http_addresses(0).host(),
+                              master.registration().http_addresses(0).port(),
+                              req.redirect_uri,
+                              req.query_string.empty() ? "?raw" : "?" + req.query_string + "&raw");
+        break;
+      }
     }
   }
-  callback(req, output);
+
+  // Error out if we do not have a redirect URL to the current master leader.
+  if (redirect.empty()) {
+    *output << "<h2>" << "Error querying master leader." << "</h2>\n";
+    return;
+  }
+
+  // Make a curl call to the current master leader and return that payload as the result of the
+  // web request.
+  EasyCurl curl;
+  faststring buf;
+  Status s = curl.FetchURL(redirect, &buf);
+  if (!s.ok()) {
+    *output << "<h2>" << "Error querying url " << redirect << "</h2>\n";
+    return;
+  }
+  *output << buf.ToString();
 }
 
 void MasterPathHandlers::HandleTabletServers(const Webserver::WebRequest& req,
@@ -396,17 +407,25 @@ void MasterPathHandlers::RootHandler(const Webserver::WebRequest& req,
             << "<div class='panel-heading'><h2 class='panel-title'> Overview</h2></div>\n";
   (*output) << "<div class='panel-body'>";
   (*output) << "<table class='table'>\n";
-  (*output) << Substitute("  <tr><td>$0<span class='yb-overview'>$1</span></td><td>$2</td></tr>\n",
+
+  // Universe UUID.
+  (*output) << "  <tr>";
+  (*output) << Substitute(" <td>$0<span class='yb-overview'>$1</span></td>",
                           "<i class='fa fa-database yb-dashboard-icon' aria-hidden='true'></i>",
-                          "Universe UUID ",
+                          "Universe UUID ");
+  (*output) << Substitute(" <td>$0</td>",
                           config.cluster_uuid());
+  (*output) << "  </tr>\n";
 
   // Replication factor.
   (*output) << "  <tr>";
   (*output) << Substitute(" <td>$0<span class='yb-overview'>$1</span></td>",
                           "<i class='fa fa-files-o yb-dashboard-icon' aria-hidden='true'></i>",
                           "Replication Factor ");
-  (*output) << Substitute(" <td>$0</td>", master_->opts().GetMasterAddresses().get()->size());
+  (*output) << Substitute(" <td>$0 <a href='$1' class='btn btn-default pull-right'>$2</a></td>",
+                          master_->opts().GetMasterAddresses().get()->size(),
+                          "/cluster-config",
+                          "See full config &raquo;");
   (*output) << "  </tr>\n";
 
   // Tserver count.
@@ -452,13 +471,6 @@ void MasterPathHandlers::RootHandler(const Webserver::WebRequest& req,
   (*output) << "<div class='col-md-12 col-lg-12'>\n";
   HandleCatalogManager(req, output, true /* skip_system_tables */);
   (*output) << "</div> <!-- col-md-12 col-lg-12 -->\n";
-
-  // Display the tablet server info.
-  (*output) << "<div class='col-md-12 col-lg-12'>\n";
-  HandleTabletServers(req, output);
-  (*output) << "</div> <!-- col-md-12 col-lg-12 -->\n";
-
-  (*output) << "</div> <!-- row dashboard-content -->\n";
 }
 
 void MasterPathHandlers::HandleMasters(const Webserver::WebRequest& req,
@@ -689,7 +701,7 @@ void MasterPathHandlers::HandleDumpEntities(const Webserver::WebRequest& req,
 }
 
 void MasterPathHandlers::HandleGetClusterConfig(
-    const Webserver::WebRequest& req, stringstream* output) {
+  const Webserver::WebRequest& req, stringstream* output) {
   *output << "<h1>Current Cluster Config</h1>\n";
   SysClusterConfigEntryPB config;
   Status s = master_->catalog_manager()->GetClusterConfig(&config);
@@ -699,36 +711,39 @@ void MasterPathHandlers::HandleGetClusterConfig(
   }
 
   *output << "<div class=\"alert alert-success\">Successfully got cluster config!</div>"
-          << "<pre class=\"prettyprint\">" << config.DebugString() << "</pre>";
+  << "<pre class=\"prettyprint\">" << config.DebugString() << "</pre>";
 }
 
+
 Status MasterPathHandlers::Register(Webserver* server) {
   bool is_styled = true;
   bool is_on_nav_bar = true;
-  // Cannot use auto with callbacks, as they won't properly deduce types with boost magic...
+
+  // The set of handlers visible on the nav bar.
   server->RegisterPathHandler(
     "/", "Home", std::bind(&MasterPathHandlers::RootHandler, this, _1, _2), is_styled,
-    is_on_nav_bar);
-
+    is_on_nav_bar, "fa fa-home");
   Webserver::PathHandlerCallback cb =
       std::bind(&MasterPathHandlers::HandleTabletServers, this, _1, _2);
   server->RegisterPathHandler(
       "/tablet-servers", "Tablet Servers",
       std::bind(&MasterPathHandlers::CallIfLeaderOrPrintRedirect, this, _1, _2, cb), is_styled,
-      is_on_nav_bar);
+      is_on_nav_bar, "fa fa-server");
   cb = std::bind(&MasterPathHandlers::HandleCatalogManager,
                  this, _1, _2, false /* skip_system_tables */);
   server->RegisterPathHandler(
       "/tables", "Tables",
       std::bind(&MasterPathHandlers::CallIfLeaderOrPrintRedirect, this, _1, _2, cb), is_styled,
-      is_on_nav_bar);
+      is_on_nav_bar, "fa fa-table");
   cb = std::bind(&MasterPathHandlers::HandleTablePage, this, _1, _2);
+
+  // The set of handlers not visible on the nav bar.
   server->RegisterPathHandler(
       "/table", "", std::bind(&MasterPathHandlers::CallIfLeaderOrPrintRedirect, this, _1, _2, cb),
       is_styled, false);
   server->RegisterPathHandler(
       "/masters", "Masters", std::bind(&MasterPathHandlers::HandleMasters, this, _1, _2), is_styled,
-      is_on_nav_bar);
+      false);
   cb = std::bind(&MasterPathHandlers::HandleDumpEntities, this, _1, _2);
   server->RegisterPathHandler(
       "/dump-entities", "Dump Entities",
@@ -737,7 +752,7 @@ Status MasterPathHandlers::Register(Webserver* server) {
   server->RegisterPathHandler(
       "/cluster-config", "Cluster Config",
       std::bind(&MasterPathHandlers::CallIfLeaderOrPrintRedirect, this, _1, _2, cb), is_styled,
-      is_on_nav_bar);
+      false);
   return Status::OK();
 }
 

diff --git a/src/yb/server/default-path-handlers.cc b/src/yb/server/default-path-handlers.cc
@@ -189,10 +189,11 @@ static void MemTrackersHandler(const Webserver::WebRequest& req, std::stringstre
 }
 
 void AddDefaultPathHandlers(Webserver* webserver) {
-  webserver->RegisterPathHandler("/logs", "Logs", LogsHandler);
-  webserver->RegisterPathHandler("/varz", "Flags", FlagsHandler);
-  webserver->RegisterPathHandler("/memz", "Memory (total)", MemUsageHandler);
-  webserver->RegisterPathHandler("/mem-trackers", "Memory (detail)", MemTrackersHandler);
+  webserver->RegisterPathHandler("/logs", "Logs", LogsHandler, true, false);
+  webserver->RegisterPathHandler("/varz", "Flags", FlagsHandler, true, false);
+  webserver->RegisterPathHandler("/memz", "Memory (total)", MemUsageHandler, true, false);
+  webserver->RegisterPathHandler("/mem-trackers", "Memory (detail)",
+                                 MemTrackersHandler, true, false);
 
   AddPprofPathHandlers(webserver);
 }
@@ -244,8 +245,7 @@ void RegisterMetricsJsonHandler(Webserver* webserver, const MetricRegistry* cons
       WriteForPrometheus, metrics, _1, _2);
   bool not_styled = false;
   bool not_on_nav_bar = false;
-  bool is_on_nav_bar = true;
-  webserver->RegisterPathHandler("/metrics", "Metrics", callback, not_styled, is_on_nav_bar);
+  webserver->RegisterPathHandler("/metrics", "Metrics", callback, not_styled, not_on_nav_bar);
 
   // The old name -- this is preserved for compatibility with older releases of
   // monitoring software which expects the old name.

diff --git a/src/yb/server/rpcz-path-handler.cc b/src/yb/server/rpcz-path-handler.cc
@@ -74,7 +74,7 @@ void RpczPathHandler(const shared_ptr<Messenger>& messenger,
 
 void AddRpczPathHandlers(const shared_ptr<Messenger>& messenger, Webserver* webserver) {
   webserver->RegisterPathHandler(
-      "/rpcz", "RPCs", std::bind(RpczPathHandler, messenger, _1, _2), false, true);
+      "/rpcz", "RPCs", std::bind(RpczPathHandler, messenger, _1, _2), false, false);
 }
 
 } // namespace yb
diff --git a/src/yb/server/server_base.cc b/src/yb/server/server_base.cc
@@ -420,13 +420,52 @@ std::string RpcAndWebServerBase::FooterHtml() const {
                     instance_pb_->permanent_uuid());
 }
 
+static void DisplayIconTile(std::stringstream* output, const string icon,
+                            const string caption, const string url) {
+  *output << "  <div class='col-sm-6 col-md-4 dbg-tile'>\n"
+          << "    <a href='" << url << "' class='thumbnail'>\n"
+          << "      <div class='dbg-icon'>\n"
+          << "        <i class='fa " << icon << "' aria-hidden='true'></i>\n"
+          << "      </div>\n"
+          << "      <div class='caption dbg-caption'>\n"
+          << "        <h3>" << caption << "</h3>\n"
+          << "      </div> <!-- caption -->\n"
+          << "    </a> <!-- thumbnail -->\n"
+          << "  </div> <!-- col-xs-6 col-md-3 -->\n";
+}
+
+static void HandleDebugPage(const Webserver::WebRequest& req, stringstream* output) {
+  *output << "<h1>Debug Utilities</h1>\n";
+
+  *output << "<div class='row debug-tiles'>\n";
+
+  // Logs.
+  DisplayIconTile(output, "fa-files-o", "Logs", "/logs");
+  // GFlags.
+  DisplayIconTile(output, "fa-flag-o", "GFlags", "/varz");
+  // Memory trackers.
+  DisplayIconTile(output, "fa-bar-chart", "Memory Breakdown", "/mem-trackers");
+  // Metrics.
+  DisplayIconTile(output, "fa-line-chart", "Metrics", "/metrics");
+  // RPCs in progress.
+  DisplayIconTile(output, "fa-tasks", "RPCs In Progress", "/rpcz");
+  // Threads.
+  DisplayIconTile(output, "fa-list-ul", "Threads", "/threadz");
+  // Total memory.
+  DisplayIconTile(output, "fa-cog", "Total Memory", "/memz");
+
+  *output << "</div> <!-- row -->\n";
+}
+
 Status RpcAndWebServerBase::Start() {
   GenerateInstanceID();
 
   AddDefaultPathHandlers(web_server_.get());
   AddRpczPathHandlers(messenger_, web_server_.get());
   RegisterMetricsJsonHandler(web_server_.get(), metric_registry_.get());
   TracingPathHandlers::RegisterHandlers(web_server_.get());
+  web_server_->RegisterPathHandler("/utilz", "Utilities", HandleDebugPage,
+                                   true, true, "fa fa-wrench");
   web_server_->set_footer_html(FooterHtml());
   RETURN_NOT_OK(web_server_->Start());
 

diff --git a/src/yb/server/server_base.h b/src/yb/server/server_base.h
@@ -41,6 +41,7 @@
 #include "yb/gutil/ref_counted.h"
 #include "yb/rpc/service_if.h"
 #include "yb/server/server_base_options.h"
+#include "yb/server/webserver.h"
 #include "yb/util/status.h"
 
 namespace yb {

diff --git a/src/yb/server/webserver-test.cc b/src/yb/server/webserver-test.cc
@@ -89,8 +89,8 @@ TEST_F(WebserverTest, TestIndexPage) {
   // Should have expected title.
   ASSERT_STR_CONTAINS(buf_.ToString(), "YugaByte");
 
-  // Should have link to default path handlers (e.g memz)
-  ASSERT_STR_CONTAINS(buf_.ToString(), "memz");
+  // Should have link to the root path handlers (Home).
+  ASSERT_STR_CONTAINS(buf_.ToString(), "Home");
 }
 
 TEST_F(WebserverTest, TestDefaultPaths) {

diff --git a/src/yb/server/webserver.cc b/src/yb/server/webserver.cc
@@ -388,13 +388,17 @@ int Webserver::RunPathHandler(const PathHandler& handler,
   return 1;
 }
 
-void Webserver::RegisterPathHandler(const string& path, const string& alias,
-    const PathHandlerCallback& callback, bool is_styled, bool is_on_nav_bar) {
+void Webserver::RegisterPathHandler(const string& path,
+                                    const string& alias,
+                                    const PathHandlerCallback& callback,
+                                    bool is_styled,
+                                    bool is_on_nav_bar,
+                                    const std::string icon) {
   std::lock_guard<boost::shared_mutex> lock(lock_);
   auto it = path_handlers_.find(path);
   if (it == path_handlers_.end()) {
     it = path_handlers_.insert(
-        make_pair(path, new PathHandler(is_styled, is_on_nav_bar, alias))).first;
+        make_pair(path, new PathHandler(is_styled, is_on_nav_bar, alias, icon))).first;
   }
   it->second->AddCallback(callback);
 }
@@ -430,7 +434,9 @@ void Webserver::BootstrapPageHeader(stringstream* output) {
   (*output) << NAVIGATION_BAR_PREFIX;
   for (const PathHandlerMap::value_type& handler : path_handlers_) {
     if (handler.second->is_on_nav_bar()) {
-      (*output) << "<li class='nav-item'><a href=\"" << handler.first << "\">"
+      (*output) << "<li class='nav-item'>"
+                << "<a href='" << handler.first << "'>"
+                << "<div><i class='" << handler.second->icon() << "'aria-hidden='true'></i></div>"
                 << handler.second->alias()
                 << "</a></li>" << "\n";
     }

diff --git a/src/yb/server/webserver.h b/src/yb/server/webserver.h
@@ -82,7 +82,9 @@ class Webserver : public WebCallbackRegistry {
 
   virtual void RegisterPathHandler(const std::string& path, const std::string& alias,
                                    const PathHandlerCallback& callback,
-                                   bool is_styled = true, bool is_on_nav_bar = true) override;
+                                   bool is_styled = true,
+                                   bool is_on_nav_bar = true,
+                                   const std::string icon = "") override;
 
   // Change the footer HTML to be displayed at the bottom of all styled web pages.
   void set_footer_html(const std::string& html);
@@ -93,10 +95,11 @@ class Webserver : public WebCallbackRegistry {
   // Container class for a list of path handler callbacks for a single URL.
   class PathHandler {
    public:
-    PathHandler(bool is_styled, bool is_on_nav_bar, std::string alias)
+    PathHandler(bool is_styled, bool is_on_nav_bar, std::string alias, const std::string icon)
         : is_styled_(is_styled),
           is_on_nav_bar_(is_on_nav_bar),
-          alias_(std::move(alias)) {}
+          alias_(std::move(alias)),
+          icon_(icon) {}
 
     void AddCallback(const PathHandlerCallback& callback) {
       callbacks_.push_back(callback);
@@ -105,6 +108,7 @@ class Webserver : public WebCallbackRegistry {
     bool is_styled() const { return is_styled_; }
     bool is_on_nav_bar() const { return is_on_nav_bar_; }
     const std::string& alias() const { return alias_; }
+    const std::string& icon() const { return icon_; }
     const std::vector<PathHandlerCallback>& callbacks() const { return callbacks_; }
 
    private:
@@ -117,6 +121,9 @@ class Webserver : public WebCallbackRegistry {
     // Alias used when displaying this link on the nav bar.
     std::string alias_;
 
+    // Icon used when displaying this link on the nav bar.
+    std::string icon_;
+
     // List of callbacks to render output for this page, called in order.
     std::vector<PathHandlerCallback> callbacks_;
   };

diff --git a/src/yb/util/thread.cc b/src/yb/util/thread.cc
@@ -269,7 +269,7 @@ Status ThreadMgr::StartInstrumentation(const scoped_refptr<MetricEntity>& metric
 
   WebCallbackRegistry::PathHandlerCallback thread_callback =
       std::bind(&ThreadMgr::ThreadPathHandler, this, _1, _2);
-  DCHECK_NOTNULL(web)->RegisterPathHandler("/threadz", "Threads", thread_callback);
+  DCHECK_NOTNULL(web)->RegisterPathHandler("/threadz", "Threads", thread_callback, true, false);
   return Status::OK();
 }