Xray-core icon indicating copy to clipboard operation
Xray-core copied to clipboard

API: Add GetAllOnlineUsers RPC to StatsService for retrieving online users

Open mr1cloud opened this issue 3 months ago • 32 comments

Added GetAllOnlineUsers method to stats manager and CLI command for getting this info.

Execute:

.\xray.exe api statsgetallonlineusers -server="127.0.0.1:8080"

Result:

{
    "users": [
        "user>>>user1>>>online"
    ]
}

Maybe this functionality will come in handy.

mr1cloud avatar Sep 02 '25 07:09 mr1cloud

OMG finally

ImMohammad20000 avatar Sep 02 '25 07:09 ImMohammad20000

OMG finally

🤣🤣🤣 its sooo funny comment

mr1cloud avatar Sep 02 '25 07:09 mr1cloud

我觉得cmd里已经有很多为机场服务的杂乱的user相关的api了

Fangliding avatar Sep 02 '25 07:09 Fangliding

我觉得cmd里已经有很多为机场服务的杂乱的user相关的api了

I can rename new method or remove it from cli.

mr1cloud avatar Sep 02 '25 07:09 mr1cloud

Add GetAllUsersOnlineIpList too

M03ED avatar Sep 02 '25 08:09 M03ED

Why core need this method?

Core already have gRPC endpoint to get user traffic stats, if you will request this endpoint every 15 seconds (with reset) – so you already have "online" users..

kastov avatar Sep 02 '25 08:09 kastov

lol, why just not count 0 in traffic?

Jolymmiles avatar Sep 02 '25 08:09 Jolymmiles

Why core need this method?

Core already have gRPC endpoint to get user traffic stats, if you will request this endpoint every 15 seconds (with reset) – so you already have "online" users..

The problem is that all panels use logs to get the user's online status or they use the GetStatsOnline method on an array of clients, which is illogical when this method can be implemented natively in the core.

mr1cloud avatar Sep 02 '25 08:09 mr1cloud

lol, why just not count 0 in traffic?

What? I dont understand what that means

mr1cloud avatar Sep 02 '25 09:09 mr1cloud

Why core need this method? Core already have gRPC endpoint to get user traffic stats, if you will request this endpoint every 15 seconds (with reset) – so you already have "online" users..

The problem is that all panels use logs to get the user's online status or they use the GetStatsOnline method on an array of clients, which is illogical when this method can be implemented natively in the core.

Why panels just do not use xray.app.stats.command.QueryStatsRequest with pattern: user>>> and reset: true?

If you will request this query every 15 seconds with reset:true, u will have users bandwidth, so if user bandwidth for this period is not zero – user was online.

kastov avatar Sep 02 '25 09:09 kastov

Why core need this method? Core already have gRPC endpoint to get user traffic stats, if you will request this endpoint every 15 seconds (with reset) – so you already have "online" users..

The problem is that all panels use logs to get the user's online status or they use the GetStatsOnline method on an array of clients, which is illogical when this method can be implemented natively in the core.

Why panels just do not use xray.app.stats.command.QueryStatsRequest with pattern: user>>> and reset: true?

If you will request this query every 15 seconds with reset:true, u will have users bandwidth, so if user bandwidth for this period is not zero – user was online.

Why use this complicated logic when there is an api for it!? You can simply hand over it to core!

ImMohammad20000 avatar Sep 02 '25 09:09 ImMohammad20000

Why use this complicated logic when there is an api for it!? You can simply hand over it to core!

Why it is complicated? Almost every panel always get user bandwidth anyway (with queryStat!), so why not to calculate online users from information which you already got?

kastov avatar Sep 02 '25 09:09 kastov

If panel is not using queryStat for getting user traffic, so how it manages bandwidth usage for users?

kastov avatar Sep 02 '25 09:09 kastov

i still think having a api for geting online users is good

ImMohammad20000 avatar Sep 02 '25 09:09 ImMohammad20000

Why use this complicated logic when there is an api for it!? You can simply hand over it to core!

Why it is complicated? Almost every panel always get user bandwidth anyway (with queryStat!), so why not to calculate online users from information which you already got?

Regarding the issue of accounting for each user's traffic, QueryStat is a terrible solution. It only shows the traffic that is currently passing through.

mr1cloud avatar Sep 02 '25 09:09 mr1cloud

I suggest we stop now, take a break, and come back with new ideas. Or wait for the owner's decision.

mr1cloud avatar Sep 02 '25 09:09 mr1cloud

IMO, why not return the OnlineMap.Count() like GetStatsOnline do, providing all the number of user IPs? QueryStatscan't help this.

hexaphaseIce avatar Sep 02 '25 10:09 hexaphaseIce

IMO, why not return the OnlineMap.Count() like GetStatsOnline do, providing all the number of user IPs? QueryStatscan't help this.

