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

[plugin]support log4j2 AsyncLogger #3715

Merged
merged 9 commits into from
Oct 28, 2019
Merged

[plugin]support log4j2 AsyncLogger #3715

merged 9 commits into from
Oct 28, 2019

Conversation

xiaoy00
Copy link
Contributor

@xiaoy00 xiaoy00 commented Oct 25, 2019

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • [1 ] Bug fix

  • New feature provided

  • Improve performance

  • Related issues


Bug fix

  • Bug description.
    there are 2 ways to implement Async log. but now this plugin just 1. so if i use Log4jContextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector
    to set async log . plugin doesn't work.

  • How to fix?
    intercept RingBufferLogEvent.setMessage(), i can get traceId,then its ok.


New feature or improvement

  • Describe the details and related test reports.

@xiaoy00
Copy link
Contributor Author

xiaoy00 commented Oct 25, 2019

/run agent-plugin-test-1

Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Could you update the document? And post the log when it actives?

@wu-sheng wu-sheng added agent Language agent related. feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. labels Oct 26, 2019
@xiaoy00 xiaoy00 changed the title [plugin]support log4j2 AsyncLogger [WIP]support log4j2 AsyncLogger Oct 28, 2019
@xiaoy00 xiaoy00 changed the title [WIP]support log4j2 AsyncLogger [plugin]support log4j2 AsyncLogger Oct 28, 2019
@wu-sheng
Copy link
Member

@zhaoyuguang Could you check this locally?

Copy link
Member

@zhaoyuguang zhaoyuguang left a comment

Choose a reason for hiding this comment

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

I have tested locally that the log print TID is normal. So this pr is LGTM.

@wu-sheng wu-sheng added this to the 6.5.0 milestone Oct 28, 2019
@wu-sheng wu-sheng merged commit a407819 into apache:master Oct 28, 2019
@enoch1024
Copy link

@wu-sheng @zhaoyuguang When I use Log4jContextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector
to set async log , plugin still doesn't work. I think it's similar to this #6815

@wu-sheng
Copy link
Member

@wu-sheng @zhaoyuguang When I use Log4jContextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector
to set async log , plugin still doesn't work. I think it's similar to this #6815

There is no chance async logger could work. Unless we hijack the log#debug by using the agent plugin mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent Language agent related. feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants