-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(log): 为 sniff 增加一个 log 信息 (#3863) #3865
base: main
Are you sure you want to change the base?
Conversation
添加了一个 sniffLog 字段到 log 中,允许它在嗅探到域名后输出一段日志
牛,这个效率太高了。 |
你们说有这个功能需求嘛,我不知道,总之我先提交个 PR 在这里,请大佬们判断 |
感觉有一些 break 比如这样的话snifflog默认被关闭了 当然可以改成loglevel=info或以上的时候自动打开这个选项 但是这样又会让log输出的地方改掉 当当然还可以选择要不要输出在access log里 但是这样又会多复杂度 也不好 log应该是诊断用途而后是审计 所以这种事最好还是别
而且ctx也没了 |
确实,本以为向下兼容,结果忘了这一点
首先排除这种方法
我这次提交...好像就是解决这个问题来的来着...?
本来我也是奔着让几位核心大佬改改再合并的,忘记说了,我是我们组里写日志信息写得最烂的一个💦 这个得来个人帮忙改一下 那...考虑好采取哪种方法了吗?还有更好的见解嘛,还是说还需要更多人来讨论讨论
好吧,说实话我才意识到那一堆信息是上下文,我一直以为那是 log 内部的内容,所以...弱弱地问一句,它很重要嘛,我还真不知道 |
hi @Fangliding ,技术上我不懂,但从出发性上我可以补充一下。 诚然,我提这个feature request最开始是为了在grafana中统计特定url的访问次数。是出于审计目的。😃但设想一个诊断用途:在set up xray的过程中,我需要判断一个对某一url的请求是如何被xray处理的。这个时候如果能够看到url,将可以极大的提升log的可读性。否则我必须先自己把url对应的ip通过dns查询出来,然后再依照ip比较log记录。这首先带来了额外工作量,其次我认为另一个问题在于dns解析。xray有自己的dns处理机制,而有些url可能会被污染。我觉得会有一种可能性是客户机查询出的ip和xray查询出的ip不一致。 另外我明白你的“tproxy收到的是ip所以就显示ip”。这个feature request可能确实和设计理念有点不一致,但我还是觉得在log里看到url会对troubleshooting很有帮助。无论怎样,我都理解并支持你们的决定。 |
我的观点:千万不要再加 log type 了 下次把 DNS log 类型也可以删掉 |
赞同, |
不用结构不会冗余代码嘛,每次都要重复写日志内容嘛?
|
添加了一个 sniffLog 字段到 log 中,允许它在嗅探到域名后输出一段日志