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.

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

Reporting to Socket.IO Admin UI #1164

Merged
merged 14 commits into from
Oct 15, 2023
Merged

Reporting to Socket.IO Admin UI #1164

merged 14 commits into from
Oct 15, 2023

Conversation

miguelgrinberg
Copy link
Owner

@miguelgrinberg miguelgrinberg commented Mar 30, 2023

  • Instrumentation for sync server
  • Instrumentation for async server
  • Unit tests
  • Documentation
  • Multi-server support
    • Enter and leave room requests should go on the message queue
    • Nodes should routinely report their connected clients to all admins
    • All nodes should emit "server_stats" at regular intervals, regardless of admins connected or not

@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2023

Codecov Report

Merging #1164 (58f35db) into main (6bdc498) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              main     #1164    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           29        31     +2     
  Lines         2021      2405   +384     
  Branches       373       431    +58     
==========================================
+ Hits          2021      2405   +384     
Files Coverage Δ
src/socketio/admin.py 100.00% <100.00%> (ø)
src/socketio/async_admin.py 100.00% <100.00%> (ø)
src/socketio/async_manager.py 100.00% <100.00%> (ø)
src/socketio/async_server.py 100.00% <100.00%> (ø)
src/socketio/async_simple_client.py 100.00% <100.00%> (ø)
src/socketio/base_manager.py 100.00% <100.00%> (ø)
src/socketio/server.py 100.00% <100.00%> (ø)
src/socketio/simple_client.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Seluj78
Copy link

Seluj78 commented Apr 12, 2023

Amazing thank you ! Can't wait to implement and test this :D

@Seluj78
Copy link

Seluj78 commented May 9, 2023

Hi @miguelgrinberg :)

Is there any news on this ? :)

@miguelgrinberg
Copy link
Owner Author

@Seluj78 I think this implementation is complete, so you are welcome to use it.

I will not merge it until I come up with a good testing strategy though.

@Seluj78
Copy link

Seluj78 commented Jun 6, 2023

Hi @miguelgrinberg

I tried testing it with Flask-Socketio, and here's the error I'm getting

  File "/Users/seluj78/Projects/xxx/.venv/src/python-socketio/src/socketio/admin.py", line 47, in __init__
    self.sio.manager.host_id if hasattr(self.sio.manager, 'host_id')
                                        ^^^^^^^^^^^^^^^^
AttributeError: 'SocketIO' object has no attribute 'manager'

The code:

socketio = SocketIO(application)
socketio.init_app(application, cors_allowed_origins="*")
InstrumentedServer(socketio, auth=False)

@miguelgrinberg
Copy link
Owner Author

@Seluj78 for Flask-SocketIO use this:

InstrumentedServer(socketio.server, auth=False)

@Seluj78
Copy link

Seluj78 commented Jun 7, 2023

