brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: Tidy logging and tests
Date Fri, 18 Jul 2014 08:36:03 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/77#discussion_r15101026
  
    --- Diff: usage/rest-server/src/main/java/brooklyn/rest/security/BrooklynPropertiesSecurityFilter.java
---
    @@ -73,18 +75,50 @@ public void doFilter(ServletRequest request, ServletResponse response,
FilterCha
                     // do nothing here, fall through to below
                 } else {
                     WebEntitlementContext entitlementContext = null;
    +                String uri = ((HttpServletRequest)request).getRequestURI();
                     try {
    -                    entitlementContext = new WebEntitlementContext(user, ((HttpServletRequest)request).getRemoteAddr(),
((HttpServletRequest)request).getRequestURI());
    +                    String uid = Integer.toHexString(hashCode());
    +                    entitlementContext = new WebEntitlementContext(user, ((HttpServletRequest)request).getRemoteAddr(),
uri, uid);
    +                    if (originalRequest.get()==null) {
    +                        // initial filter application
    +                        originalRequest.set(uri);
    +                    } else {
    +                        // this filter is being applied *again*, probably due to forwarding
(e.g. from '/' to '/index.html')
    +                        log.debug("REST request {} being forwarded from {} to {}", new
Object[] { uid, originalRequest.get(), uri });
    +                        // clear the entitlement context before setting to avoid warnings
    +                        Entitlements.clearEntitlementContext();
    +                    }
                         Entitlements.setEntitlementContext(entitlementContext);
    -                    log.debug("REST starting processing request {}", entitlementContext);
    +                    
    +                    log.debug("REST starting processing request {} with {}", uri, entitlementContext);
    +                    if (!((HttpServletRequest)request).getParameterMap().isEmpty()) {
    +                        log.debug("  REST req {} parameters: {}", uid, ((HttpServletRequest)request).getParameterMap());
    +                    }
    +                    if (((HttpServletRequest)request).getContentLength()>0) {
    +                        int len = ((HttpServletRequest)request).getContentLength();
    +                        log.debug("  REST req {} upload content type {}", uid, ((HttpServletRequest)request).getContentType()+"
length "+len
    +                            // would be nice to do this, but the stream can only be read
once;
    +                            // TODO figure out how we could peek at it
    +//                            +(len>0 && len<4096 ? ""+Streams.readFullyString(((HttpServletRequest)request).getInputStream())
: "") 
    +                            );
    +                    }
    +                    
                         chain.doFilter(request, response);
    -                    log.debug("REST completed processing request {}", entitlementContext);
    +                    
    +                    log.debug("REST completed, code {}, processing request {} with {}",

    +                        new Object[] { ((HttpServletResponse)response).getStatus(), uri,
entitlementContext } );
                         return;
    +                    
                     } catch (Throwable e) {
    +                    // NB errors are typically already caught at this point
                         if (log.isDebugEnabled())
    -                        log.debug("REST failed processing request "+entitlementContext+":
"+e, e);
    +                        log.debug("REST failed processing request "+uri+" with "+entitlementContext+":
"+e, e);
    +                    
    +                    log.warn("REST failed processing request "+uri+" with "+entitlementContext+":
"+e, e);
    --- End diff --
    
    Looks like we log this at debug and then again at warn (identical). Why not just warn?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message