Soft-deprecate /metrics endpoint#68
Conversation
/metrics is where notebook server publishes prometheus metrics. Currently, if nbresuse is installed, it's impossible to get prometheus metrics out of running notebooks. Just removing /metrics caused issues with third-party code (#36) that depended on the JSON API from the endpoint, and so was reverted. Instead, we 'soft' deprecate that endpoint. 1. Re-add /api/metrics/v1 endpoint, with exact same semantics as /metrics. This is enabled unconditionally. 2. /metrics is also enabled by default, but can be turned off with a notebook config. This allows admins who need prometheus metrics to turn it on. 3. The bundled notebook extension will read from /api/metrics/v1 endpoint, so for those users things will 'just work'. The path to removing /metrics is: 1. Release this as 0.4.0 even though there aren't any breaking changes. This makes it easier for code using legacy endpoints to simply pin to nbresuse<0.4.0 2. Find third party users relying on /metrics, and simply change them to use /api/metrics/v1 - should be a fairly simple change. They can then be pin to nbresuse>=0.4.0 3. When we're sure enough packages have switched, we can release a 0.5.0 where /metrics is disabled by default, but enablable with a config. 4. Eventually, we can simply remove /metrics and the associated config. Ref #36
Brings in jupyter-server/jupyter-resource-usage#68 so we can move towards getting prometheus metrics from notebooks.
|
Thanks! The path to removing /metrics looks good and similar to what was summarized in the roadmap, with one more step of deprecation and backward compatibility with |
| setuptools.setup( | ||
| name="nbresuse", | ||
| version="0.3.6", | ||
| version="0.4.0", |
There was a problem hiding this comment.
Should this be part of a separate release commit?
There was a problem hiding this comment.
It is a separate commit, just in the same PR. Should probably be ok?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
Sounds good 👍
So to summarize:
- 0.4.0 with the soft-deprecated endpoint. Users in lab 2.x will still see the indicator because
/metricsis 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
/metricsendpoint (or enablable with a config) so users with the lab 2.x + nbresuse combination don't see the indicator twice in the status bar
- /metrics now responds with prometheus metrics, rather than nbresuse metrics (jupyter-server/jupyter-resource-usage#68) - Notebook Server's /metrics endpoint is now available without authentication, so prometheus can scrape those metrics (jupyter/notebook#5870) - Annotations are set up on the user pods to tell prometheus the port and path to scrape. - NetworkPolicy is set up to allow prometheus server to hit the singleuser server and the hub pods. Note that this required manually editing netpol in prometheus - should add that feature to the prometheus chart.
|
Thanks for reviewing & engaging, @jtpio! Would you feel comfortable merging this PR? :D |
|
Sure. Was holding a bit more in case you had some input of the summary in #68 (comment) |
|
Would you like to proceed with the release? Otherwise I can do it later today / tomorrow. |
|
@jtpio nope, would be great if you could do the release! |
Brings in jupyter-server/jupyter-resource-usage#68 so we can move towards getting prometheus metrics from notebooks.
Brings in jupyter-server/jupyter-resource-usage#68 so we can move towards getting prometheus metrics from notebooks.
Brings in jupyter-server/jupyter-resource-usage#68 so we can move towards getting prometheus metrics from notebooks.
Brings in jupyter-server/jupyter-resource-usage#68 so we can move towards getting prometheus metrics from notebooks.
Brings in jupyter-server/jupyter-resource-usage#68 so we can move towards getting prometheus metrics from notebooks.
Brings in jupyter-server/jupyter-resource-usage#68 so we can move towards getting prometheus metrics from notebooks.
/metrics is where notebook server publishes prometheus metrics.
Currently, if nbresuse is installed, it's impossible to get
prometheus metrics out of running notebooks.
Just removing /metrics caused issues with third-party
code (#36) that depended
on the JSON API from the endpoint, and so was reverted.
Instead, we 'soft' deprecate that endpoint.
/metrics. This is enabled unconditionally.
a notebook config. This allows admins who need prometheus metrics
to turn it on.
endpoint, so for those users things will 'just work'.
The path to removing /metrics is:
This makes it easier for code using legacy endpoints to simply pin
to nbresuse<0.4.0
to use /api/metrics/v1 - should be a fairly simple change. They can
then be pin to nbresuse>=0.4.0
where /metrics is disabled by default, but enablable with a config.
Ref #36