Together-Java / TJ-Bot

TJ-Bot is a Discord Bot used on the Together Java server. It is maintained by the community, anyone can contribute.
https://togetherjava.org
GNU General Public License v3.0
98 stars 82 forks source link

Add a command to check JShell sessions #1119

Open Alathreon opened 1 month ago

Alathreon commented 1 month ago

Is your feature request related to a problem? Please describe. Currently, sessions are hard to monitor, users can't know if their session is alive and mods can't know how the sessions are doing.

Describe the solution you'd like In order to more easily monitor the sessions for both the users and more importantly mods, a new command should be added, displaying the sessions and information about them.

Id of the session                    | Expire in | Time since creation | Total eval time  | Doing an operation
175710436967055361                   |    5min   | 35min               | 50s              | true
e7a6feaf-d815-4bd1-ae9a-2daac3bbad33 |    10s    | 50s                 | 2s               | false

Describe alternatives you've considered Do nothing.

Additional context It is for the new JShell command which allow users to run custom code.

marko-radosavljevic commented 1 month ago

Sure, that would help, but the main thing it should include is the user/userID. So we can see if someone is abusing it, check what are they running, etc.

Time since creation/expire in/name of the session is more useful for debugging and figuring out if something is wrong with the sessions/containers. But it's quite low-level, it's info you can gather from the vps, where you would do debugging and investigation anyway.

For discord command, I think higher level overview is more important, users and what they are running, how they are using it, maybe how often they use it. etc is more valuable. From that context, session ID and creation time doesn't help much, it's too low level.

It's technical details we don't care about, and they are hard to interpret, how would you know if everything is okay or not, at first glance? Session ID is meaningless, because I don't know who it belongs to, what is running inside it, is there an issue with it, or it's running properly.

Alathreon commented 1 month ago

it should include is the user/userID

It does include it, I guess I could simplify it without the session_ part, alright, modified The id of the session is either the id of the user or an uuid if it is a one-time session I guess an improvment would be to replace the id of the user by the user itself, it's more complex, because you can't control username length so they can mess up with the table formatting. On top of that, while it's better for user, it's worse for us. I just added total eval time so you can see if an user is trying to spam the command.

But let me be clear, the idea of this command is simple : allow the users to check their sessions (or sessions of other), and also for mods to check the state of the sessions without needing to go check the VPS. So the goal of this is rather to monitor backend session as a complement of the logs, rather than monitor the users.

For what you are asking, monitoring the users, it will be a separate PR, because this current PR is mainly about the ability of the back to send more data, but in order to monitor the user, rather than a modification of the back, it will require to store user commands and results in a database so we can check if they are trying to do anything, because once a session is destroyed, data about it will be destroyed.

Alathreon commented 1 month ago

@marko-radosavljevic

marko-radosavljevic commented 1 month ago

Sure, I guess.

For me, checking the state of containers is faster and more comfortable on the vps. So this future is not that useful to me. And if I need to investigate/debug containers, I will have to be on the vps anyway. Also, while I'm there, I can also check logs, or present this data in a more useful way UI wise. What would be more useful is a more abstract overview of the system, something I discussed in my previous comment, and something that can be used for monitoring and moderation purposes. Also, alerts would be even more useful than monitoring. But I already commented on that, and I understand that this is not the goal of this feature.

Users being able to check their session can be useful, I guess? Haven't actually used jshell for real, so not sure what QoL features it needs. But 90%+ of our users don't even know how to use slash commands, and 99%+ don't know how to use jshell. And from that perspective, I think it would be very rare that someone wants or needs this. Maybe you can give me a user story/use case that I'm missing? It just seems too low level and technical, and info that the end user shouldn't ever have to even think about. It should be automatic, in it just works way and abstracted away.

If it's a common problem that users do not know if their session is alive, and how much time is left, maybe there is a way to nicely display session timer while using jshell? It should be as simple as possible to be aware of your sessions and to manage them. Remember, most of our users are not power users, don't know how to use slash commands or jshell. So the goal is to have an intuitive jshell to use, with low barrier to entry, with good UI/UX, and (ideally) never having to 'debug' it with technical commands and additional info.

I don't think users should be able to check sessions of other users, I don't think they would even want or need something like that, and it might be privacy invading if someone is just semi-privately exploring something in a thread/spam. So maybe limit this to the staff ambassadors+?

marko-radosavljevic commented 1 month ago

I will use jshell more, if nothing, just to test current UI/UX flow, what QoL features are missing, is it intuitive to use, and if some extra info about sessions is useful or needed.

I don't really mind it tho, it seems like a relatively low effort thing, and we can easily improve it or change it later.