Add telemetry events for Jupyterhub REST APIs #3218
Conversation
kiendang
force-pushed
the
event-schemas
branch
from
48b04ef
to
c799a7f
Compare
kiendang
force-pushed
the
event-schemas
branch
from
c799a7f
to
22e7311
Compare
Originally sorting was added for easier testing
jupyterhub/apihandlers/auth.py
Outdated
| self.eventlog.record_event( | ||
| eventlogging_schema_fqn('auth-token-action'), | ||
| 1, | ||
| {'action': 'get', 'token_id': token}, |
There was a problem hiding this comment.
We shouldn't include the token itself in events. This should be orm_token.api_id
jupyterhub/apihandlers/auth.py
Outdated
| self.eventlog.record_event( | ||
| eventlogging_schema_fqn('auth-token-action'), | ||
| 1, | ||
| {'action': 'create', 'token_id': api_token,}, |
jupyterhub/apihandlers/users.py
Outdated
| 1, | ||
| { | ||
| 'action': 'create', | ||
| 'target_user': {'username': user.name, 'admin': False}, |
There was a problem hiding this comment.
| 'target_user': {'username': user.name, 'admin': False}, | |
| 'target_user': {'username': user.name, 'admin': admin}, |
jupyterhub/apihandlers/users.py
Outdated
| 1, | ||
| { | ||
| 'action': 'get', | ||
| 'target_user': {'username': user.name, 'admin': user.admin}, |
There was a problem hiding this comment.
Let's try to use the same fields as the user model, so 'name' here so it's user.name instead of the redundant user.username
| 'requester': self.current_user.name, | ||
| 'target_user': {'username': user.name, 'admin': user.admin}, | ||
| 'prior_state': prior_state, | ||
| 'auth_state_change': 'auth_state' in data, |
jupyterhub/apihandlers/users.py
Outdated
| 1, | ||
| { | ||
| 'action': 'get', | ||
| 'target_user': user.name, |
There was a problem hiding this comment.
should target_user always have the same schema ({name, admin})?
There was a problem hiding this comment.
should we do the same for requester everywhere as well?
jupyterhub/apihandlers/users.py
Outdated
| 'action': 'delete', | ||
| 'target_user': user.name, | ||
| 'requester': self.current_user.name, | ||
| 'token_id': token_id, |
There was a problem hiding this comment.
| 'token_id': token_id, | |
| 'token_id': token.api_id, |
same schema for /authorizations/token and /users/{name}/tokens endpoints
|
I've marked this as draft for now. @minrk @yuvipanda What do you think are the next steps for this PR (and presumably jupyter-server/jupyter_server#364)? |
|
I think it just needs review (and now conflict resolution). @yuvipanda do you want to have a look over the schemas? I can handle the conflicts. |
Add event schemas + code for emitting those events for the endpoints in Jupyterhub REST APIs.