logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Remko Popma (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LOG4J2-695) Custom Logger with restrictions on existing methods
Date Wed, 02 Jul 2014 10:27:24 GMT

    [ https://issues.apache.org/jira/browse/LOG4J2-695?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14049807#comment-14049807
] 

Remko Popma commented on LOG4J2-695:
------------------------------------

Hmm... your request goes a bit beyond the scope of fixing bugs in Log4j. I'm basically giving
you free consulting... All right then, constructive criticism. You better sit down first though,
you may not like what I have to say...

First, please tell me that your development teams will be allowed to use the "normal" Log4j2
API in addition to the CustomLogger, and the CustomLogger is intended for optional use to
send events to splunk. If your teams are forced to use CustomLogger for _all_ their logging
needs, you may want to get good life insurance because your teams will really, really not
like it. They will ask questions like this:
* Why is there no trace or debug capability any more? Why provide a logger that can only log
info/warn/error/fatal? In my applications I only use INFO at startup, and WARN/ERROR if there
is a problem. By then it is too late and your log file will not contain any detail on what
lead to this error. This will make it very difficult for your teams to solve issues, making
the logging almost useless...
* One of the most important functions of logging is error handling: a stack trace in the log
file is a huge, huge help in finding and fixing bugs. CustomLogger has stripped out the methods
that take a {{Throwable}} argument. If your teams cannot use the normal API to log stack traces
they will not thank you for this.
* Similarly, the log4j API has methods for parameterized logging: e.g. {{logger.info("Calculation
result is {}, based on input1={}, input2={} and input3={}", result, input1, input2, input3);}}
That feature has been removed from CustomLogger. Again, I think this is a step back, not an
improvement.

I think the kind of log events you want to send to splunk are more rare than the normal logging
that developers need. I would keep these separate: CustomLogger for splunk, normal Log4j for
normal logging. 

If my fears are correct and your teams will only be allowed to use CustomLogger, I would strongly
suggest that you at least:
* revive the trace and debug levels - developers need to do more logging that what you want
to put into splunk
* revive the ability to log stack traces
* revive the ability to have parameterized logging
* you can discourage (but why? they are simply different use cases) non-splunk logging by
marking the methods with the @deprecated annotation. Putting a customlogger_warning="deprecated
method not recommended for prod" message in the log is not very useful in my opinion.

Now, implementation style:
* The class declared as lowercase {{customLogger}}. The java convention is for class names
to start with an uppercase letter. The file is uppercase {{CustomLogger.java}}, however. (Does
that compile?)
* Some private methods start with uppercase. The Java convention is for methods to start with
lowercase. This compiles but looks sloppy.
* Some variable names use underscore to separate words, e.g. {{app_name}}, {{event_name}}.
The Java convention is to use camelCase, so that should be {{appName}}, {{eventName}}.
* {{static final int stringBuilderInitialCapacity}} is using camelCase. The Java convention
for constants is to use all uppercase, so that would be something like {{INITIAL_CAPACITY}}.
* The {{validate...}} methods could be rewritten as one-liners: {{return action != null &&
action.length() >= 5;}}
* Result.success and Result.failure -> Result.SUCCESS, Result.FAILURE (constants)

Performance:
* The current logic creates the formattedString regardless of whether the level is enabled.
I would recommend first checking {{if (logger.isEnabled(level, ...) }} before creating the
formattedString object.
* Your biggest bottleneck will be the call to {{UUID.randomUUID()}} for every log event. A
quick test in a profiler should prove this. Note that this is done in the application thread,
so async loggers will not help here. Do you really need this? If you configure a PatternLayout
with %sequenceNumber every log message will get a unique number in your log file with almost
no overhead.

> Custom Logger with restrictions on existing methods
> ---------------------------------------------------
>
>                 Key: LOG4J2-695
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-695
>             Project: Log4j 2
>          Issue Type: Bug
>          Components: API
>            Reporter: SIBISH BASHEER
>              Labels: customlogger
>         Attachments: AppAsyncMain.java, CustomLogger.java, CustomLogger.java, final code
custom logger.zip
>
>
> I have been looking at the Custom/Extended logger discussions. But none of them seems
to fulfil what i am looking for.
> 1) I want custom methods as below:
> {code}
>     private static CustomLogger logger = CustomLogger.getLogger(AppAsyncMain.class);
>    
>     logger.info( transaction_id, app_name + event_name +
> 					"inside the loop" + "inside the loop of the sample app" +
> 					"success" + "looped in" + "loop_count" +
> 					String.valueOf(i));
> {code}
> 					
> 	log:
> {code}
> 2014-06-30 16:09:28,268 log_level="INFO" thread_name="main" class_name="com.custom.samplelog4j.AppAsyncMain"
transaction_id="79ea1071-9565-405a-aa18-75d271694bf2" event_id="dd5c69c0-4400-41fd-8a2e-5d538d8e8c9b"
app="Sample Logging SDK App" event_name="Sample Event" action="start of sample app" desc="start
of api" result="success" reason="start" token="abcdefg" alias="abc@gmail.com" 
> {code}
> 	
> 2) I want to show warning in existing logger methods so the teams using the custom logger
doesn't use these methods other than for testing:
> {code}
>    logger.info("start of statement");
> {code}
>    
>    log:
> {code}
>    2014-06-30 16:12:31,065 log_level="INFO" thread_name="main" class_name="com.custom.samplelog4j2.AppAsyncMain"
start of statement  customlogger_warning="method not recommended for production use" 
> {code}
>    
> 3) Custom validations for the fields:
> {code}
>     	private static String validateFields(String app_name, String event_name,
> 			String action, String desc, Result result, String reason) {
> 		String validateStatus = "";
> 		if (!ValidateAppName(app_name)) {
> 			validateStatus = "app_name";
> 		} else if (!ValidateEventName(event_name)) {
> 			validateStatus = "event_name";
> 		} else if (!ValidateAction(action)) {
> 			validateStatus = "action";
> 		} else if (!ValidateDesc(desc)) {
> 			validateStatus = "desc";
> 		} else if (!ValidateReason(result, reason)) {
> 			validateStatus = "reason";
> 		}
> 		return validateStatus;
> 	}
> {code}
> Options tried:
> 1.
> * extended ExtendedLoggerWrapper
> * created the map of the Custom logger
> * This option was failing because of "writing to a closed appender"
> * Attached is the code "CustomLogger.java"
>    
> 2. Modified the AbstractLogger in Trunk and added the below methods:
> {code}
>       @Override
>     public void info(final String message) {
>     String updtMessage = message + " amexlogger_error=\"Incorrect method used\"";
>         logIfEnabled(FQCN, Level.INFO, null, updtMessage, (Throwable) null);
>     }
>  public void info(final String transactionId, final String app_name, final String event_name,
final String action, final String desc, final String result, final String reason, final String...
moreFields) { 
>        String message = "transaction_id=" + transactionId + " " + "app_name=" + app_name
+ " " + "event_name=" + event_name + " " + "action=" + action;
>  
>         logIfEnabled(FQCN, Level.INFO, null, message, (Throwable) null);
>     }
> {code}
> 	I don't want to modify the methods inside the log4j-api. 
> 	
> Please help me with the correct approach on how to use log4j2 for this usecase.
> Thanks
> Sibish



--
This message was sent by Atlassian JIRA
(v6.2#6252)

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message