openrewrite / rewrite-logging-frameworks

OpenRewrite recipes for assisting with Java logging migration tasks.
Apache License 2.0
25 stars 20 forks source link

SLF4J Fluent logging api migration recipe #67

Open timtebeek opened 2 years ago

timtebeek commented 2 years ago

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.
soloturn commented 9 months ago

@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 commented 9 months ago

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

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