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.

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

SLF4J Fluent logging api migration recipe #67

Open
timtebeek opened this issue Aug 21, 2022 · 2 comments
Open

SLF4J Fluent logging api migration recipe #67

timtebeek opened this issue Aug 21, 2022 · 2 comments
Labels
recipe Recipe Requested

Comments

@timtebeek
Copy link
Contributor

SLF4J 2.0.0 has been released: https://www.slf4j.org/faq.html#changesInVersion200
That means we can now adopt the fluent logging api: https://www.slf4j.org/manual.html#fluent

I expect this to be most useful around guard blocks such as the following:

void before() {
	if (log.isInfoEnabled()) {
		log.info("Log an expensive argument", expensiveArgument());
	}
}
void after() {
	log.atInfo()
		.addArgument(() -> expensiveArgument())
		.log("Log an expensive argument");
}
static List<String> expensiveArgument() {
	return new ArrayList<>(100_000_000);
}

But it can also be adopted independent of the guard blocks used previously.

The following log statements are equivalent in their output (for the default implementation):

int newT = 15;
int oldT = 16;

// using traditional API
logger.debug("Temperature set to {}. Old temperature was {}.", newT, oldT);

// using fluent API, add arguments one by one and then log message
logger.atDebug().addArgument(newT).addArgument(oldT).log("Temperature set to {}. Old temperature was {}.");

// using fluent API, log message with arguments
logger.atDebug().log("Temperature set to {}. Old temperature was {}.", newT, oldT);

// using fluent API, add one argument and then log message providing one more argument
logger.atDebug().addArgument(newT).log("Temperature set to {}. Old temperature was {}.", oldT);

// using fluent API, add one argument with a Supplier and then log message with one more argument.
// Assume the method t16() returns 16.
logger.atDebug().addArgument(() -> t16()).log(msg, "Temperature set to {}. Old temperature was {}.", oldT);

Migration would like to need to change the dependency version of both the api and the bindings.

@timtebeek timtebeek added the recipe Recipe Requested label Nov 30, 2023
@soloturn
Copy link

soloturn commented Dec 2, 2023

@timtebeek can you please point to one example in the openrewrite code which does a similar rearrangement of parameters, for the 2nd use case? am looking for terasology, where the guards are not so important, but method calls like this:

-        logger.warn("String [{}] does not match the conditions: {}", value,
-                predicates.stream()
+        logger.atWarn().
+                addArgument(value).
+                addArgument(() -> predicates.stream()
                         .filter(p -> !p.test(value))
                         .map(StringConstraint::getDescription)
-                        .collect(Collectors.joining(",", "[", "]")));
+                        .collect(Collectors.joining(",", "[", "]"))).
+                log("String [{}] does not match the conditions: {}");

i saw the slf4j yml, https://github.com/openrewrite/rewrite-logging-frameworks/blob/main/src/main/resources/META-INF/rewrite/slf4j.yml , with old and new method. and i am looking at the method ParameterizedLogging. but i am not sure this is the best example, as it tinkers with the log message which was not the first goal.

@timtebeek
Copy link
Contributor Author

Thanks for looking into this @soloturn ! There's not really a good example that comes to mind here, and it's an interesting one given the variable number of arguments to any logging statements. I'd lean towards using a context sensitive JavaTemplate here, where you gradually build up the template string based on how many logging arguments there are now.

Perhaps the best way to start is with draft pull request containing just a placeholder recipe & visitor, and a suite of small targeted tests that assert what we want as in and output. I'm thinking of individual tests to test the handling of

  • a single literal argument,
  • a single variable argument
  • a single method invocation argument, which becomes a lambda
  • an exception argument that triggers a stacktrace
  • multiple arguments
  • ...

That way we have a clear goal to work towards, and can more easily figure out when we have sufficient coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Recipe Requested
Projects
Status: Recipes Wanted
Development

No branches or pull requests

2 participants