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] [Comment Edited] (LOG4J2-695) Custom Logger with restrictions on existing methods
Date Thu, 03 Jul 2014 07:35:24 GMT

    [ 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=&quot;App1&quot;
..."). 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: remkop@yahoo.com):
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=&quot;App1&quot;
..."). 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="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