Skip to content

Conversation

@5idu
Copy link
Contributor

@5idu 5idu commented May 23, 2023

Describe what this PR does / why we need it

There is no trace id field in the xecho access log, but we do need it.

Describe how you did it

Just replace the default logger in the middleware

Describe how to verify it

The trace id field is included in the output access log

@5idu 5idu temporarily deployed to tablestore-live May 23, 2023 07:00 — with GitHub Actions Inactive
@5idu 5idu changed the title update xecho access log with trace id feat: update xecho access log with trace id May 23, 2023
var fields = make([]xlog.Field, 0, 8)

defer func() {
logger := xlog.FromContext(ctx.Request().Context())
Copy link
Member

Choose a reason for hiding this comment

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

这里FromContext取的是default log,这个主要是用于业务日志,建议用Jupiter()日志,写入系统日志

@5idu 5idu temporarily deployed to tablestore-live May 29, 2023 01:11 — with GitHub Actions Inactive
@codecov-commenter
Copy link

codecov-commenter commented May 29, 2023

Codecov Report

Merging #889 (4661bae) into master (9e2e9bb) will decrease coverage by 0.30%.
Report is 46 commits behind head on master.
The diff coverage is 48.71%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #889      +/-   ##
==========================================
- Coverage   53.05%   52.75%   -0.30%     
==========================================
  Files         202      202              
  Lines       11336    11438     +102     
==========================================
+ Hits         6014     6034      +20     
- Misses       4866     4944      +78     
- Partials      456      460       +4     
Files Changed Coverage Δ
pkg/xlog/context.go 31.70% <37.93%> (-13.75%) ⬇️
pkg/server/xgrpc/interceptor.go 38.01% <50.00%> (-6.01%) ⬇️
pkg/xlog/api.go 32.69% <66.66%> (-0.65%) ⬇️
pkg/client/rocketmq/push_consumer.go 48.06% <100.00%> (ø)
pkg/server/xecho/config.go 56.36% <100.00%> (ø)
pkg/server/xecho/middleware.go 68.13% <100.00%> (-3.12%) ⬇️

... and 9 files with indirect coverage changes

@stale
Copy link

stale bot commented Jul 30, 2023

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time. Thank you for your contributions.

@stale stale bot added the stale label Jul 30, 2023
@5idu
Copy link
Contributor Author

5idu commented Jul 31, 2023

@hnlq715 麻烦看下,这个 PR 已经好久了

@stale
Copy link

stale bot commented Jul 31, 2023

This issue is no longer marked as stale.

@stale stale bot removed the stale label Jul 31, 2023
span.SetAttributes(semconv.HTTPServerAttributesFromHTTPRequest(pkg.Name(), c.Request().URL.Path, c.Request())...)

ctx = xlog.NewContext(ctx, xlog.Default(), span.SpanContext().TraceID().String())
ctx = xlog.NewContext(ctx, xlog.Jupiter(), span.SpanContext().TraceID().String())
Copy link
Member

Choose a reason for hiding this comment

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

可以考虑把jupiter和default日志都注入到ctx,然后框架用xlog.J(ctx),业务项目用xlog.L(ctx)来获取对应日志实例

@5idu

@5idu 5idu temporarily deployed to tablestore-live August 1, 2023 02:11 — with GitHub Actions Inactive

func NewContext(ctx context.Context, l *Logger, traceID string) context.Context {
return context.WithValue(ctx, loggerKey{}, l.With(String(traceIDField, traceID)))
func NewContextWithDefaultLogger(ctx context.Context, l *Logger, traceID string) context.Context {
Copy link
Member

Choose a reason for hiding this comment

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

这里尽量保证兼容,不改API

}

func FromContext(ctx context.Context) *Logger {
func NewContextWithJupiterLogger(ctx context.Context, l *Logger, traceID string) context.Context {
Copy link
Member

Choose a reason for hiding this comment

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

这里新增的这个api可以不export出来,用newContextWithJupiterLogger

return context.WithValue(ctx, jupiterLoggerKey{}, l.With(String(traceIDField, traceID)))
}

func GetDefaultLoggerFromContext(ctx context.Context) *Logger {
Copy link
Member

Choose a reason for hiding this comment

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

同上

return l
}

func GetJupiterLoggerFromContext(ctx context.Context) *Logger {
Copy link
Member

Choose a reason for hiding this comment

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

同上

@5idu 5idu temporarily deployed to tablestore-live August 2, 2023 02:15 — with GitHub Actions Inactive
}

func FromContext(ctx context.Context) *Logger {
func newContextWithJupiterLogger(ctx context.Context, l *Logger, traceID string) context.Context {
Copy link
Member

Choose a reason for hiding this comment

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

这个FromContext还是可以继续保留API,可以加个Deprecated注释,推荐使用xlog.L

@5idu 5idu temporarily deployed to tablestore-live August 2, 2023 03:26 — with GitHub Actions Inactive
@sysulq sysulq merged commit d2545ff into douyu:master Aug 15, 2023
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