[GitHub] [accumulo] dlmarion commented on a diff in pull request #3745: Add Message Prefix for File deletion actions in GCRun

["dlmarion (via GitHub)" <gi...@apache.org>]

dlmarion commented on code in PR #3745:
URL: https://github.com/apache/accumulo/pull/3745#discussion_r1327187307


##########
server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java:
##########
@@ -97,7 +98,8 @@ public class GCRun implements GarbageCollectionEnvironment {
   private long errors = 0;
 
   public GCRun(Ample.DataLevel level, ServerContext context) {
-    this.log = LoggerFactory.getLogger(level.name() + GCRun.class);
+    this.log = LoggerFactory.getLogger(GCRun.class.getName() + level.name());

Review Comment:
   So, I believe the correct way to achieve this is using the log4j MDC/NDC feature. In log4j2 these features have been merged into one (see https://logging.apache.org/log4j/2.x/manual/thread-context.html). When using this feature your logger name stays the same and you add context to the log message via the ThreadContext. So, in this case you would add the level to the ThreadContext. in the log4j2 properties file, we would need to use `%x` in the pattern so that the ThreadContext is included in the log message in the file.



##########
server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java:
##########
@@ -87,7 +87,9 @@
  * A single garbage collection performed on a table (Root, MD) or all User tables.
  */
 public class GCRun implements GarbageCollectionEnvironment {
+  // loggers are not static to support unique naming by level
   private final Logger log;
+  private static String fileActionPrefix = "FILE-ACTION:";

Review Comment:
   I'm not sure why we need this variable. You could just add "FILE-ACTION" at the beginning of each log message. The only reason to make it a variable is in the case that you think it would change in a future version.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org