Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added exceptionLevel to allow different log level for exceptions #364

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+54 −0
Open

Added exceptionLevel to allow different log level for exceptions #364

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b9162e
Added exceptionLevel to allow different log level for exceptions
fnmps Sep 18, 2023
ac31c72
Replaced ternary operation
fnmps Sep 19, 2023
e1f5472
Solved checkstyle issues
fnmps Sep 19, 2023
eb56f33
Fixed qulice errors
fnmps Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/main/java/com/jcabi/aspects/Loggable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@
*/
int value() default Loggable.INFO;

/**
* Log level for exceptions. If not defined then the log level of value is used.
* @return The log level
*/
int exceptionLevel() default -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be a constant here, probably Loggable.WARNING

Copy link
Author

@fnmps fnmps Sep 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreoss Creating a constant on this class would cause it to be visible to developers using the Loggable annotation, which could cause confusion. At most I could create a class for the constant, so it would be less confusing to developers using the library.
Should I proceed with that implementation or is it overkill?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fnmps I don't quite get why we need a class. -1 means default, so you can add Loggable.DEFAULT. onException probably would be a better name than exceptionLevel


/**
* Maximum amount allowed for this method (a warning will be
* issued if it takes longer).
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/jcabi/aspects/aj/MethodLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ private Object wrap(final ProceedingJoinPoint point, final Method method,
} else {
origin = "somewhere";
}
level = annotation.exceptionLevel() == -1 ? level : annotation.exceptionLevel();
fnmps marked this conversation as resolved.
Show resolved Hide resolved
if (LogHelper.enabled(level, method.getDeclaringClass())) {
LogHelper.log(
level,
Expand Down
35 changes: 34 additions & 1 deletion src/test/java/com/jcabi/aspects/LoggableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ final class LoggableTest {
* Foo toString result.
*/
private static final transient String RESULT = "some text";
private static final transient String DEBUG_LOG = "DEBUG";
private static final transient String ERROR_LOG = "ERROR";

@Test
void logsSimpleCall() {
Expand Down Expand Up @@ -161,7 +163,29 @@ void logsShortArray() {
Matchers.stringContainsInOrder(shorts)
);
}


@Test
void logsWithErrorExceptionLevel() throws Exception {
final StringWriter writer = new StringWriter();
Logger.getRootLogger().addAppender(
new WriterAppender(new SimpleLayout(), writer)
);
try {
LoggableTest.Foo.errorExceptionLogging();
} catch(Exception e) {
// Assert prepend DEBUG
MatcherAssert.assertThat(
writer.toString(),
new LoggableTest.RegexContainsMatcher(DEBUG_LOG)
);
// Assert exception ERROR
MatcherAssert.assertThat(
writer.toString(),
new LoggableTest.RegexContainsMatcher(ERROR_LOG)
);
}
}

@Test
void logsWithExplicitLoggerName() throws Exception {
final StringWriter writer = new StringWriter();
Expand Down Expand Up @@ -235,6 +259,15 @@ public static String text() throws Exception {
public static String explicitLoggerName() {
return LoggableTest.Foo.hiddenText();
}

/**
* Method annotated with Loggable specifying exceptionLevel.
* @return A String
*/
@Loggable(value = Loggable.DEBUG, exceptionLevel = Loggable.ERROR, prepend = true)
public static String errorExceptionLogging() {
throw new RuntimeException();
}

/**
* Revert string.
Expand Down
Loading