This is not a bad idea, but why get a bundle of ip addresses of all active users? The essence of the method is to get active users, just using one method.

mr1cloud avatar Sep 02 '25 10:09 mr1cloud

IMO, why not return the OnlineMap.Count() like GetStatsOnline do, providing all the number of user IPs? QueryStatscan't help this.

This is not a bad idea, but why get a bundle of ip addresses of all active users? The essence of the method is to get active users, just using one method.

Having users IP list is more useful than just seeing which user is online or no, as others said most of user management projects already implemented this with just using query stats. Getting all users IP with single request is way better and can be used for ip limit projects

M03ED avatar Sep 02 '25 10:09 M03ED

IMO, why not return the OnlineMap.Count() like GetStatsOnline do, providing all the number of user IPs? QueryStatscan't help this.

This is not a bad idea, but why get a bundle of ip addresses of all active users? The essence of the method is to get active users, just using one method.

Having users IP list is more useful than just seeing which user is online or no, as others said most of user management projects already implemented this with just using query stats. Getting all users IP with single request is way better and can be used for ip limit projects

Okay, I added this functionality in the future.

mr1cloud avatar Sep 02 '25 10:09 mr1cloud

IMO, why not return the OnlineMap.Count() like GetStatsOnline do, providing all the number of user IPs? QueryStatscan't help this.

This is not a bad idea, but why get a bundle of ip addresses of all active users? The essence of the method is to get active users, just using one method.

Having users IP list is more useful than just seeing which user is online or no, as others said most of user management projects already implemented this with just using query stats. Getting all users IP with single request is way better and can be used for ip limit projects

And i already added method to getting user IP list #4360

mr1cloud avatar Sep 02 '25 10:09 mr1cloud

And i already added method to getting user IP list #4360

Thanks for your contribution but that's for a single user, when we have a high amount of users ( more than 1K ) we need to make request for every single user one by one and this will take lots of resources

M03ED avatar Sep 02 '25 11:09 M03ED

IMO, why not return the OnlineMap.Count() like GetStatsOnline do, providing all the number of user IPs? QueryStatscan't help this.

This is not a bad idea, but why get a bundle of ip addresses of all active users? The essence of the method is to get active users, just using one method.

Not a bundle of ip addresses but the count of them for each user, it just the count of connected devices/clients for each user/uuid.

I mean return a list both of online user and its devices count, in this PR we just get user name.

It can still get all active users, just using one method, and provide devices count as well. It's also easier to query how many clients connected to the server.

Anyway, thanks for your contribution in providing these APIs.

hexaphaseIce avatar Sep 02 '25 11:09 hexaphaseIce

IMO, why not return the OnlineMap.Count() like GetStatsOnline do, providing all the number of user IPs? QueryStatscan't help this.

This is not a bad idea, but why get a bundle of ip addresses of all active users? The essence of the method is to get active users, just using one method.

Not a bundle of ip addresses but the count of them for each user, it just the count of connected devices/clients for each user/uuid.

I mean return a list both of online user and its devices count, in this PR we just get user name.

It can still get all active users, just using one method, and provide devices count as well. It's also easier to query how many clients connected to the server.

Anyway, thanks for your contribution in providing these APIs.

Having just count of devices is not the best option, situations like using marzban that provide using multiple nodes can face some problem, for example if the user just switches between servers to just test them were not gonna realize it's the same IP, but if we get list of ips with timestamp of there last connection we can detect this and prevent conflicts.

M03ED avatar Sep 02 '25 11:09 M03ED

不应当由 Xray-core 定义什么是 “online”,应当返回所有用户的最后在线时间以及最近几个 IP、流量统计,一个命令解决所有问题

RPRX avatar Sep 03 '25 02:09 RPRX

然后会有人说机场一次性抛20万个用户出来巨型回复处理不了 有的需求不该管的就不该有

Fangliding avatar Sep 03 '25 02:09 Fangliding

不过这个是基于 onlineMap 吗,我还没看那个是什么,总之这个 PR 不急着合并,流量统计本身就能定义是否 online

RPRX avatar Sep 03 '25 02:09 RPRX

不过这个是基于 onlineMap 吗,我还没看那个是什么,总之这个 PR 不急着合并,流量统计本身就能定义是否 online

I think statistics are not the best way to get the online status of a user.

A little later, I will implement a method that will combine all requests at once.

mr1cloud avatar Sep 03 '25 06:09 mr1cloud

Namely: getting the client's ip addresses, as is done in my GetStatsOnlineIpList method, and traffic (as you say).

Have I forgotten anything?

mr1cloud avatar Sep 03 '25 06:09 mr1cloud

@RPRX When you are update the documents for vless encryption?

ImMohammad20000 avatar Sep 03 '25 06:09 ImMohammad20000