Repository: jupyterhub/jupyterhub
Pull request: more debugging and health checks for services
- log that external services are added (helps catch accidental external services due to missing fields)
- check connectivity of services with web endpoints periodically

closes #934

Problem statement:
Improve logging information for services
**How to reproduce the issue**

Configure a managed service, but forget to include the `command` argument (or other important ones, such as the URL)/

**What you expected to happen**

It would be nice is JupyterHub gave more information in the logging to indicate that a managed service had started or not and warned the admin if he service appears to not be working as expected. One option would be to have services implement a "health" endpoint that the hub and hit to make sure the service is running as expected.

**What actually happens**

JupyterHub fails to start the managed service, but the logs don't say that. When you go to access the service it fails.

**Share what version of JupyterHub you are using**

0.7.2.

Hints from issue discussion:
@ellisonbg Good suggestion 👍

Commit message: more debugging and health checks for services

- log that external services are added (helps catch accidental external services due to missing fields)
- check connectivity of services with web endpoints periodically

Patch to review:
diff --git a/jupyterhub/app.py b/jupyterhub/app.py
index 2e8834419e..2daf64f032 100644
--- a/jupyterhub/app.py
+++ b/jupyterhub/app.py
@@ -259,6 +259,9 @@ class JupyterHub(Application):
     proxy_check_interval = Integer(30,
         help="Interval (in seconds) at which to check if the proxy is running."
     ).tag(config=True)
+    service_check_interval = Integer(60,
+        help="Interval (in seconds) at which to check connectivity of services."
+    ).tag(config=True)
 
     data_files_path = Unicode(DATA_FILES_PATH,
         help="The location of jupyterhub data files (e.g. /usr/local/share/jupyter/hub)"
@@ -1058,6 +1061,19 @@ def init_services(self):
                 self.db.delete(service)
         self.db.commit()
 
+    @gen.coroutine
+    def check_services_health(self):
+        """Check connectivity of all services"""
+        for name, service in self._service_map.items():
+            if not service.url:
+                continue
+            try:
+                yield service.orm.server.wait_up(timeout=1)
+            except TimeoutError:
+                self.log.warning("Cannot connect to %s service %s at %s", service.kind, name, service.url)
+            else:
+                self.log.debug("%s service %s running at %s", service.kind.title(), name, service.url)
+
     @gen.coroutine
     def init_spawners(self):
         db = self.db
@@ -1463,13 +1479,32 @@ def start(self):
             self.exit(1)
 
         for service_name, service in self._service_map.items():
-            if not service.managed:
-                continue
-            try:
-                service.start()
-            except Exception as e:
-                self.log.critical("Failed to start service %s", service_name, exc_info=True)
-                self.exit(1)
+            msg = '%s at %s' % (service_name, service.url) if service.url else service_name
+            if service.managed:
+                self.log.info("Starting managed service %s", msg)
+                try:
+                    service.start()
+                except Exception as e:
+                    self.log.critical("Failed to start service %s", service_name, exc_info=True)
+                    self.exit(1)
+            else:
+                self.log.info("Adding external service %s", msg)
+
+            if service.url:
+                tries = 10 if service.managed else 1
+                for i in range(tries):
+                    try:
+                        yield service.orm.server.wait_up(http=True, timeout=1)
+                    except TimeoutError:
+                        if service.managed:
+                            status = yield service.spawner.poll()
+                            if status is not None:
+                                self.log.error("Service %s exited with status %s", service_name, status)
+                                break
+                    else:
+                        break
+                else:
+                    self.log.error("Cannot connect to %s service %s at %s. Is it running?", service.kind, service_name, service.url)
 
         loop.add_callback(self.proxy.add_all_users, self.users)
         loop.add_callback(self.proxy.add_all_services, self._service_map)
@@ -1481,6 +1516,10 @@ def start(self):
             pc = PeriodicCallback(self.check_proxy, 1e3 * self.proxy_check_interval)
             pc.start()
 
+        if self.service_check_interval and any(s.url for s in self._service_map.values()):
+            pc = PeriodicCallback(self.check_services_health, 1e3 * self.service_check_interval)
+            pc.start()
+
         if self.last_activity_interval:
             pc = PeriodicCallback(self.update_last_activity, 1e3 * self.last_activity_interval)
             pc.start()
diff --git a/jupyterhub/services/service.py b/jupyterhub/services/service.py
index 207088d7d7..443c3fc471 100644
--- a/jupyterhub/services/service.py
+++ b/jupyterhub/services/service.py
@@ -160,6 +160,11 @@ class Service(LoggingConfigurable):
     def managed(self):
         """Am I managed by the Hub?"""
         return bool(self.command)
+    
+    @property
+    def kind(self):
+        """Am I managed (as text)"""
+        return 'managed' if self.managed else 'external'
 
     command = Command(minlen=0,
         help="Command to spawn this service, if managed."


Code review comments:
diff_hunk: @@ -160,6 +160,11 @@ class Service(LoggingConfigurable):
     def managed(self):
         """Am I managed by the Hub?"""
         return bool(self.command)
+    
+    @property
+    def kind(self):
+        """Am I managed (as text)"""
path: jupyterhub/services/service.py
text: @user1:
Perhaps `"""Am I self managed?"""`. I'm not sure that the suggestion is better, but `as text` wasn't clear to me.

Or maybe: `"""Determine kind of service. True, self managed. False, externally managed."""`

@user1:
Or perhaps better still: `"""Determine kind of service. True, managed; False, external."""`