The server started without a problem now, but when I enter connection details (http://127.0.0.1:5000/admin and no auth) on https://admin.socket.io/#/, I get this error

  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/async_drivers/threading.py", line 23, in __call__
    return self.app(self)
           ^^^^^^^^^^^^^^
TypeError: InstrumentedServer._eio_websocket_handler() missing 1 required positional argument: 'ws'
post request handler error
Traceback (most recent call last):
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py", line 430, in handle_request
    socket.handle_post_request(environ)
TypeError: InstrumentedServer._eio_handle_post_request() missing 1 required positional argument: 'environ'

Let me know if you need the full error log

@miguelgrinberg
Copy link
Owner Author

miguelgrinberg commented Jun 7, 2023

I think this is likely a mistake on my part, not something you are doing wrong. I tested mostly websocket connections. Are you working with polling connections?

EDIT: actually I don't know, it seems to work well for me. I'm using the app.py example from the Flask-SocketIO repo, with these changes:

diff --git a/example/app.py b/example/app.py
index 24c464e..bd275da 100644
--- a/example/app.py
+++ b/example/app.py
@@ -11,10 +11,13 @@ async_mode = None

 app = Flask(__name__)
 app.config['SECRET_KEY'] = 'secret!'
-socketio = SocketIO(app, async_mode=async_mode)
+socketio = SocketIO(app, async_mode=async_mode, cors_allowed_origins="https://admin.socket.io")
 thread = None
 thread_lock = Lock()

+from socketio.admin import InstrumentedServer
+InstrumentedServer(socketio.server, auth=False)
+

 def background_thread():
     """Example of how to send server generated events to clients."""

@Seluj78
Copy link

Seluj78 commented Jun 21, 2023

Weird...

My code:

socketio = SocketIO(application)
socketio.init_app(application, cors_allowed_origins="*")
InstrumentedServer(socketio.server, auth=False)

And the error:

21-06-2023:15:56:58,894 WARNING  [/Users/seluj78/Projects/Datascientest/hub-backend/.venv/lib/python3.11/site-packages/werkzeug/_internal.py:187 (_log()] 75927:  * Debugger is active!
post request handler error
Traceback (most recent call last):
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py", line 430, in handle_request
    socket.handle_post_request(environ)
TypeError: InstrumentedServer._eio_handle_post_request() missing 1 required positional argument: 'environ'
21-06-2023:15:57:07,118 ERROR    [/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py:439 (handle_request()] 75927: post request handler error
Traceback (most recent call last):
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py", line 430, in handle_request
    socket.handle_post_request(environ)
TypeError: InstrumentedServer._eio_handle_post_request() missing 1 required positional argument: 'environ'
Traceback (most recent call last):
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/lib/python3.11/site-packages/sentry_sdk/integrations/flask.py", line 88, in sentry_patched_wsgi_app
    return SentryWsgiMiddleware(lambda *a, **kw: old_app(self, *a, **kw))(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/lib/python3.11/site-packages/sentry_sdk/integrations/wsgi.py", line 140, in __call__
    reraise(*_capture_exception(hub))
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/lib/python3.11/site-packages/sentry_sdk/_compat.py", line 54, in reraise
    raise value
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/lib/python3.11/site-packages/sentry_sdk/integrations/wsgi.py", line 133, in __call__
    rv = self.app(
         ^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/lib/python3.11/site-packages/sentry_sdk/integrations/flask.py", line 88, in <lambda>
    return SentryWsgiMiddleware(lambda *a, **kw: old_app(self, *a, **kw))(
                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/lib/python3.11/site-packages/flask/app.py", line 2551, in __call__
    return self.wsgi_app(environ, start_response)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/flask-socketio/src/flask_socketio/__init__.py", line 43, in __call__
    return super(_SocketIOMiddleware, self).__call__(environ,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/middleware.py", line 63, in __call__
    return self.engineio_app.handle_request(environ, start_response)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/lib/python3.11/site-packages/socketio/server.py", line 607, in handle_request
    return self.eio.handle_request(environ, start_response)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py", line 410, in handle_request
    packets = socket.handle_get_request(
              ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/socket.py", line 103, in handle_get_request
    return getattr(self, '_upgrade_' + transport)(environ,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/socket.py", line 158, in _upgrade_websocket
    return ws(environ, start_response)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/async_drivers/threading.py", line 23, in __call__
    return self.app(self)
           ^^^^^^^^^^^^^^
TypeError: InstrumentedServer._eio_websocket_handler() missing 1 required positional argument: 'ws'
post request handler error
Traceback (most recent call last):
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py", line 430, in handle_request
    socket.handle_post_request(environ)
TypeError: InstrumentedServer._eio_handle_post_request() missing 1 required positional argument: 'environ'
21-06-2023:15:57:32,138 ERROR    [/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py:439 (handle_request()] 75927: post request handler error
Traceback (most recent call last):
  File "/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py", line 430, in handle_request
    socket.handle_post_request(environ)
TypeError: InstrumentedServer._eio_handle_post_request() missing 1 required positional argument: 'environ'
Invalid session _RekZpUcl1pM90fPAAAA (further occurrences of this error will be logged with level INFO)
21-06-2023:15:58:07,133 ERROR    [/Users/seluj78/Projects/Datascientest/hub-backend/.venv/src/python-engineio/src/engineio/server.py:742 (_log_error_once()] 75927: Invalid session _RekZpUcl1pM90fPAAAA (further occurrences of this error will be logged with level INFO)

@miguelgrinberg
Copy link
Owner Author

@Seluj78 I have pushed some fixes into this branch. Let me know if they help.

@Seluj78
Copy link

Seluj78 commented Sep 11, 2023

I haven't tried in a while but I just cloned this branch again and installed it and it seems like it's working ! I just added InstrumentedServer(socketio.server, auth=False) to test it with https://admin.socket.io :) Can't wait for it to be merged !

@miguelgrinberg
Copy link
Owner Author

@Seluj78 the app.py examples in examples/server/wsgi and examples/server/asgi (in this PR branch) have the code to add instrumentation in the recommended way.

@Seluj78
Copy link

Seluj78 commented Sep 11, 2023

@Seluj78 the app.py examples in examples/server/wsgi and examples/server/asgi (in this PR branch) have the code to add instrumentation in the recommended way.

Great ! I will check these out, I was just testing quickly.

Once you've merged this I will test it more thoroughly and deploy it in production where we have about 500 active connections at the same time.
Do you have any idea/benchmark of the performance impact that the instrumentation have ? It might not be a great idea to have it running in production if it slows down the server by a substantial amount.

@miguelgrinberg
Copy link
Owner Author

@Seluj78 if full instrumentation affects performance you can instrument for production mode, which disables the most expensive monitoring features. This is all based on the JavaScript implementation.

@Seluj78
Copy link

Seluj78 commented Sep 11, 2023

Indeed I've just seen that, thank you. I was wondering what the performance impact is, but idk how to benchmark it. I'll try and see :)

@miguelgrinberg miguelgrinberg merged commit 4bf4877 into main Oct 15, 2023
19 checks passed
@miguelgrinberg miguelgrinberg deleted the admin-ui branch October 15, 2023 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants