MigrateWebMvcTagsToObservationConvention Recipe crashes

[123Haynes]

What version of OpenRewrite are you using?

I am using

• OpenRewrite v8.54.0
• Maven/Gradle plugin v6.10.0
• rewrite-spring v6.8.2

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

            <plugin>
                <groupId>org.openrewrite.maven</groupId>
                <artifactId>rewrite-maven-plugin</artifactId>
                <version>6.10.0</version>
                <configuration>
                    <exportDatatables>true</exportDatatables>
                    <activeRecipes>
                        <recipe>org.openrewrite.java.spring.boot3.MigrateWebMvcTagsToObservationConvention</recipe>
                    </activeRecipes>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>org.openrewrite.recipe</groupId>
                        <artifactId>rewrite-spring</artifactId>
                        <version>6.8.2</version>
                    </dependency>
                </dependencies>
            </plugin>

What is the smallest, simplest way to reproduce the problem?

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.springframework.boot.actuate.metrics.web.servlet.WebMvcTagsProvider;

import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Tags;
import lombok.AllArgsConstructor;

@AllArgsConstructor
public class WebMvcExample implements WebMvcTagsProvider {

    /**
     * Instance of {@link WebMvcTagsProvider}.
     */
    private final WebMvcTagsProvider webMvcTagsProvider;

    @Override
    public Iterable<Tag> getTags(final HttpServletRequest request, final HttpServletResponse response, final Object handler, final Throwable exception) {
        return Tags.of(webMvcTagsProvider.getTags(request, response, handler, exception)).and("ABC", "ABC");
    }

    @Override
    public Iterable<Tag> getLongRequestTags(final HttpServletRequest request, final Object handler) {
        return Tags.of(webMvcTagsProvider.getLongRequestTags(request, handler)).and("ABC", "ABC");
    }
}

What did you expect to see?

I expect to either see the working migration or that nothing changes and openrewrite continues to the next recipe.

What did you see instead?

Openrewrite logs an exception and ends without applying any migrations.

What is the full stack trace of any errors you encountered?

Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:6.10.0:run failed: Error while visiting src\main\java\example\WebMvcExample .java: java.lang.ClassCastException: class org.openrewrite.java.tree.J$MethodInvocation cannot be cast to class org.openrewrite.java.tree.J$Identifier (org.openrewrite.java.tree.J$MethodInvocation and org.openrewrite.java.tree.J$Identifier are in unnamed module of loader org.codehaus.plexus.classworlds.realm.ClassRealm @12cd129c)
[ERROR]   org.openrewrite.java.spring.boot3.MigrateWebMvcTagsToObservationConvention$1.refactorTagsUsage(MigrateWebMvcTagsToObservationConvention.java:199)
[ERROR]   org.openrewrite.java.spring.boot3.MigrateWebMvcTagsToObservationConvention$1.visitStatement(MigrateWebMvcTagsToObservationConvention.java:190)
[ERROR]   org.openrewrite.java.spring.boot3.MigrateWebMvcTagsToObservationConvention$1.visitStatement(MigrateWebMvcTagsToObservationConvention.java:71)
[ERROR]   org.openrewrite.java.JavaVisitor.visitMethodInvocation(JavaVisitor.java:887)
[ERROR]   org.openrewrite.java.spring.boot3.MigrateWebMvcTagsToObservationConvention$1.visitMethodInvocation(MigrateWebMvcTagsToObservationConvention.java:281)
[ERROR]   org.openrewrite.java.spring.boot3.MigrateWebMvcTagsToObservationConvention$1.visitMethodInvocation(MigrateWebMvcTagsToObservationConvention.java:71)
[ERROR]   org.openrewrite.java.tree.J$MethodInvocation.acceptJava(J.java:4281)
[ERROR]   org.openrewrite.java.tree.J.accept(J.java:63)
[ERROR]   org.openrewrite.TreeVisitor.visit(TreeVisitor.java:245)
[ERROR]   org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:311)
[ERROR]   org.openrewrite.java.JavaVisitor.visitRightPadded(JavaVisitor.java:1380)
[ERROR]   org.openrewrite.java.JavaVisitor.visitMethodInvocation(JavaVisitor.java:907)
[ERROR]   org.openrewrite.java.spring.boot3.MigrateWebMvcTagsToObservationConvention$1.visitMethodInvocation(MigrateWebMvcTagsToObservationConvention.java:281)
[ERROR]   org.openrewrite.java.spring.boot3.MigrateWebMvcTagsToObservationConvention$1.visitMethodInvocation(MigrateWebMvcTagsToObservationConvention.java:71)
[ERROR]   org.openrewrite.java.tree.J$MethodInvocation.acceptJava(J.java:4281)
[ERROR]   org.openrewrite.java.tree.J.accept(J.java:63)
[ERROR]   ...

