feat: add logging support #389
Conversation
yuluo-yx
commented
Apr 21, 2024
•
yuluo-yx
commented
- For feat: add logger #388
- use golang 1.N.P version syntax
Signed-off-by: yuluo-yx <[email protected]>
Signed-off-by: yuluo-yx <[email protected]>
Signed-off-by: YuLuo <[email protected]>
Signed-off-by: yuluo-yx <[email protected]>
Signed-off-by: yuluo-yx <[email protected]>
Signed-off-by: yuluo-yx <[email protected]>
Signed-off-by: yuluo-yx <[email protected]>
|
ptal @LinuxSuRen , this feat is ready for review. |
There was a problem hiding this comment.
A few suggestion. Please take a look at it.
I guess some usages are incorrect. See also:
2024-04-22T11:13:34.014+0800 DPANIC server cmd/server.go:236 odd number of arguments passed as key-value pairs for logging {"ignored key": "[::]:7070"}
2024-04-22T11:13:34.014+0800 INFO server cmd/server.go:236 gRPC server listening at %v
2024-04-22T11:13:34.015+0800 DPANIC server cmd/server.go:288 odd number of arguments passed as key-value pairs for logging {"ignored key": "[::]:8080"}
2024-04-22T11:13:34.015+0800 INFO server cmd/server.go:288 HTTP server listening at %v
got it, I will fix it. |
Signed-off-by: yuluo-yx <[email protected]>
.github/workflows/build.yaml
Outdated
| @@ -13,7 +13,7 @@ jobs: | |||
| - name: Set up Go | |||
| uses: actions/setup-go@v4 | |||
| with: | |||
| go-version: 1.20.x | |||
| go-version: 1.22.2 | |||
There was a problem hiding this comment.
why specify this version instead of 1.22.x
There was a problem hiding this comment.
As of Go 1.21, toolchain versions must use the 1.N.P syntax.
go.mod does not match this syntax and there is no additional toolchain directive, which may cause some go commands to fail.
There was a problem hiding this comment.
https://github.com/actions/setup-go#getting-go-version-from-the-gomod-file
see here if we can just using 1.22, and it will choose the latest patch version of 1.22.x automatically.
There was a problem hiding this comment.
By the way, why we need to upgrade the Go version from 1.20 to 1.22?
There was a problem hiding this comment.
By the way, why we need to upgrade the Go version from
1.20to1.22?
advance with the times. 😆😆
Also as the language develops, some toolchains may report errors.
see envoyproxy/gateway#3215.
There was a problem hiding this comment.
https://github.com/actions/setup-go#getting-go-version-from-the-gomod-file
see here if we can just using
1.22, and it will choose the latest patch version of1.22.xautomatically.
got it, let me try.
There was a problem hiding this comment.
https://github.com/actions/setup-go#getting-go-version-from-the-gomod-file
see here if we can just using
1.22, and it will choose the latest patch version of1.22.xautomatically.
It may be ok in action but it fails when I set it up locally, can you try to set it up locally in your area? @Ink-33
There was a problem hiding this comment.
advance with the times.
No problem of course. It will be better if reviewers could see the specific reasons to upgrade Go version. For example:
- performance improve
- bugfix
- need some new APIs
- .etc.
Anyway, people could learn from those reasons. And this is a good to have suggestion.
There was a problem hiding this comment.
https://github.com/actions/setup-go#getting-go-version-from-the-gomod-file
see here if we can just using
1.22, and it will choose the latest patch version of1.22.xautomatically.It may be ok in action but it fails when I set it up locally, can you try to set it up locally in your area? @Ink-33
Do you modify go.mod? Try to keep using 1.n.p in go.mod file and use 1.22 for github action.
There was a problem hiding this comment.
Do you modify go.mod? Try to keep using 1.n.p in go.mod file and use 1.22 for github action.
got it, I will try.
Signed-off-by: yuluo-yx <[email protected]>
yuluo-yx
force-pushed
the
0421-yuluo/add-logging
branch
from
1837b57
to
4b8636b
Compare
|
I would like to know if there is something special in this log lib you added, because I found that lots of colon were missing in the logging message🤔 |
|
And I also notice that you seems like constructing logging message manually instead of logging formatted. Are there any advantages? |
Signed-off-by: yuluo-yx <[email protected]>
e.g. xxxLogger.Info("this is a msg", "age", age, "name", name)
{"level":"info","msg":"this is a msg","age":20,"name":"test"} |
Signed-off-by: Rick <[email protected]>
I do not understand? Can you give an example? |
I got it, thanks. |
|
This mistake is made by me. Sorry about it. I think this kind of log message is good for log system, but is not friendly for human reading. |
I have got that you are using structured logs so you can ingore this👍 |
Signed-off-by: yuluo-yx <[email protected]>
no problem, If has any problem, please call me. |
Signed-off-by: Rick <[email protected]>
|
|
Please ignore the report error.
|
* chore(deps): update redis docker tag to v6.2.13 * Update app version --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: github-action update-app-version <[email protected]>
