Skip to content
This repository has been archived by the owner on Jun 14, 2023. It is now read-only.

Support use logrus to transmit trace context to log #19

Merged
merged 3 commits into from
May 19, 2021
Merged

Support use logrus to transmit trace context to log #19

merged 3 commits into from
May 19, 2021

Conversation

mrproliu
Copy link
Contributor

See SkyAPM/go2sky#104
Support the logrus log framework

@mrproliu mrproliu added enhancement New feature or request plugin labels May 14, 2021
@mrproliu mrproliu requested review from wu-sheng and arugal May 14, 2021 09:24
@mrproliu mrproliu added this to the 1.1.0 milestone May 14, 2021
@arugal
Copy link
Member

arugal commented May 15, 2021

Hi @mrproliu you also need to update the plugin summary.

wu-sheng
wu-sheng reviewed May 15, 2021
View reviewed changes
logrus/README.md Outdated Show resolved Hide resolved
@wu-sheng
Copy link
Member

Hi @mrproliu you also need to update the plugin summary.

We should separate that list to trace plugins and log plugins.

@mrproliu
Copy link
Contributor Author

Hi @mrproliu you also need to update the plugin summary.

We should separate that list to trace plugins and log plugins.

Have finished, Please take a look.

@wu-sheng
Copy link
Member

Update the PR title please.

@mrproliu mrproliu changed the title Support use logrus to transmit trace id to log Support use logrus to transmit trace context to log May 18, 2021
wu-sheng
wu-sheng reviewed May 19, 2021
View reviewed changes
logrus/example_logrus_test.go
Comment on lines +51 to +53
// Output:
// {"SW_CTX":"[,,N/A,N/A,-1]","level":"info","msg":"test1"}
// level=info msg=test2 SW_CTX="[,,N/A,N/A,-1]"
Copy link
Member

Choose a reason for hiding this comment

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

Let's add the output into the doc too.

Copy link
Member

Choose a reason for hiding this comment

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

But more like real data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's add the output into the doc too.

already add the context format at https://github.com/SkyAPM/go2sky-plugins/pull/19/files#diff-f583953a040e7174c73614f1e1e0d8e81695a399c931598d2e28a4b2bb5494cfR22.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But more like real data.

Golang will check output is the same with stdout, test failure if not the same. Should I need to add some real data to the comment?

@wu-sheng wu-sheng merged commit 91ed112 into SkyAPM:master May 19, 2021
@wu-sheng wu-sheng mentioned this pull request May 19, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wu-sheng wu-sheng wu-sheng approved these changes

@arugal arugal Awaiting requested review from arugal

Assignees
No one assigned
Labels
enhancement New feature or request plugin
Projects
None yet
Milestone
1.1.0
3 participants
@mrproliu @arugal @wu-sheng