Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add email into access log for shadowsocks and vmess #1767

Merged
merged 1 commit into from
Jul 13, 2019

Conversation

Gondnat
Copy link
Contributor

@Gondnat Gondnat commented Jun 19, 2019

给 shadowsocks 和 vmess 的 access log 里添加了 email 字段,效果如下,如果没有设置 email,则不会显示。
mux模式暂时也无法显示。
image

@hzlmy2002
Copy link

这个功能挺好的,尤其是在一个用户有多个ip地址的情况下

@joviqiao
Copy link

确实好,这个什么时候发布?

@hzlmy2002
Copy link

hzlmy2002 commented Jun 26, 2019

这个改进解决了这个#1406,改动的地方也不多,坐等合并到master分支@kslr

@kslr
Copy link
Contributor

kslr commented Jun 27, 2019

我觉得可以设置为可选项,需要的人可以开启这个功能。

@Gondnat
Copy link
Contributor Author

Gondnat commented Jun 27, 2019

我觉得可以设置为可选项,需要的人可以开启这个功能。

嗯,应该添加一个选择项,mux部分我没太看懂,可以添加email信息进去么?
可以的话,我再完善一下

@kslr kslr requested a review from xiaokangwang June 27, 2019 06:57
@hzlmy2002
Copy link

理论上是可以的,不然通过email进行流量统计没办法实现

@xiaokangwang
Copy link
Contributor

现在有没有程序会去抓取这个log然后进行处理么?这个修改改变了输出的格式,分析log的程序会出现问题吧。

@Gondnat
Copy link
Contributor Author

Gondnat commented Jun 28, 2019

现在有没有程序会去抓取这个log然后进行处理么?这个修改改变了输出的格式,分析log的程序会出现问题吧。

把email放到最后应该,就不会有影响,如下这种
image

@Gondnat
Copy link
Contributor Author

Gondnat commented Jun 28, 2019

把 Email 地址放到 每一行的末尾,以保证可能的 log 分析软件兼容。
image

@Gondnat
Copy link
Contributor Author

Gondnat commented Jul 1, 2019

我觉得可以设置为可选项,需要的人可以开启这个功能。

设置,我想的是在 "app/log/config.proto", 添加配置
bool access_log_email = 6;
然后在 "app/log/log.go" 的
func (g *Instance) Handle(msg log.Message)
添加判断,如果 access_log_email 没开,则将 Email 置为空

case *log.AccessMessage:
	if g.accessLogger != nil {
		if !g.config.AccessLogEmail {
			msg.Email = ""
		}
		g.accessLogger.Handle(msg)
	}

@kslr @xiaokangwang 看一下还有别的更好的方式么?

@eycorsican
Copy link
Contributor

我觉得这种修改最难的点在于如何在 JSON 配置文件中安排这个开关,因为那面向用户,也要求版本间兼容,你不可能这个版本加上去后,觉得不好,下个版本又修改或者撤回,那很麻烦。

我的建议是不需要在 JSON 配置里加入开关,考虑到 @xiaokangwang 的担忧,我觉得那并不算什么大问题,写抓取程序的开发者升级后对应修改就可以,因为 access log 的字段并没有规范文档,v2ray 没必要一定保证它的一致性,其它类似的修改也方便很多,比如有个 PR 也在 access log 里加入了 inbound tag 或 outbound tag。

另外如果要合并,小更改建议重新 push 到一个 commit 里,方便其它开发者查看 master 上的更改。

@Gondnat
Copy link
Contributor Author

Gondnat commented Jul 2, 2019

我觉得这种修改最难的点在于如何在 JSON 配置文件中安排这个开关,因为那面向用户,也要求版本间兼容,你不可能这个版本加上去后,觉得不好,下个版本又修改或者撤回,那很麻烦。

我的建议是不需要在 JSON 配置里加入开关,考虑到 @xiaokangwang 的担忧,我觉得那并不算什么大问题,写抓取程序的开发者升级后对应修改就可以,因为 access log 的字段并没有规范文档,v2ray 没必要一定保证它的一致性,其它类似的修改也方便很多,比如有个 PR 也在 access log 里加入了 inbound tag 或 outbound tag。

另外如果要合并,小更改建议重新 push 到一个 commit 里,方便其它开发者查看 master 上的更改。

我把log合并了一下。
config 我确实找不出来太优雅的解决方案。暂时还没有放到提交里

@xiaokangwang
Copy link
Contributor

现在并不清楚是否有人使用了V2Ray这里的日志。但是我觉得其他程序应该是使用API来访问这里的功能的吧。如果有人使用程序来抓取的话,这里应该是要么正则匹配失败,要么是把email放到放到域名里面。

@xiaokangwang
Copy link
Contributor

现在比较担心的是一些程序可能会缺少维护,导致突然不能用。

@kslr 你觉得可以通过把这个修改列入changelog,来避免这个修改对于其他程序的影响么?

@joviqiao
Copy link

这玩意讨论半天

@kslr
Copy link
Contributor

kslr commented Jul 10, 2019

不好意思,我刚刚看到新消息。
如果某些平台用户需要解析日志,他们可以固定在某个版本,评估后升级,我想很少有人会在生产环境开自动升级。
所以我觉得现在可以合并,我会在Changelog中描述这个情况。

Copy link
Contributor

@xiaokangwang xiaokangwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on @kslr 's opinion, I believe this pull request is ready to be merged.

@kslr kslr merged commit 2451eed into v2ray:master Jul 13, 2019
@joviqiao
Copy link

@Gondnat 我已经升级到最新了,但是没有email,怎么让打印email? 我看到这个功能已经发布了呢

@Gondnat
Copy link
Contributor Author

Gondnat commented Jul 15, 2019

@Gondnat 我已经升级到最新了,但是没有email,怎么让打印email? 我看到这个功能已经发布了呢

4.20.0没有包括这个改动

@kslr
Copy link
Contributor

kslr commented Jul 15, 2019

如果你立刻需要,就只能按照指南先行编译。

@joviqiao
Copy link

收到,谢谢

@Gondnat Gondnat deleted the addEmailIntoAccessLog branch July 15, 2019 09:50
@cooper-q
Copy link

请问 添加email这个功能大约 什么时候发版本

@famdude
Copy link

famdude commented Oct 25, 2022

Can I ask you how should I activate this feature? I can's see the email, in log file.
@kslr

@famdude
Copy link

famdude commented Oct 25, 2022

Can I ask you how should I activate this feature? I can's see the email, in log file. @kslr

An also, how should I set email for users?!

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.

8 participants