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

modify log-format, then every plugin log is associated with access log #1454

Merged
merged 3 commits into from
Nov 4, 2024

Conversation

rinfx
Copy link
Collaborator

@rinfx rinfx commented Oct 30, 2024

修改log wrapper记录日志时的format,使得每条插件的日志能够与网关访问日志关联起来,便于排查问题

@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.52%. Comparing base (ef31e09) to head (83e52e0).
Report is 178 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1454      +/-   ##
==========================================
+ Coverage   35.91%   43.52%   +7.61%     
==========================================
  Files          69       76       +7     
  Lines       11576    12320     +744     
==========================================
+ Hits         4157     5362    +1205     
+ Misses       7104     6622     -482     
- Partials      315      336      +21     

see 69 files with indirect coverage changes

CH3CHO
CH3CHO previously approved these changes Oct 31, 2024
Copy link
Collaborator

@CH3CHO CH3CHO left a comment

Choose a reason for hiding this comment

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

LGTM

@CH3CHO CH3CHO dismissed their stale review October 31, 2024 08:43

need double check

@@ -36,7 +36,8 @@ type Log struct {
}

func (l Log) log(level LogLevel, msg string) {
msg = fmt.Sprintf("[%s] %s", l.pluginName, msg)
requestID, _ := proxywasm.GetProperty([]string{"x_request_id"})
msg = fmt.Sprintf("[%s] [%s] %s", l.pluginName, string(requestID), msg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

如果上下文里没有 request ID,日志就会输出一个空的 [],是不是不太好?

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

还有如果用户走下面 57 行的 logFormat 函数输出日志,还是不会输出 request ID 的。。。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

logFormat函数改了下,然后如果request_id获取失败的话,输出个nil?

Copy link
Collaborator

@CH3CHO CH3CHO left a comment

Choose a reason for hiding this comment

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

LGTM

@CH3CHO CH3CHO merged commit 7e8b044 into alibaba:main Nov 4, 2024
12 checks passed
@rinfx rinfx deleted the add-request-id-to-plugin-log branch November 5, 2024 01:46
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