Project-Faster / qpep

A working version of qpep standalone go client/server, designed to accelerate high-latency connections, like those provided by geostationary satellites.
https://docs.projectfaster.org
Other
3 stars 1 forks source link

Add feature to indicate session state on tray icon #2

Closed bizzbyster closed 2 years ago

bizzbyster commented 2 years ago

Currently when running the tray icon on Windows, there is no indication that qpep client is "connected" to a qpep server. In addition, it is not possible to know that data is going through qpep. Perhaps we could add a connected indicator and also some kind of activity notification -- maybe a simple activity indicator.

parvit commented 2 years ago

Hi, I was thinking about handling the visual part with an updating icon, right now the tray icon is fixed and with the modificaion you would have a three-state indicator: 1) Current icon: Not Connected / Disabled 2) Blinking Yellow icon: Connecting / Temporary disconnection 3) Green fixed icon: Connected

The underlying changes would be of:

The actual status of the icon would be based on the tray's request result, so right now the client request would be redundant, however i'm inclined to have it so that in the future new features/apis can be added with less effort.

Some of those might include things like dynamic configuration or statistics collection from the server or it also could be a way to let other systems know about the current state of the connection.

Let me know what you think, Vittorio.

bizzbyster commented 2 years ago

I like it. @mfoxworthy can you comment please?

mfoxworthy commented 2 years ago

When we say connected, what does that actually mean?. The proxy itself just proxies connections, but there is no state. Is the connection status just based on the http port? How well will this method scale when there are thousands of clients on a server? Have you thought about how this would work behind a load balancer?

parvit commented 2 years ago

Hi, @mfoxworthy from my initial proposal i've actually expanded a bit what was needed in terms of api.

Two are actually needed: 1) /api/v1/echo which returns a json with the remote address / port of the client (as seen by the server) 2) /api/v1/status/:addr which consumes the address parameter to return a count of the active connections that the server is handling for that client

So basically if you have both the correct echo and at least a running connection than it is counted as connected, instead you would keep the connecting status. Fixed disconnected status is only given if the tray has not launched the local client or server.

This handles in a clean way both failed connections to the server and temporary loss of connection. I suppose it also cover the case for load balancers, because if at least one of the responding servers answers the connection than the path is open.

mfoxworthy commented 2 years ago

@parvit Most excellent. Peter and I just spoke about this, and I was going to update the issue. But part 2 in your last message addresses the topic perfectly. I think we need to think through the load balancer a bit more, but I think this initial version should cover most of the features.

bizzbyster commented 2 years ago

I like the idea of a state variable in the UI --

Current icon: Not Connected / Disabled Blinking Yellow icon: Connecting / Temporary disconnection Green fixed icon: Connected

And then also a metric that indicates activity in some way -- bytes transferred over the tunnel, etc.

bizzbyster commented 2 years ago

@parvit @mfoxworthy thoughts? I'd like to get this feature design nailed down today if possible.

parvit commented 2 years ago

@bizzbyster The collection of statistics as you say would be a considerably higher effort, and it would not be a week effort anymore but something more.

How much more depends on the exact list of statistics you would require to have and the dedicated gui it will need, because i think everything would not fit into the tray icon.

If instead we want to keep that as a next feature to develop i will soon have the change ready to test.

bizzbyster commented 2 years ago

Sounds good -- let's add that as an additional feature. @mfoxworthy agreed?

mfoxworthy commented 2 years ago

@bizzbyster I agree.

parvit commented 2 years ago

Hi @bizzbyster @mfoxworthy, the PR for this feature is ready please test it and send me feedback.

bizzbyster commented 2 years ago

Awesome I will!

mfoxworthy commented 2 years ago

Hi @bizzbyster @mfoxworthy, the PR for this feature is ready please test it and send me feedback.

Wow! That was fast! Thanks!

parvit commented 2 years ago

Hi, was there progress on testing this ?

bizzbyster commented 2 years ago

@parvit I'll be totally honest -- my Windows license seems to have expired and I'm waiting on my IT to help me resolve it. Very annoying! Will provide feedback as soon as I'm able.

parvit commented 2 years ago

well ain't that a bummer. hope you get it sorted out soon.

Il Mar 19 Lug 2022, 20:01 bizzbyster @.***> ha scritto:

@parvit https://github.com/parvit I'll be totally honest -- my Windows license seems to have expired and I'm waiting on my IT to help me resolve it. Very annoying! Will provide feedback as soon as I'm able.

— Reply to this email directly, view it on GitHub https://github.com/Project-Faster/qpep/issues/2#issuecomment-1189392627, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWGJD4FIFIEO3DSRKGSWXWLVU3UOFANCNFSM525MJ5JA . You are receiving this because you were mentioned.Message ID: @.***>

parvit commented 2 years ago

@mfoxworthy what about you? were you able to test?

bizzbyster commented 2 years ago

I'm sorted out and will test this weekend.

bizzbyster commented 2 years ago

We have a few more running over satellite who want to test but I would like to get some basic sense of it working before I offer it.

bizzbyster commented 2 years ago

@parvit I'm testing the new tray icon indicating session state and it seems stuck in waiting...

image

Despite the fact that the server is up and running. I will email you the server IP so maybe you can see if it works for you?

parvit commented 2 years ago

Its really late here so i won't be able to test it right now, tomorrow morning i'll try.

To get one thing out of the way: did you build and use all the three updated binaries together? Meaning the qpep client/server and the tray (obviously the tray you're ok as the icon is the right one).

If so than the client/server should be producing a qpep.log file in the running directory, can you post it? thanks.

bizzbyster commented 2 years ago

I will look for the log file on my test machine.

parvit commented 2 years ago

Seems that by email the test was completed successfully, however i can't close / merge the change as i don't have write access for this repo.

parvit commented 2 years ago

Closing the issue, i'll open a bug for the testing of the api server.