SpringBoot MVC and WebFlux metrics filters adversely affect the response when metrics recording fails

We provided a custom WebMvcTagsProvider to add some additional Tags. One of the tags we added was an accept header. In one of our tests we do not pass a value for this header and it was coming back as null. Since Tag cannot take a null value this returned a NullPointer. The result of this was that the response returned a 200 but the content of the response could not be processed by an Apache HTTP Client. I'm not sure why the apache client cannot parse the message as other libraries seem to be able to handle it. But regardless, I think the behavior of the WebMvCMetricsFilter should have a more predictable behavior in case of this issue like not messing up the body or returning some kind of 5xx response.

WebMvCMetricsFilter should be wrapped in a try catch when it goes to get the Tags so that it does not cause failures like this. Or it should fail in a way that a non 200 response is returned. To reproduce this error you can use this CustomTagsProvider:

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

import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.actuate.metrics.web.servlet.WebMvcTags;
import org.springframework.boot.actuate.metrics.web.servlet.WebMvcTagsProvider;
import org.springframework.http.MediaType;
import org.springframework.stereotype.Component;

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

@Component
public class CustomWebMvcTagsProvider implements WebMvcTagsProvider {

    @Override
    public Iterable<Tag> getTags(HttpServletRequest request, HttpServletResponse response, Object handler, Throwable exception) {
        throw new NullPointerException();
    }

    @Override
    public Iterable<Tag> getLongRequestTags(HttpServletRequest request, Object handler) {
        return Tags.of(WebMvcTags.method(request), WebMvcTags.uri(request, null));
    }

}

The client code that reproduces this failure is this:

public class App2 {

    public static void main(String[] args) {

        DefaultHttpClient httpClient = new DefaultHttpClient();

        HttpPost p = new HttpPost("http://localhost:8080/someEndpoint");
        HttpRequestBase r = p;
        r.setHeader(HTTP.CONN_DIRECTIVE, HTTP.CONN_KEEP_ALIVE);

        try {
            HttpResponse res = httpClient.execute(r);
            System.out.println("Content Length : " + res.getEntity().getContentLength());
            System.out.println("Content Type : " + res.getEntity().getContentType());
            BufferedReader rd = new BufferedReader(new InputStreamReader(res.getEntity().getContent()));

            String content = EntityUtils.toString(res.getEntity());
            //bwr.write(result.toString());
            //bwr.flush();
            //bwr.close();
            System.out.println("Response ---- > ");
            System.out.println("Response Status = " + res.getStatusLine());
            System.out.println("Response = " + content.toString());

        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}

The dependencies for the apache client code is:

    <dependencies>
        <dependency>
            <groupId>org.apache.httpcomponents</groupId>
            <artifactId>httpclient</artifactId>
            <version>4.5.5</version>
        </dependency>

        <dependency>
            <groupId>org.apache.httpcomponents</groupId>
            <artifactId>httpclient-cache</artifactId>
            <version>4.5.5</version>
        </dependency>

        <dependency>
            <groupId>org.apache.httpcomponents</groupId>
            <artifactId>httpasyncclient</artifactId>
            <version>4.1.4</version>
        </dependency>

        <dependency>
            <groupId>org.apache.httpcomponents</groupId>
            <artifactId>httpcore</artifactId>
            <version>4.4.4</version>
        </dependency>
    </dependencies>

Comment From: wilkinsona

WebFlux is also affected by this.