Are you interested in contributing a fix to OpenRewrite?

yes, but I don't know much about Java Recipes yet so I'd need help with it for sure.

[123Haynes]

I tried debugging this a bit further.
Seems that the method getLongRequestTags in my example class is the problem.
Without it the recipe works... kinda...

I get the following output in that case:

import io.micrometer.common.KeyValues;
import org.springframework.boot.actuate.metrics.web.servlet.WebMvcTagsProvider;
import org.springframework.http.server.observation.DefaultServerRequestObservationConvention;
import org.springframework.http.server.observation.ServerRequestObservationContext;

public class WebMvcExample extends DefaultServerRequestObservationConvention {

    /**
     * Instance of {@link WebMvcTagsProvider}.
     */
    private final WebMvcTagsProvider webMvcTagsProvider;

    public WebMvcExample(WebMvcTagsProvider webMvcTagsProvider) {
        this.webMvcTagsProvider = webMvcTagsProvider;
    }

    @Override
    public KeyValues getHighCardinalityKeyValues(ServerRequestObservationContext context) {
        KeyValues values = super.getHighCardinalityKeyValues(context);
        return values;
    }
}

Seems like the custom Tag that was added with .and(...) before, completely vanished after the migration.

So there are 2 issues:

• The crash since the recipe can't handle all methods
• The implementation if and() is used
  I think at least the crash should be adressed. Maybe with an additional precondition?

Edit:
After debugging this a bit more and looking at the recipe code I think the easiest solution is to add a precondition.
The WebMvcTagsProvider interface defined 2 methods. getTags and getLongRequestTags.
Currently the recipe can only handle getTags.
So I think the easiest solution would be to add a precondition that checks if a class declares getLongRequestTags and only apply the migration if that is not the case.

So probably change https://github.com/openrewrite/rewrite-spring/blob/main/src/main/java/org/openrewrite/java/spring/boot3/MigrateWebMvcTagsToObservationConvention.java#L71
to something like:

        return Preconditions.check(Preconditions.and(new FindImplementations(WEBMVCTAGSPROVIDER_FQ), Preconditions.not(new DeclaresMethod<>(GET_LONG_REQUEST_TAGS)), new JavaVisitor<ExecutionContext>() {

This exact code won't work though because FindImplementations is a recipe and DeclaresMethod is a JavaIsoVisitor.
If someone can point me in the right direction how to correctly code this precondition I'll gladly open a PR to implement this :)

[timtebeek]

Hi @123Haynes thanks for the report and troubleshooting already. Would it work if you use new FindImplementations(WEBMVCTAGSPROVIDER_FQ).getVisitor() ?

Good indeed to make the recipe safely back off first, before looking at extending coverage.

[123Haynes]

@timtebeek That worked, thanks 😄 I've created a PR to at least adress the crashing of the recipe for now.

[timtebeek]

Great! That buys us some time to look towards a proper fix as well, but good to not many any incorrect changes already. :)