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
WELD-1948 Bean store leak - add DEBUG log message with the list of affected bean identifiers #1073
Conversation
mkouba
commented
Jul 14, 2015
- implemented only for HttpRequestContextImpl
- also updated bean identifier index log message
affected bean identifiers - implemented only for HttpRequestContextImpl - also updated bean identifier index log message
Triggering build using a merge of 8362296 on branch 2.3: |
Build 924 is now running using a merge of 8362296 on branch 2.3: |
Build 924 outcome was SUCCESS using a merge of 8362296 on branch 2.3: |
ContextLogger.LOG.beanStoreLeakDuringAssociation(this.getClass().getName(), request); | ||
ContextLogger.LOG.beanStoreLeakAffectedBeanIdentifiers(this.getClass().getName(), Iterables.toMultiRowString(beanStore)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only be building the multi-row string if leak happens AND debug logging is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, except for using BasicLogger.isDebugEnabled()
together with logging interface does not seem to be a good practice. On the other hand, we already have a similar piece of code in Weld. Let's fix it.
Build 924 outcome was SUCCESS using a merge of 8362296 on branch 2.3: |
Triggering build using a merge of 611cb72 on branch 2.3: |
Build 939 is now running using a merge of 611cb72 on branch 2.3: |
Build 939 outcome was SUCCESS using a merge of 611cb72 on branch 2.3: |
2 similar comments
Build 939 outcome was SUCCESS using a merge of 611cb72 on branch 2.3: |
Build 939 outcome was SUCCESS using a merge of 611cb72 on branch 2.3: |
Merged, thanks! |