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

print access log according to the attachment settings #988

Merged
merged 3 commits into from
Aug 2, 2022

Conversation

rayzhang0603
Copy link
Collaborator

print access log according to the attachment settings

@@ -157,6 +158,9 @@ public class MotanConstants {
public static final String TRACE_CRECEIVE = "TRACE_CRECEIVE";// client端接收response
public static final String TRACE_CDECODE = "TRACE_CDECODE"; // client端解码response

// ------------------ attachment constants -----------------
public static final String ATT_PRINT_TRACE_LOG = "print_trace_log"; // 针对单请求是否打印(access)日志
Copy link
Collaborator

Choose a reason for hiding this comment

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

控制的是access log,使用trace字眼是否合适呢?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

trace log是一个纵向行为,trace log 标记可以控制整个链路中不同位置的log 行为,不只是针对access 日志,后续其他地方也可能会处理该标记,包括业务侧。

}

@Override
public Response filter(Caller<?> caller, Request request) {
if (accessLog == null) {
accessLog = caller.getUrl().getBooleanParameter(URLParamType.accessLog.getName(), URLParamType.accessLog.getBooleanValue());
}
if (accessLog || MotanSwitcherUtil.isOpen(ACCESS_LOG_SWITCHER_NAME)) {
if (accessLog || needLog(request)) {
Copy link
Collaborator

@snail007 snail007 Jul 26, 2022

Choose a reason for hiding this comment

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

这里判断有点别扭,应该只一个方法needLog去判断,是否需要打印逻辑封装在needLog里面。
如果按着目前判断思路,if (accessLog || 特例情况1 || 特例情况2 || 特例情况2||...),特例多起来不易于理解和维护。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这里的想法是把access自身配置显示独立出来,其他都属于非配置情况下打印accesslog的场景,后续其他判断条件都会在needlog方法中进行判断。这里可以对needlog方法增加注释来进行补充说明

Copy link
Collaborator

@sunnights sunnights left a comment

Choose a reason for hiding this comment

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

LGTM

@rayzhang0603 rayzhang0603 merged commit 4f1f473 into master Aug 2, 2022
@rayzhang0603 rayzhang0603 deleted the feat/print_trace_log branch September 5, 2022 02:27
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.

3 participants