Filter for logging http request parameters

Question

I have written a filter that intercepts http calls and logs request parameters.

public void doFilter(ServletRequest request, ServletResponse response,
        FilterChain filterChain) throws IOException,
        ServletException
    {
        HttpServletRequest httpRequest = (HttpServletRequest) request;
        HttpServletResponse httpResponse = (HttpServletResponse) response;
        String uri = httpRequest.getRequestURI();
        String queryString = httpRequest.getQueryString();
        long startTime = System.currentTimeMillis();
        if (shouldBeRedirected(uri)) {
            String type = getType(uri);
            String redirectUrl = Util.getURLBasedUponStabilityLevel(Constants.STABILITY_LEVEL);
            ((HttpServletResponse) response).sendRedirect(redirectUrl + type);
            return;
        }
        filterChain.doFilter(httpRequest, httpResponse);
        long endTime = System.currentTimeMillis();
        Timestamp startTimestamp = Timestamp.getInstance(startTime);

        StringBuilder logString = new StringBuilder();
        logString.append("Request URL: ");
        logString.append(uri);
        logString.append(" Query String: ");
        logString.append(queryString);
        logString.append(" Session Id: ");
        logString.append(httpRequest.getRequestedSessionId());
        logString.append(" Took: ");
        logString.append(endTime - startTime);
        logString.append("milliseconds. ");
        logString.append(" StartTime: ");
        logString.append(startTimestamp);
        if(httpRequest.getMethod().equals("POST")){
            JSONObject postParameters = new JSONObject();
            Map parameterMap = httpRequest.getParameterMap();
            Iterator<Map.Entry<String, String[]>> it = parameterMap.entrySet().iterator();
            while (it.hasNext()) {
               Map.Entry<String, String[]> entry = it.next();
               if(entry.getValue().length == 1){
                   postParameters.put(entry.getKey(), entry.getValue()[0].toString());}
               else{
                  JSONArray valueList = new JSONArray();
                  for(String value : entry.getValue()){
                      valueList.add(value.toString());
                  }
                  postParameters.put(entry.getKey(), valueList);
               }
            }
            logString.append(" Post Parameters: ");
            logString.append(postParameters.toString());
        }
        LOG.info(logString.toString());
    }

Code looks a bit clumsy to me, any suggestions to improve this?

Answer 1

The redirect doesn't seem to be part of the logging. Maybe put it in a separate filter that runs before this one?

The logged string could be built using String.format() or MessageFormat which would be more readable. Most logging frameworks have such formatting built into their logging methods anyway.

Do you really need to convert the post parameters into JSON? Isn't the standard toString representation of the Map good enough for logging? Apart from that most JSON libraries have a built-in mechanism to convert a Map into a JSON object so you don't need to do it yourself.

Answer 2

Elaborating on RoToRa's comments:

1. If you are going to build a string by concatenation, then using a string builder like that only improves performance if there is a loop. That doesn't apply here. So your code could be written as:

   String logString = "Request URL: ") + uri + " Query String: " + queryString
           + " Session Id: " + httpRequest.getRequestedSessionId() + " Took: "
           + (endTime - startTime) + " milliseconds. " + " StartTime: "
           + startTimestamp 
           + (httpRequest.getMethod().equals("POST") ? 
                 httpRequest.getParameterMap() : "");
   LOG.info(logString);
   

2. You should be using a format String and parameters; e.g.

   LOG.info("Request URL: {} Query String: {} Session Id: {} Took: {} " +
            "milliseconds. StartTime: {} {}",
            uri, queryString, httpRequest.getRequestedSessionId(), 
            (endTime - startTime), startTimestamp,
            (httpRequest.getMethod().equals("POST") ?
                 httpRequest.getParameterMap() : ""));
   

   This is not just neater. It is also a lot more efficient, since the work of interpolating the parameters into the format will only happen if "info" level logging is enabled.

   (Just using String.format is not the solution ...)