From 50131105f31e2fbd8d609a2e93415895115f3ecd Mon Sep 17 00:00:00 2001 From: Sally Wilsak Date: Sat, 25 Aug 2018 14:51:58 -0400 Subject: [PATCH 1/9] Get the list of exporters from entrypoints `exporter_map` is deprecated, so let's use the list of exporters fetched from the installed entrypoints. There's a supposed attribute `export_from_notebook` that should be set to a friendly string name if the exporter should be exposed in the front-end. However, the exporters defined in `nbconvert` don't have it set, so I haven't used it to determine inclusion in the list. Instead, I've used the entrypoint name as the friendly name, which looks like it was the intention from the way they are named. I ran the unit tests and tried starting up the notebook server and accessing the API endpoint to verify the JSON looked correct. --- jupyter_server/services/nbconvert/handlers.py | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/jupyter_server/services/nbconvert/handlers.py b/jupyter_server/services/nbconvert/handlers.py index 7e27783ef0..63e731238f 100644 --- a/jupyter_server/services/nbconvert/handlers.py +++ b/jupyter_server/services/nbconvert/handlers.py @@ -4,18 +4,32 @@ from ...base.handlers import APIHandler + class NbconvertRootHandler(APIHandler): @web.authenticated def get(self): try: - from nbconvert.exporters.export import exporter_map + from nbconvert.exporters import base except ImportError as e: raise web.HTTPError(500, "Could not import nbconvert: %s" % e) res = {} - for format, exporter in exporter_map.items(): - res[format] = info = {} - info['output_mimetype'] = exporter.output_mimetype + exporters = base.get_export_names() + for exporter_name in exporters: + try: + exporter_class = base.get_exporter(exporter_name) + except ValueError: + # I think the only way this will happen is if the entrypoint + # is uninstalled while this method is running + continue + # XXX: According to the docs, it looks like this should be set to None + # if the exporter shouldn't be exposed to the front-end and a friendly + # name if it should. However, none of the built-in exports have it defined. + # if not exporter_class.export_from_notebook: + # continue + res[exporter_name] = { + "output_mimetype": exporter_class.output_mimetype, + } self.finish(json.dumps(res)) From c8c853d32e96f742342ffb20ddb4c48580cfe069 Mon Sep 17 00:00:00 2001 From: Bill Major Date: Tue, 28 Aug 2018 15:42:15 -0400 Subject: [PATCH 2/9] Allow access control headers to be overriden in jupyter_notebook_config.py --- jupyter_server/base/handlers.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/jupyter_server/base/handlers.py b/jupyter_server/base/handlers.py index f0980378a1..37341f5a01 100755 --- a/jupyter_server/base/handlers.py +++ b/jupyter_server/base/handlers.py @@ -589,8 +589,11 @@ def finish(self, *args, **kwargs): return super(APIHandler, self).finish(*args, **kwargs) def options(self, *args, **kwargs): - self.set_header('Access-Control-Allow-Headers', - 'accept, content-type, authorization, x-xsrftoken') + if 'Access-Control-Allow-Headers' in self.settings.get('headers', {}): + self.set_header('Access-Control-Allow-Headers', self.settings['headers']['Access-Control-Allow-Headers']) + else: + self.set_header('Access-Control-Allow-Headers', + 'accept, content-type, authorization, x-xsrftoken') self.set_header('Access-Control-Allow-Methods', 'GET, PUT, POST, PATCH, DELETE, OPTIONS') From 674b65c19dc32b6e755b40c6351cca1aaa147dbf Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Tue, 11 Sep 2018 11:08:05 -0700 Subject: [PATCH 3/9] Prevent access through 'NoneType' when closing activity_stream Although I'm unable to reproduce the issue, its a safe change to prevent an AttributeError from occuring ('NoneType' object has no attribute 'close'). The user that reported this is attempting to launch a kernel and I believe the launch only partially completed such that `kernel._activity_stream` did not get established. (This occurred from Jupyter Enterprise Gateway where we deal with remote kernel launches across resource-managed clusters, so things are a bit more involved relative to kernel establishment.) --- jupyter_server/services/kernels/kernelmanager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jupyter_server/services/kernels/kernelmanager.py b/jupyter_server/services/kernels/kernelmanager.py index 8356b50697..bb5dba34b8 100644 --- a/jupyter_server/services/kernels/kernelmanager.py +++ b/jupyter_server/services/kernels/kernelmanager.py @@ -282,8 +282,9 @@ def shutdown_kernel(self, kernel_id, now=False): """Shutdown a kernel by kernel_id""" self._check_kernel_id(kernel_id) kernel = self._kernels[kernel_id] - kernel._activity_stream.close() - kernel._activity_stream = None + if kernel._activity_stream: + kernel._activity_stream.close() + kernel._activity_stream = None self.stop_buffering(kernel_id) self._kernel_connections.pop(kernel_id, None) self.last_kernel_activity = utcnow() From 0abc68ca5490311c25981650f1cdb398c176cfdb Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 25 Sep 2018 20:52:21 -0700 Subject: [PATCH 4/9] When adapting also log the version jupyter_client is expecting. It's still (IMHO) unclear how the adaptation goes. Like does "adapting to protocol X.Y.Z." mean the end-result is X.Y.Z or what you got is X.Y.Z and it's X.Y.W after adaptation. --- jupyter_server/services/kernels/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_server/services/kernels/handlers.py b/jupyter_server/services/kernels/handlers.py index c75f243643..073cb31752 100644 --- a/jupyter_server/services/kernels/handlers.py +++ b/jupyter_server/services/kernels/handlers.py @@ -188,7 +188,7 @@ def _finish_kernel_info(self, info): protocol_version = info.get('protocol_version', client_protocol_version) if protocol_version != client_protocol_version: self.session.adapt_version = int(protocol_version.split('.')[0]) - self.log.info("Adapting to protocol v%s for kernel %s", protocol_version, self.kernel_id) + self.log.info("Adapting to protocol v%s for kernel %s (client expecting %s)", protocol_version, self.kernel_id, client_protocol_version) if not self._kernel_info_future.done(): self._kernel_info_future.set_result(info) From 3d1cb4c70b1080e306a1310979c2f079d32264fc Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Wed, 26 Sep 2018 11:46:14 -0700 Subject: [PATCH 5/9] More descriptive message. --- jupyter_server/services/kernels/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_server/services/kernels/handlers.py b/jupyter_server/services/kernels/handlers.py index 073cb31752..cf744894c9 100644 --- a/jupyter_server/services/kernels/handlers.py +++ b/jupyter_server/services/kernels/handlers.py @@ -188,7 +188,7 @@ def _finish_kernel_info(self, info): protocol_version = info.get('protocol_version', client_protocol_version) if protocol_version != client_protocol_version: self.session.adapt_version = int(protocol_version.split('.')[0]) - self.log.info("Adapting to protocol v%s for kernel %s (client expecting %s)", protocol_version, self.kernel_id, client_protocol_version) + self.log.info("Adapting from protocol version {protocol_version} (kernel {kernel_id}) to {client_protocol_version} (client).".format(protocol_version=protocol_version, kernel_id=self.kernel_id, client_protocol_version=client_protocol_version)) if not self._kernel_info_future.done(): self._kernel_info_future.set_result(info) From 1ec99a43813afaedc34edbde2efddea92a6742f6 Mon Sep 17 00:00:00 2001 From: Maxim Vov Date: Wed, 26 Sep 2018 17:15:40 +0300 Subject: [PATCH 6/9] Moved the 'metrics' file to a new package named 'promethues'. Then split the file into two files, one containing the metrics themselves, the other containing any function that have something to do with prometheus. Finally, Added new metrics into the prometheus.metrics file that represent the number of running terminals and added the functionality for that metric to be recorded in the terminal.api_handlers file. --- jupyter_server/log.py | 3 ++- jupyter_server/metrics.py | 15 +------------- jupyter_server/prometheus/__init__.py | 0 jupyter_server/prometheus/log_functions.py | 24 ++++++++++++++++++++++ jupyter_server/prometheus/metrics.py | 21 +++++++++++++++++++ jupyter_server/terminal/api_handlers.py | 8 +++++++- 6 files changed, 55 insertions(+), 16 deletions(-) create mode 100644 jupyter_server/prometheus/__init__.py create mode 100644 jupyter_server/prometheus/log_functions.py create mode 100644 jupyter_server/prometheus/metrics.py diff --git a/jupyter_server/log.py b/jupyter_server/log.py index 64b35d811d..3621a70cae 100644 --- a/jupyter_server/log.py +++ b/jupyter_server/log.py @@ -7,7 +7,8 @@ import json from tornado.log import access_log -from .metrics import prometheus_log_method +from .prometheus.log_functions import prometheus_log_method + def log_request(handler): """log a bit more information about each request than tornado's default diff --git a/jupyter_server/metrics.py b/jupyter_server/metrics.py index 24a08d8c88..338b59d0d1 100644 --- a/jupyter_server/metrics.py +++ b/jupyter_server/metrics.py @@ -1,18 +1,5 @@ -""" -Prometheus metrics exported by Jupyter Notebook Server +from notebook.prometheus.metrics import HTTP_REQUEST_DURATION_SECONDS -Read https://prometheus.io/docs/practices/naming/ for naming -conventions for metrics & labels. -""" - -from prometheus_client import Histogram - -# This is a fairly standard name for HTTP duration latency reporting -HTTP_REQUEST_DURATION_SECONDS = Histogram( - 'http_request_duration_seconds', - 'duration in seconds for all HTTP requests', - ['method', 'handler', 'status_code'], -) def prometheus_log_method(handler): """ diff --git a/jupyter_server/prometheus/__init__.py b/jupyter_server/prometheus/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/jupyter_server/prometheus/log_functions.py b/jupyter_server/prometheus/log_functions.py new file mode 100644 index 0000000000..338b59d0d1 --- /dev/null +++ b/jupyter_server/prometheus/log_functions.py @@ -0,0 +1,24 @@ +from notebook.prometheus.metrics import HTTP_REQUEST_DURATION_SECONDS + + +def prometheus_log_method(handler): + """ + Tornado log handler for recording RED metrics. + + We record the following metrics: + Rate - the number of requests, per second, your services are serving. + Errors - the number of failed requests per second. + Duration - The amount of time each request takes expressed as a time interval. + + We use a fully qualified name of the handler as a label, + rather than every url path to reduce cardinality. + + This function should be either the value of or called from a function + that is the 'log_function' tornado setting. This makes it get called + at the end of every request, allowing us to record the metrics we need. + """ + HTTP_REQUEST_DURATION_SECONDS.labels( + method=handler.request.method, + handler='{}.{}'.format(handler.__class__.__module__, type(handler).__name__), + status_code=handler.get_status() + ).observe(handler.request.request_time()) diff --git a/jupyter_server/prometheus/metrics.py b/jupyter_server/prometheus/metrics.py new file mode 100644 index 0000000000..c4defa772e --- /dev/null +++ b/jupyter_server/prometheus/metrics.py @@ -0,0 +1,21 @@ +""" +Prometheus metrics exported by Jupyter Notebook Server + +Read https://prometheus.io/docs/practices/naming/ for naming +conventions for metrics & labels. +""" + + +from prometheus_client import Histogram, Gauge + + +HTTP_REQUEST_DURATION_SECONDS = Histogram( + 'http_request_duration_seconds', + 'duration in seconds for all HTTP requests', + ['method', 'handler', 'status_code'], +) + +TERMINAL_CURRENTLY_RUNNING_TOTAL = Gauge( + 'terminal_currently_running_total', + 'counter for how many terminals are running', +) diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 1e92b58e4c..84a0eec4b2 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -1,7 +1,8 @@ import json from tornado import web, gen from ..base.handlers import APIHandler -from ..utils import url_path_join +from notebook.prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL + class TerminalRootHandler(APIHandler): @@ -11,12 +12,16 @@ def get(self): tm = self.terminal_manager terms = [{'name': name} for name in tm.terminals] self.finish(json.dumps(terms)) + TERMINAL_CURRENTLY_RUNNING_TOTAL.set( + len(terms) + ) @web.authenticated def post(self): """POST /terminals creates a new terminal and redirects to it""" name, _ = self.terminal_manager.new_named_terminal() self.finish(json.dumps({'name': name})) + TERMINAL_CURRENTLY_RUNNING_TOTAL.inc() class TerminalHandler(APIHandler): @@ -38,5 +43,6 @@ def delete(self, name): yield tm.terminate(name, force=True) self.set_status(204) self.finish() + TERMINAL_CURRENTLY_RUNNING_TOTAL.dec(1) else: raise web.HTTPError(404, "Terminal not found: %r" % name) From 3bc7091e0f4546afb29f1394d27bbeac5cdc3504 Mon Sep 17 00:00:00 2001 From: Maxim Vov Date: Wed, 26 Sep 2018 18:14:43 +0300 Subject: [PATCH 7/9] Fixed imports, from static to dynamic. --- jupyter_server/metrics.py | 2 +- jupyter_server/terminal/api_handlers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyter_server/metrics.py b/jupyter_server/metrics.py index 338b59d0d1..a67a252ade 100644 --- a/jupyter_server/metrics.py +++ b/jupyter_server/metrics.py @@ -1,4 +1,4 @@ -from notebook.prometheus.metrics import HTTP_REQUEST_DURATION_SECONDS +from ..prometheus.metrics import HTTP_REQUEST_DURATION_SECONDS def prometheus_log_method(handler): diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 84a0eec4b2..1a70840fc8 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -1,7 +1,7 @@ import json from tornado import web, gen from ..base.handlers import APIHandler -from notebook.prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL +from ..prometheus.metrics import TERMINAL_CURRENTLY_RUNNING_TOTAL From 25b9f805b3c720456f458923e1a97eb381e2f655 Mon Sep 17 00:00:00 2001 From: Maxim Vov Date: Thu, 27 Sep 2018 01:55:52 +0300 Subject: [PATCH 8/9] Added docs to the prometheus package and the terminal.api_handlers file. --- jupyter_server/terminal/api_handlers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 1a70840fc8..4ffec14825 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -12,6 +12,7 @@ def get(self): tm = self.terminal_manager terms = [{'name': name} for name in tm.terminals] self.finish(json.dumps(terms)) + # Update the metric below to the length of the list 'terms' TERMINAL_CURRENTLY_RUNNING_TOTAL.set( len(terms) ) @@ -21,6 +22,7 @@ def post(self): """POST /terminals creates a new terminal and redirects to it""" name, _ = self.terminal_manager.new_named_terminal() self.finish(json.dumps({'name': name})) + # Increase the metric by one because a new terminal was created TERMINAL_CURRENTLY_RUNNING_TOTAL.inc() @@ -43,6 +45,7 @@ def delete(self, name): yield tm.terminate(name, force=True) self.set_status(204) self.finish() + # Decrease the metric below by one because a terminal has been shutdown TERMINAL_CURRENTLY_RUNNING_TOTAL.dec(1) else: raise web.HTTPError(404, "Terminal not found: %r" % name) From 9eafe12952ba02ccc25cd863700d89cf7e779a16 Mon Sep 17 00:00:00 2001 From: Maxim Vov Date: Sat, 29 Sep 2018 16:56:40 +0300 Subject: [PATCH 9/9] Added metrics for number of kernels running labeled by type --- jupyter_server/prometheus/metrics.py | 6 ++++++ jupyter_server/services/kernels/kernelmanager.py | 16 ++++++++++++++++ jupyter_server/terminal/api_handlers.py | 9 +++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/jupyter_server/prometheus/metrics.py b/jupyter_server/prometheus/metrics.py index c4defa772e..abc9d0e16b 100644 --- a/jupyter_server/prometheus/metrics.py +++ b/jupyter_server/prometheus/metrics.py @@ -19,3 +19,9 @@ 'terminal_currently_running_total', 'counter for how many terminals are running', ) + +KERNEL_CURRENTLY_RUNNING_TOTAL = Gauge( + 'kernel_currently_running_total', + 'counter for how many kernels are running labeled by type', + ['type'] +) diff --git a/jupyter_server/services/kernels/kernelmanager.py b/jupyter_server/services/kernels/kernelmanager.py index bb5dba34b8..b7f6d9f9a3 100644 --- a/jupyter_server/services/kernels/kernelmanager.py +++ b/jupyter_server/services/kernels/kernelmanager.py @@ -26,6 +26,8 @@ from jupyter_server._tz import utcnow, isoformat from ipython_genutils.py3compat import getcwd +from notebook.prometheus.metrics import KERNEL_CURRENTLY_RUNNING_TOTAL + class MappingKernelManager(MultiKernelManager): """A KernelManager that handles @@ -178,6 +180,13 @@ def start_kernel(self, kernel_id=None, path=None, **kwargs): lambda : self._handle_kernel_died(kernel_id), 'dead', ) + + # Increase the metric of number of kernels running + # for the relevant kernel type by 1 + KERNEL_CURRENTLY_RUNNING_TOTAL.labels( + type=self._kernels[kernel_id].kernel_name + ).inc() + else: self._check_kernel_id(kernel_id) self.log.info("Using existing kernel: %s" % kernel_id) @@ -288,6 +297,13 @@ def shutdown_kernel(self, kernel_id, now=False): self.stop_buffering(kernel_id) self._kernel_connections.pop(kernel_id, None) self.last_kernel_activity = utcnow() + + # Decrease the metric of number of kernels + # running for the relevant kernel type by 1 + KERNEL_CURRENTLY_RUNNING_TOTAL.labels( + type=self._kernels[kernel_id].kernel_name + ).dec() + return super(MappingKernelManager, self).shutdown_kernel(kernel_id, now=now) def restart_kernel(self, kernel_id): diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 4ffec14825..d64e1acb3f 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -12,6 +12,7 @@ def get(self): tm = self.terminal_manager terms = [{'name': name} for name in tm.terminals] self.finish(json.dumps(terms)) + # Update the metric below to the length of the list 'terms' TERMINAL_CURRENTLY_RUNNING_TOTAL.set( len(terms) @@ -22,6 +23,7 @@ def post(self): """POST /terminals creates a new terminal and redirects to it""" name, _ = self.terminal_manager.new_named_terminal() self.finish(json.dumps({'name': name})) + # Increase the metric by one because a new terminal was created TERMINAL_CURRENTLY_RUNNING_TOTAL.inc() @@ -45,7 +47,10 @@ def delete(self, name): yield tm.terminate(name, force=True) self.set_status(204) self.finish() - # Decrease the metric below by one because a terminal has been shutdown - TERMINAL_CURRENTLY_RUNNING_TOTAL.dec(1) + + # Decrease the metric below by one + # because a terminal has been shutdown + TERMINAL_CURRENTLY_RUNNING_TOTAL.dec() + else: raise web.HTTPError(404, "Terminal not found: %r" % name)