Item 7490: Upgrade to Log4J 2 - Part 1 #1378
Conversation
|
The builds and teamcity tests will fail as the build.gradle which is in SVN will not be checked in until the work is complete. |
|
In the past, I've added conditionals to the (svn-based) build.gradle file to keep it working with both develop and my branch. And then I remove the conditionals after I finally merge to develop. In my case, I was conditionalizing on the presence of the assay module, but you could conditionalize on something else: https://svn.mgt.labkey.host/stedi/trunk/server/build.gradle/?p=64176 |
There is more to that, the gradle plugin needs to read the changed log4j.xml (which is also in SVN). |
We can make a short-term change to the plugin to pick up a different log4j.xml based on, say, the presence of some property if that makes things easier, but since you're having to change all the import paths for log4j, I'm not seeing a clear way to have both log4j options available based on some property or other without a lot of extra work. |
labkey-jeckels
left a comment
labkey-jeckels
left a comment
There was a problem hiding this comment.
I guess I'll hold the final approval until it's ready to merge?
| { | ||
| Runnable testThread = this::randomSentence; | ||
|
|
||
| for (int i=0; i < 4000; i++) |
There was a problem hiding this comment.
Do we get value from having this many threads? 4000 is quite a stress test. Just calling randomSentence() directly in the for loop seems like it would be fine
| @@ -31,20 +37,28 @@ | |||
| * Date: 2012-02-28 | |||
| * Time: 4:10 PM | |||
| */ | |||
| public class MXBeanAppender extends org.apache.log4j.AppenderSkeleton implements ErrorsMXBean | |||
| @Plugin(name = "SessionAppender", category = "Core", elementType = "appender", printObject = true) | |||
There was a problem hiding this comment.
Looks like name was copy/pasted, should probably be MXBeanAppender?
| { | ||
| if (eventId==0 || eventId<Integer.parseInt(e.getProperty("eventId"))) | ||
| // TODO - find replacement - maybe a ThreadLocal using a WeakHashMap of Event->EventId? |
There was a problem hiding this comment.
Is this on the list for the next story?
…upgrade # Conflicts: # api/src/org/labkey/api/util/SafeFileAppender.java
| public void write(String message, @Nullable Throwable t, String level) | ||
| { | ||
| String formattedDate = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSXXX").format(new Date()); | ||
| String formattedDate = new SimpleDateFormat("dd MMM yyyy HH:mm:ss,SSS").format(new Date()); |
There was a problem hiding this comment.
Don't create a new date format for every invocation - use DateUtil.formatDateTime() which caches and uses a thread-safe implementation
Rationale
This PR is the starting point towards the effort to upgrade our logging to log4j2. After the code review of this branch, we will not merge this branch in develop. For the next part of the work, a new branch will be created from this branch and pull request from the new branch to this branch will be created and code review for that story and merge it back. This will keep on happening until we are done and then this original branch will be merged to develop. Let me know if someone has suggestion for an easier approach.
Related Pull Requests
Changes
Shorter version:
org.apache.log4j.Logger.getLogger()-->org.apache.logging.log4j.LogManager.getLogger()org.apache.log4j-->org.apache.logging.log4j