Spring Only log status in ServletRequestHandledEvent

The ServletRequestHandledEvent is very convenient for logging handled HTTP servlet requests.

For example in Spring Boot Web (Kotlin):

/**
 * An [EventListener] for logging handled HTTP requests.
 */
@Component
class HttpRequestEventListener {

    private val log = KotlinLogging.logger {}

    @EventListener
    fun handle(event: RequestHandledEvent) {
        log.trace { "HTTP request handled: ${event.description}" }
    }
}

... this logs all requests as:

HTTP request handled: url=[/example]; client=[127.0.0.1]; method=[GET]; servlet=[dispatcherServlet]; session=[null]; user=[null]; time=[115ms]; status=[OK]

However, this is a bit confusing at first, as this will print "status=OK" even when the HTTP response status code is not 200. The status comes from the RequestHandledEvent here, which prints "OK" if there was no exception.

In this pull request the HTTP status code was added to ServletRequestHandledEvent, but it is not included in the description.

Would it make sense to include it in the description too?

To return, for example, this

url=[/example]; client=[127.0.0.1]; method=[GET]; servlet=[dispatcherServlet]; session=[null]; user=[null]; time=[115ms]; status=[OK]; statusCode=[404]

Of course, it's easy enough to adjust the logging in our event handler, but including the statusCode seems like a generally useful thing to me.

If this would be a welcome addition, I can provide a pull request.

Comment From: snicoll

I agree that the OK status if there isn't a failure is a bit confusing. I am not sure we'd go as far as adding the description to it. Adding to team attention to get more feedback from the team.

Comment From: rstoyanchev

I think the base class should not log "status=[OK|Failed:...]" since it doesn't know the status and it's misleading to log OK which implies 200 but the actual status could be even in the 4xx-5xx range. The base class should focus on logging "failure:..." if there is one. The ServletRequestHandledEvent subclass on the other hand should log the actual status.

Comment From: snicoll

Sounds good, I've requalified this issue in that direction.