> On March 23, 2017, 5:39 p.m., Jared Stewart wrote: > > Is there a log4j configuration being applied somewhere? I'm wondering > > where I should expect the new Pulse logs to show up.
I'm not entirely clear on how log4j workst. That said... All the pulse loggers are now being instantiated directly from log4j's LogManager. LogService (in geode-core) extends LogManager and adds the configurations. It appears that those configurations are then loaded into the LogManager class. So in theory, those should propogate to the pulse log4j things. I did some cursory testing on gfsh directly, and it appears that logs are showing up in the locator's logs. (Does `start pulse` target the locator? Are the locator and manager separate things? They're both being listed on the same member within pulse, so I don't know if the appenders just happen to be sharing that log file.) [update:] Looking at it again, I'm not 100% sure those aren't the REST API calls that were already showing up in the logs. But there is a marked difference in logging present between the current `develop` branch and this patch, so I was led to believe that we are in fact capturing the pulse log calls. Could this be a potential problem in a standalone deployment of pulse? ====== After some testing, it looks like, when run embedded, that the LogService's managing of Log4J settings percolates through. Formatting and location should be updated by the log4j configurations. - Patrick ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57822/#review169899 ----------------------------------------------------------- On March 22, 2017, 10:33 p.m., Patrick Rhomberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57822/ > ----------------------------------------------------------- > > (Updated March 22, 2017, 10:33 p.m.) > > > Review request for geode, Jinmei Liao, Jared Stewart, Kevin Duling, Ken Howe, > Kirk Lund, and Swapnil Bawaskar. > > > Repository: geode > > > Description > ------- > > To avoid dependency on geode-core, the pulse loggers are instantiated > directly from LogManager, rather than canonical LogService (which itself > extends LogManager). > > Significant reduction of logging level state checks, relying on Log4j > handling. > Significant reduction of string concatenation, relying on Log4j2 string > substitutions. > Reduction of logging using an exception e.getMessage, favoring instead > passing the exception itself for the stacktrace. > Multiple identical exception blocks collapsed. > > ================ > > Explicitly not addressed in this ticket: > - A great deal of logging could do with localization. > - A number of exception blocks catch one or more types of exception and only > log the exception before moving. These are specific exceptions and not the > general `Exception` class. The catches could be simplified, but could that > lead to a catch an error that would otherwise be raised? > > The original ticket was not about updating from a depreciated class, but > about how logs were not being collected. The answer to this was that Log4j > wasn't tracking them, so they weren't being gathered; extending Log4j to do > Pulse tracking should resolve the issue, but is it okay that this is only > tangentally answering the problem? > > > Diffs > ----- > > geode-pulse/build.gradle a038f734266939f3c12b713ce0745c013b4adb91 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java > e1666ecf38331983d8c47c4b46b3b7e87faaf854 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/controllers/ExceptionHandlingAdvice.java > 80a3c63b5d9170cf9933c43edf56da78dc62bf46 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/controllers/PulseController.java > 12b6172cbbc79651e972179532f2b79623a1992e > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Cluster.java > e71388d134c96549ee9995c4c874615ee66fe7c1 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/DataBrowser.java > 03077c27003811718e3974575a15b6892f6d8e2f > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java > 9633b9a1f50df051b8fb9b4f4787a1d25a0ab019 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JmxManagerFinder.java > fa2b5b79f40a95b0a20be38c20e61d9e94a39688 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/PulseConfig.java > dc643b49462af9365859d899f2c798f0f2448b72 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/PulseConstants.java > b36c283630fcd3d143c1398ac0fecf45eec0eb54 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Repository.java > b228e4a754100fe07c9dbec232d5e88809aefeef > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/log/LogWriter.java > 6f90e7a48a10ab2778ee00cf3d707a476ebe91eb > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/log/MessageFormatter.java > 924520d3ba70a48379bd6ff9a6ce4d6e0d4ed804 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/log/PulseLogWriter.java > 241b34bfe5bc56e062c43944ec7aaf1cfaa657c7 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/log/PulseLogger.java > d22f188248d2d8fb685074523e22eac7c26f5e20 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java > 884e51fa71b79de9e5ab9e7f0f933e37b6031438 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java > 4d300f04ff82f701509d44b83dd46698dbc6035e > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/LogoutHandler.java > e18e35d596552a40715ee772e559ffc2d077af5e > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java > a088ccb4176db19f9ec0ec3570ad23d286609f7b > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java > d5a913793cd22c408398f3e76767ea7379c14042 > > geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java > df315728dbcc07444249f68829b9bed349736ac9 > > > Diff: https://reviews.apache.org/r/57822/diff/2/ > > > Testing > ------- > > precheckin running. Test/LegacyDUnitTest have already come back green. > > Testing not done: actually verifying anything involving Pulse logging. I > still need to learn how pulse is used. > > > Thanks, > > Patrick Rhomberg > >
