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

Remko Popma edited comment on LOG4J2-695 at 7/3/14 7:34 AM:
------------------------------------------------------------

Wow. Sorry, but I find this all a little depressing. :-O

I would at least try to reduce the number of parameters: could the appName not 
be removed and instead configured in the log4j configuration? (In the 
patternLayout, have "appName="App1" ..."). Surely this is going to be 
the same string, so not much point to let the application specify it again and 
again and again...
(In the config you could get this value from a system property also.)

For Result success/failure, how about providing two variations of all 
CustomLogger methods, like {{infoSuccess(...)}} and {{infoFailure(...)}}. That 
reduces the number of parameters that your (poor, poor) team needs to supply. 
Internally you then delegate to the {{info(...)}} method with the appropriate 
Result enum value. 

And perhaps some of the other parameters can be made optional? (Reason is only 
valid for failures, no?)

About the performance results, I took a look at the test code and it is worse 
than you think:
The log4j-1.x code actually executes the loop 5 million times, and is faster 
than the log4j-2 code which only executes the loop 500,000 times. Something is 
fishy there... I suspect there is a bug or two lurking in the performance test 
or CustomLogger implementations. - I already found one bug in the different 
loop iterations, I'm sure there are more...

To be honest, the performance test code looks messy and is trying to do too 
much in my opinion.
I recommend two things: first, remove that bit with the Executor and 10 worker  
threads from your performance test (or make it a separate test, whatever, just 
don't include it in your log performance comparison).
Second, try stepping through the performance test in a debugger while it is 
executing for at least a few iterations. I suspect that it may not be doing 
what you think it is doing... Is the log4j2 test throwing exceptions in every 
iteration? (That would be an explanation.) Is the log4j-1.x code faster because 
it is skipping work somehow? 
Third, why not have a single performance test class for testing both the 
log4j-1.x and the log4j2 based loggers. Then you can compare apples to apples...
Fourth, measure {{long startNanos = System.nanoTime();}} immediately before the 
loop, and measure {{long endNanos = System.nanoTime();}} immediately after the 
loop, and print that difference. It is not clear to me what the current 
performance test is trying to measure nor what the results actually mean.


was (Author: [email protected]):
Wow. Sorry, but I find this all a little depressing. :-O

I would at least try to reduce the number of parameters: could the appName not 
be removed and instead configured in the log4j configuration? (In the 
patternLayout, have "appName="App1" ..."). Surely this is going to be 
the same string, so not much point to let the application specify it again and 
again and again...
(In the config you could get this value from a system property also.)

For Result success/failure, how about providing two variations of all 
CustomLogger methods, like {{infoSuccess(...)}} and {{infoFailure(...)}}. That 
reduces the number of parameters that your (poor, poor) team needs to supply. 
Internally you then delegate to the {{info(...)}} method with the appropriate 
Result enum value. 

And perhaps some of the other parameters can be made optional? (Reason is only 
valid for failures, no?)

About the performance results, I took a look at the test code and it is worse 
than you think:
The log4j-1.x code actually executes the loop 5 million times, and is faster 
than the log4j-2 code which only executes the loop 500,000 times. Something is 
fishy there... I suspect there is a bug or two lurking in the performance test 
or CustomLogger implementations. - I already found one bug in the different 
loop iterations, I'm sure there are more...

To be honest, the performance test code looks messy and is trying to do too 
much in my opinion.
I recommend two things: first, remove that bit with the Executor and 10 worker  
threads from your performance test (or make it a separate test, whatever, just 
don't include it in your log performance comparison).
Second, try stepping through the performance test in a debugger while it is 
executing for at least a few iterations. I suspect that it may not be doing 
what you think it is doing... Is the log4j2 test throwing exceptions in every 
iteration? (That would be an explanation.) Is the log4j-1.x code faster because 
it is skipping work somehow? 
Third, why not have a single performance test class for testing both the 
log4j-1.x and the log4j2 based loggers. Then you can compare apples to apples...

> 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 V2 7 2 2014.zip, final code custom logger.zip, performance log4j 
> vs log4j2.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="[email protected]" 
> {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: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to