-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#28 SLF4J logging #89
Changes from 4 commits
8716371
301b7bd
54ae505
1f4bb7c
ed23f4d
89d3e04
4621b9d
6b695d1
a178ed5
1442dd8
ad64b11
0f2cb76
9bbe514
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
/** | ||
* The MIT License (MIT) | ||
* | ||
* Copyright (c) 2015 Yegor Bugayenko | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included | ||
* in all copies or substantial portions of the Software. | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
|
||
package org.takes.facets.slf4j; | ||
|
||
import java.io.IOException; | ||
import java.net.Socket; | ||
import lombok.EqualsAndHashCode; | ||
import org.takes.http.Back; | ||
|
||
/** | ||
* Logs Back.accept() calls. | ||
* | ||
* <p>The class is immutable and thread-safe. | ||
* @author Dmitry Zaytsev (dmitry.zaytsev@gmail.com) | ||
* @version $Id$ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's add |
||
*/ | ||
@EqualsAndHashCode(of = "origin", callSuper = false) | ||
public final class BkLogged extends LogWrap implements Back { | ||
/** | ||
* Original back. | ||
*/ | ||
private final transient Back origin; | ||
|
||
/** | ||
* Ctor. | ||
* @param back Original | ||
*/ | ||
public BkLogged(final Back back) { | ||
this(back, LogWrap.Level.TRACE); | ||
} | ||
|
||
/** | ||
* Ctor. | ||
* @param back Original | ||
* @param lvl Log level | ||
*/ | ||
public BkLogged(final Back back, final LogWrap.Level lvl) { | ||
super(back.getClass(), lvl); | ||
this.origin = back; | ||
} | ||
|
||
@Override | ||
public void accept(final Socket socket) throws IOException { | ||
final long started = System.currentTimeMillis(); | ||
this.origin.accept(socket); | ||
this.log( | ||
"[%s] #accept(%s) in [%d] ms", | ||
this.origin, | ||
socket, | ||
System.currentTimeMillis() - started | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
/** | ||
* The MIT License (MIT) | ||
* | ||
* Copyright (c) 2015 Yegor Bugayenko | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included | ||
* in all copies or substantial portions of the Software. | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
|
||
package org.takes.facets.slf4j; | ||
|
||
import java.io.IOException; | ||
import lombok.EqualsAndHashCode; | ||
import org.takes.http.Exit; | ||
import org.takes.http.Front; | ||
|
||
/** | ||
* Logs Front.start() calls. | ||
* | ||
* <p>The class is immutable and thread-safe. | ||
* @author Dmitry Zaytsev (dmitry.zaytsev@gmail.com) | ||
* @version $Id$ | ||
*/ | ||
@EqualsAndHashCode(of = "origin", callSuper = false) | ||
public final class FtLogged extends LogWrap implements Front { | ||
/** | ||
* Original front. | ||
*/ | ||
private final transient Front origin; | ||
|
||
/** | ||
* Ctor. | ||
* @param front Original | ||
*/ | ||
public FtLogged(final Front front) { | ||
this(front, LogWrap.Level.TRACE); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you sure we need TRACE level by default? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @longtimeago I think logs every method's call with parameters and return values will be too big. It could be used for debug only. Level.DEBUIG will be better ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would store default level inside the |
||
} | ||
|
||
/** | ||
* Ctor. | ||
* @param front Original | ||
* @param lvl Log level | ||
*/ | ||
public FtLogged(final Front front, final LogWrap.Level lvl) { | ||
super(front.getClass(), lvl); | ||
this.origin = front; | ||
} | ||
|
||
@Override | ||
public void start(final Exit exit) throws IOException { | ||
this.log( | ||
"[%s] #start(%s)", | ||
this.origin, | ||
exit | ||
); | ||
this.origin.start(exit); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
/** | ||
* The MIT License (MIT) | ||
* | ||
* Copyright (c) 2015 Yegor Bugayenko | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included | ||
* in all copies or substantial portions of the Software. | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
|
||
package org.takes.facets.slf4j; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
/** | ||
* Wrap slf4j functionality. | ||
* | ||
* <p>The class is immutable and thread-safe. | ||
* @author Dmitry Zaytsev (dmitry.zaytsev@gmail.com) | ||
* @version $Id$ | ||
* @since 0.11 | ||
*/ | ||
@SuppressWarnings({ "PMD.CyclomaticComplexity", "PMD.LoggerIsNotStaticFinal" }) | ||
public class LogWrap { | ||
/** | ||
* Log levels. | ||
*/ | ||
public enum Level { | ||
/** | ||
* Trace level. | ||
*/ | ||
TRACE, | ||
/** | ||
* Debug level. | ||
*/ | ||
DEBUG, | ||
/** | ||
* Info level. | ||
*/ | ||
INFO, | ||
/** | ||
* Warn level. | ||
*/ | ||
WARN, | ||
/** | ||
* Severe level. | ||
*/ | ||
SEVERE | ||
}; | ||
|
||
/** | ||
* Log level. | ||
*/ | ||
private final transient LogWrap.Level level; | ||
|
||
/** | ||
* Logger. | ||
*/ | ||
private final transient Logger logger; | ||
|
||
/** | ||
* Ctor. | ||
* @param clazz Logger class | ||
* @param lvl Log level | ||
*/ | ||
public LogWrap(final Class<?> clazz, final LogWrap.Level lvl) { | ||
this.level = lvl; | ||
this.logger = LoggerFactory.getLogger(clazz); | ||
} | ||
|
||
/** | ||
* Log message. | ||
* @param format Format string | ||
* @param param Parameters | ||
* @checkstyle CyclomaticComplexityCheck (2 line) | ||
*/ | ||
public final void log(final String format, final Object... param) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. first of all, I'm not sure we need to check if some level is enabled for all levels. In general, check is meaningful for DEBUG and TRACE as almost all application have default log level INFO. Take a look at how it could look like: public final void log(final String format, final Object... param) {
switch (this.level) {
case TRACE :
if (this.logger.isTraceEnabled()) {
this.logger.trace(String.format(format, param));
}
break;
case DEBUG:
if (this.logger.isDebugEnabled()) {
this.logger.debug(String.format(format, param));
}
break;
case INFO:
this.logger.info(String.format(format, param));
break;
case WARN:
this.logger.warn(String.format(format, param));
break;
case SEVERE:
this.logger.error(String.format(format, param));
break;
default:
throw new IllegalArgumentException("Unknown log level");
}
} what do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to check for all levels. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well ... there is more clever solution - delegate formatting to the underlying logger: switch (this.level) {
case TRACE :
this.logger.trace(format, param);
break;
case DEBUG:
this.logger.debug(format, param);
break;
case INFO:
this.logger.info(format, param);
break;
case WARN:
this.logger.warn(format, param);
break;
case SEVERE:
this.logger.error(format, param);
break;
default:
throw new IllegalArgumentException("Unknown log level");
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also propose to use ERROR instead of SEVERE to be consistent with SLF4J |
||
if (this.level == LogWrap.Level.TRACE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider using switch by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @longtimeago I think switch with if in each block will be less readable. isn't it? |
||
&& this.logger.isTraceEnabled()) { | ||
this.logger.trace(String.format(format, param)); | ||
} else if ( | ||
this.level == LogWrap.Level.DEBUG | ||
&& this.logger.isDebugEnabled()) { | ||
this.logger.debug(String.format(format, param)); | ||
} else if ( | ||
this.level == LogWrap.Level.INFO | ||
&& this.logger.isInfoEnabled()) { | ||
this.logger.trace(String.format(format, param)); | ||
} else if ( | ||
this.level == LogWrap.Level.WARN | ||
&& this.logger.isWarnEnabled()) { | ||
this.logger.warn(String.format(format, param)); | ||
} else if ( | ||
this.level == LogWrap.Level.SEVERE | ||
&& this.logger.isErrorEnabled()) { | ||
this.logger.error(String.format(format, param)); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
/** | ||
* The MIT License (MIT) | ||
* | ||
* Copyright (c) 2015 Yegor Bugayenko | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included | ||
* in all copies or substantial portions of the Software. | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
|
||
package org.takes.facets.slf4j; | ||
|
||
import java.io.IOException; | ||
import java.util.ArrayList; | ||
import java.util.Collection; | ||
import java.util.Iterator; | ||
import lombok.EqualsAndHashCode; | ||
import org.takes.Request; | ||
import org.takes.Response; | ||
import org.takes.facets.auth.Identity; | ||
import org.takes.facets.auth.Pass; | ||
|
||
/** | ||
* Logs Pass methods calls.. | ||
* | ||
* <p>The class is immutable and thread-safe. | ||
* @author Dmitry Zaytsev (dmitry.zaytsev@gmail.com) | ||
* @version $Id$ | ||
*/ | ||
@EqualsAndHashCode(of = "origin", callSuper = false) | ||
public final class PsLogged extends LogWrap implements Pass { | ||
/** | ||
* Original pass. | ||
*/ | ||
private final transient Pass origin; | ||
|
||
/** | ||
* Ctor. | ||
* @param pass Original | ||
*/ | ||
public PsLogged(final Pass pass) { | ||
this(pass, LogWrap.Level.TRACE); | ||
} | ||
|
||
/** | ||
* Ctor. | ||
* @param pass Original | ||
* @param lvl Log level | ||
*/ | ||
public PsLogged(final Pass pass, final LogWrap.Level lvl) { | ||
super(pass.getClass(), lvl); | ||
this.origin = pass; | ||
} | ||
|
||
@Override | ||
public Iterator<Identity> enter(final Request request) throws IOException { | ||
final long started = System.currentTimeMillis(); | ||
final Iterator<Identity> iterator = this.origin.enter(request); | ||
final Collection<Identity> resp = new ArrayList<Identity>(1); | ||
final StringBuilder sbr = new StringBuilder("["); | ||
if (iterator.hasNext()) { | ||
final Identity next = iterator.next(); | ||
sbr.append(next); | ||
sbr.append(','); | ||
resp.add(next); | ||
} | ||
sbr.append(']'); | ||
this.log( | ||
"[%s] #enter(%s) return [%s] in [%d] ms", | ||
this.origin, | ||
request, | ||
sbr.toString(), | ||
System.currentTimeMillis() - started | ||
); | ||
return resp.iterator(); | ||
} | ||
|
||
@Override | ||
public Response exit(final Response response, final Identity identity) | ||
throws IOException { | ||
final long started = System.currentTimeMillis(); | ||
final Response resp = this.origin.exit(response, identity); | ||
this.log( | ||
"[%s] #exit(%s,%s) return [%s] in [%d] ms", | ||
this.origin, | ||
response, | ||
identity, | ||
resp, | ||
System.currentTimeMillis() - started | ||
); | ||
return resp; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add
<optional>true</optional>
here, in case a user doesn't want to use SLF4J logging, this dependency won't be in his project