Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Soft-deprecate /metrics endpoint #68

Merged
merged 3 commits into from
Nov 19, 2020
Merged

Soft-deprecate /metrics endpoint #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
147b231
Soft-deprecate /metrics endpoint
yuvipanda Nov 17, 2020
0a4c009
Bump version to 0.4.0
yuvipanda Nov 17, 2020
87c306f
Auto-format config.py with black
yuvipanda Nov 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion nbresuse/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,16 @@ def load_jupyter_server_extension(nbapp):
resuseconfig = ResourceUseDisplay(parent=nbapp)
nbapp.web_app.settings["nbresuse_display_config"] = resuseconfig
base_url = nbapp.web_app.settings["base_url"]

if not resuseconfig.disable_legacy_endpoint:
nbapp.web_app.add_handlers(
".*", [(url_path_join(base_url, "/metrics"), ApiHandler)]
)

nbapp.web_app.add_handlers(
".*", [(url_path_join(base_url, "/metrics"), ApiHandler)]
".*", [(url_path_join(base_url, "/api/metrics/v1"), ApiHandler)]
)

callback = ioloop.PeriodicCallback(
PrometheusHandler(PSUtilMetricsLoader(nbapp)), 1000
)
Expand Down
10 changes: 10 additions & 0 deletions nbresuse/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ class ResourceUseDisplay(Configurable):
Holds server-side configuration for nbresuse
"""

disable_legacy_endpoint = Bool(
True,
help="""
Disable legacy /metrics endpoint

This prevents nbresuse from shadowing the prometheus /metrics endpoint.
""",
config=True,
)

process_memory_metrics = List(
trait=PSUtilMetric(),
default_value=[{"name": "memory_info", "attribute": "rss"}],
Expand Down
2 changes: 1 addition & 1 deletion nbresuse/static/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ define([
return;
}
$.getJSON({
url: utils.get_body_data('baseUrl') + 'metrics',
url: utils.get_body_data('baseUrl') + 'api/metrics/v1',
success: function (data) {
totalMemoryUsage = humanFileSize(data['rss']);

Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

setuptools.setup(
name="nbresuse",
version="0.3.6",
version="0.4.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be part of a separate release commit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a separate commit, just in the same PR. Should probably be ok?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking that we might want to add a couple more changes between this PR and the release.

For example moving the lab extension here to the nbresuse repo: jupyterlab/jupyterlab#9363

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jtpio hmm, ideally I'd like to do a quick release here, and then maybe do a 0.5 with other changes - so only change in 0.4 is the deprecation. Since existing JupyterLab installations will work with 0.4 anyway, it should be probably ok?

Copy link
Member

@jtpio jtpio Nov 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good 👍

So to summarize:

  • 0.4.0 with the soft-deprecated endpoint. Users in lab 2.x will still see the indicator because /metrics is still available
  • if the status bar item is removed from core for lab 3.0, then we release nbresuse 0.5.0 with this change: Add a JupyterLab extension for the memory usage status bar item #69. With 0.5.0 removing the /metrics endpoint (or enablable with a config) so users with the lab 2.x + nbresuse combination don't see the indicator twice in the status bar

url="https://github.com/yuvipanda/nbresuse",
author="Yuvi Panda",
description="Simple Jupyter extension to show how much resources (RAM) your notebook is using",
Expand Down