Skip to content

Conversation

@mr1cloud
Copy link
Contributor

@mr1cloud mr1cloud commented Sep 2, 2025

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.

@ImMohammad20000
Copy link

OMG finally

@mr1cloud
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

OMG finally

🤣🤣🤣 its sooo funny comment

@Fangliding
Copy link
Member

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

@mr1cloud
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

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

I can rename new method or remove it from cli.

@M03ED
Copy link
Contributor

M03ED commented Sep 2, 2025

Add GetAllUsersOnlineIpList too

@kastov
Copy link
Contributor

kastov commented Sep 2, 2025

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..

@Jolymmiles
Copy link
Contributor

lol, why just not count 0 in traffic?

@mr1cloud
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

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
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

lol, why just not count 0 in traffic?

What? I dont understand what that means

@kastov
Copy link
Contributor

kastov commented Sep 2, 2025

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.

@ImMohammad20000
Copy link

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!

@kastov
Copy link
Contributor

kastov commented Sep 2, 2025

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
Copy link
Contributor

kastov commented Sep 2, 2025

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

@ImMohammad20000
Copy link

ImMohammad20000 commented Sep 2, 2025

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

@mr1cloud
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

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
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

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

@hexaphaseIce
Copy link

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

@mr1cloud
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

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.

@M03ED
Copy link
Contributor

M03ED commented Sep 2, 2025

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

@mr1cloud
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

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
Copy link
Contributor Author

mr1cloud commented Sep 2, 2025

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

@M03ED
Copy link
Contributor

M03ED commented Sep 2, 2025

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

@hexaphaseIce
Copy link

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.

@M03ED
Copy link
Contributor

M03ED commented Sep 2, 2025

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.

@RPRX
Copy link
Member

RPRX commented Sep 3, 2025

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

@Fangliding
Copy link
Member

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

@RPRX
Copy link
Member

RPRX commented Sep 3, 2025

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

@mr1cloud
Copy link
Contributor Author

mr1cloud commented Sep 3, 2025

不过这个是基于 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
Copy link
Contributor Author

mr1cloud commented Sep 3, 2025

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

Have I forgotten anything?

@ImMohammad20000
Copy link

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

@mr1cloud
Copy link
Contributor Author

mr1cloud commented Sep 3, 2025

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

This discussion is not related to this question. Better open the issue....

@yuhan6665
Copy link
Member

Code-wise looks ok

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.

9 participants