feat: support sse transfer mode #122
Conversation
|
Don't allow this PR, I will update this PR when the other 2 PR are passed, |
|
When this function is stable, it should be possible to remove websocket dependencies. |
|
We should switch to SSE and not keep an option websockets/SSE. Has no use for the user |
You are righ, I will have further updates after you merge other PR |
| export { init, pushData, slice }; |
There was a problem hiding this comment.
Line breaks automatically added by ide🤣
|
I've noticed no movement for a week. Is there something wrong with this pr? |
Hi. just haven't got the time yet |
|
Okay, continue to wait for your news. Now I am using |
There was a problem hiding this comment.
This pull request does more than what is advertised. In particular, it also handles multiples clients at once, which is an entire topic entirely (#119).
Please keep the changes to the bare minimum, which is to switch from websocket to sse
Now that's 2 different pr's. |
|
It has been a month, have you encountered any difficulties? |
There was a problem hiding this comment.
This PR does more than advertized and it makes it difficult to review.
I'd expect the minimum code to switch the communication from websockets to SSE, nothing more nothing less.
If some stuff needs to be changed because of the websockets-> SSe migration, then please say what and why.
Thank you
statsviz.go
Outdated
| @@ -82,7 +81,7 @@ func Register(mux *http.ServeMux, opts ...Option) error { | |||
| // updates metrics data and provides two essential HTTP handlers: | |||
| // - the Index handler serves Statsviz user interface, allowing you to | |||
| // visualize runtime metrics on your browser. | |||
| // - The Ws handler establishes a WebSocket connection allowing the connected | |||
| // - The Ws handler establishes a data connection allowing the connected | |||
There was a problem hiding this comment.
The Ws handler is used in examples and is part of the public API, but since ws means websockets and the communication won't be performed via websockets anymore, the Ws handler must be renamed to SSE. Removing it would be a breaking change however.
So, we should keep the Ws handler and mark is as deprecated, and make it an alias of the SSE handler.
| panic("unexpected failure to encode plot definitions: " + err.Error()) | ||
| func intercept(h http.Handler, cfg *plot.Config, extraConfig map[string]any) http.HandlerFunc { | ||
| var plotsdefjs []byte | ||
| //Using parentheses helps gc |
There was a problem hiding this comment.
//Using parentheses helps gc
I've never heard of that, can you point to an blog post or a benchmark or anything about that?
internal/static/js/app.js
Outdated
| function throttle(func, delay) { | ||
| let initial = true; | ||
| let last = null; | ||
| let timer = null; | ||
| return function () { | ||
| const context = this; | ||
| const args = arguments; | ||
| if (initial) { | ||
| func.apply(context, args); | ||
| initial = false; | ||
| last = Date.now(); | ||
| } else { | ||
| clearTimeout(timer); | ||
| timer = setTimeout(function () { | ||
| const now = Date.now(); | ||
| if (now - last >= delay) { | ||
| func.apply(context, args); | ||
| last = now; | ||
| } | ||
| }, delay - (Date.now() - last)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const updatePlots = throttle(() => { |
There was a problem hiding this comment.
It can avoid repeated redrawing of DOM caused by browser energy saving mode. This will not affect normal users.
Described in #120
internal/static/js/app.js
Outdated
| @@ -123,7 +156,7 @@ const updatePlots = () => { | |||
| plot.update(xrange, data, shapes); | |||
| } | |||
| }); | |||
| } | |||
| }, PlotsDef.sendFrequency||1000) | |||
There was a problem hiding this comment.
why is the sendFrequency setting used here? it wasn't before and I don't understand why it's needed now. Updates are pushed from the server, so I'd expect browser-side updates to be performed when data gets pushed.
There was a problem hiding this comment.
This sets a minimum time for DOM redrawing, maybe I should set it smaller, such as 100
modify example
|
create a new method |
support #117