Allow specifying extra params to scrub from logs #1538
Conversation
jtpio
force-pushed
the
configurable-scrub-param-keys
branch
from
996ec7d to
6732048
Compare
|
CI is all passing ✅ This should now be ready for review! |
jupyter_server/log.py
Outdated
| @@ -59,6 +62,10 @@ def log_request(handler, record_prometheus_metrics=True): | |||
| except AttributeError: | |||
| logger = access_log | |||
|
|
|||
| extra_param_keys = [] | |||
| if hasattr(handler, "serverapp") and hasattr(handler.serverapp, "extra_log_scrub_param_keys"): | |||
| extra_param_keys = handler.serverapp.extra_log_scrub_param_keys | |||
There was a problem hiding this comment.
Would it be more "standard" to put this in the settings for the app? Then you could use handler.settings.get("extra_log_scrub_param_keys")?
There was a problem hiding this comment.
That would also allow other extensions to add keys to this in init_settings.
There was a problem hiding this comment.
Thanks @vidartf for the review.
I guess it's fine to have it in the settings yes. And we can keep it as a trait in the app, so it's still easily configurable?
If so I can push such change tomorrow.
There was a problem hiding this comment.
I just pushed a new commit, let me know if that's what you had in mind.
jupyter_server/serverapp.py
Outdated
| @@ -2006,6 +2007,24 @@ def _default_terminals_enabled(self) -> bool: | |||
|
|
|||
| Set to False to disable recording the http_request_duration_seconds metric. | |||
| """, | |||
| config=True, | |||
There was a problem hiding this comment.
This makes this other key configurable. Was this intentional?
There was a problem hiding this comment.
Ah thanks for catching this. Fixed in cf006d8
Fixes #1536
c.ServerApp.extra_log_scrub_param_keysCurrently the extra param keys do not replace the default set of keys, to avoid altering the current behavior by mistake. Instead they are added to the current set of param keys.