You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Patrick Rhomberg <pr...@pivotal.io> on 2017/03/22 21:59:10 UTC
Review Request 57861: GEODE-1274: Migration from PulseLogWriter to
Log4j standard and removal of associated classes.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57861/
-----------------------------------------------------------
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/57861/diff/1/
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