Improvements in audit logger for onboarding

A few optimizations, more unit-tests,
log via the handling resource's logger instead
of filter's logger.

Change-Id: I40cef2c86a82b25ded1f8fdca1ec3b0f2fe062d8
Issue-ID: SDC-1451
Signed-off-by: vempo <vitaliy.emporopulo@amdocs.com>

4 files changed, 66 insertions, 78 deletions

diff --git a/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/api/LoggerFactory.java b/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/api/LoggerFactory.java
index 1f3df8bcc0..96debb56b0 100644
--- a/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/api/LoggerFactory.java
+++ b/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/api/LoggerFactory.java
@@ -36,8 +36,8 @@ public class LoggerFactory {
 
     // use the no-op service to prevent recursion in case of an attempt to log an exception as a
     // result of a logger initialization error
-    private static final LoggerCreationService SERVICE = ServiceBinder.getCreationServiceBinding().orElse(
-            new NoOpLoggerCreationService());
+    private static final LoggerCreationService SERVICE = ServiceBinder.getCreationServiceBinding().orElseGet(
+            NoOpLoggerCreationService::new);
 
     private LoggerFactory() {
         // prevent instantiation
@@ -53,10 +53,10 @@ public class LoggerFactory {
 
     private static class NoOpLoggerCreationService implements LoggerCreationService {
 
-        private static final Logger NO_OP_LOGGER = new NoOpLogger();
-
         private static class NoOpLogger implements Logger {
 
+            private static final Logger INSTANCE = new NoOpLogger();
+
             @Override
             public String getName() {
                 return "No-Op Logger";
@@ -211,13 +211,13 @@ public class LoggerFactory {
         @Override
         public Logger getLogger(String className) {
             Objects.requireNonNull(className, "Name cannot be null");
-            return NO_OP_LOGGER;
+            return NoOpLogger.INSTANCE;
         }
 
         @Override
         public Logger getLogger(Class<?> clazz) {
             Objects.requireNonNull(clazz, "Class cannot be null");
-            return NO_OP_LOGGER;
+            return NoOpLogger.INSTANCE;
         }
     }
 }
diff --git a/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/api/LoggingContext.java b/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/api/LoggingContext.java
index c17149e98e..894dd2c00c 100644
--- a/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/api/LoggingContext.java
+++ b/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/api/LoggingContext.java
@@ -36,7 +36,7 @@ import org.openecomp.sdc.logging.spi.LoggingContextService;
 public class LoggingContext {
 
     private static final LoggingContextService SERVICE =
-            ServiceBinder.getContextServiceBinding().orElse(new NoOpLoggingContextService());
+            ServiceBinder.getContextServiceBinding().orElseGet(NoOpLoggingContextService::new);
 
     private LoggingContext() {
         // prevent instantiation
@@ -64,8 +64,6 @@ public class LoggingContext {
 
     private static class NoOpLoggingContextService implements LoggingContextService {
 
-        private static final ContextData EMPTY_CONTEXT = ContextData.builder().build();
-
         @Override
         public void put(ContextData contextData) {
             Objects.requireNonNull(contextData, "Context data cannot be null");
@@ -73,7 +71,7 @@ public class LoggingContext {
 
         @Override
         public ContextData get() {
-            return EMPTY_CONTEXT;
+            return EmptyContextData.INSTANCE;
         }
 
         @Override
@@ -92,5 +90,14 @@ public class LoggingContext {
             Objects.requireNonNull(callable, "Callable cannot be null");
             return callable;
         }
+
+        private static class EmptyContextData {
+
+            private static final ContextData INSTANCE = ContextData.builder().build();
+
+            private EmptyContextData() {
+                // prevent instantiation
+            }
+        }
     }
 }
diff --git a/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/servlet/jaxrs/LoggingRequestFilter.java b/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/servlet/jaxrs/LoggingRequestFilter.java
index 498587414a..0e1b7d715b 100644
--- a/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/servlet/jaxrs/LoggingRequestFilter.java
+++ b/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/servlet/jaxrs/LoggingRequestFilter.java
@@ -19,12 +19,10 @@ package org.openecomp.sdc.logging.servlet.jaxrs;
 import static org.openecomp.sdc.logging.LoggingConstants.DEFAULT_PARTNER_NAME_HEADER;
 import static org.openecomp.sdc.logging.LoggingConstants.DEFAULT_REQUEST_ID_HEADER;
 
-import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
 import java.util.UUID;
+import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.container.ContainerRequestFilter;
-import javax.ws.rs.container.ResourceInfo;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.ext.Provider;
 import org.openecomp.sdc.logging.api.ContextData;
@@ -66,19 +64,27 @@ public class LoggingRequestFilter implements ContainerRequestFilter {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(LoggingRequestFilter.class);
 
-    private ResourceInfo resource;
+    private HttpServletRequest httpRequest;
 
     private HttpHeader requestIdHeader = new HttpHeader(DEFAULT_REQUEST_ID_HEADER);
     private HttpHeader partnerNameHeader = new HttpHeader(DEFAULT_PARTNER_NAME_HEADER);
+    private boolean includeHttpMethod = true;
 
     /**
-     * Injection of a resource that matches the request from JAX-RS context.
+     * Injection of HTTP request object from JAX-RS context.
      *
-     * @param resource automatically injected by JAX-RS container
+     * @param httpRequest automatically injected by JAX-RS container
      */
     @Context
-    public void setResource(ResourceInfo resource) {
-        this.resource = resource;
+    public void setHttpRequest(HttpServletRequest httpRequest) {
+        this.httpRequest = httpRequest;
+    }
+
+    /**
+     * Configuration parameter to include the HTTP method of a request in service name.
+     */
+    public void setHttpMethodInServiceName(boolean includeHttpMethod) {
+        this.includeHttpMethod = includeHttpMethod;
     }
 
     /**
@@ -100,22 +106,10 @@ public class LoggingRequestFilter implements ContainerRequestFilter {
     @Override
     public void filter(ContainerRequestContext containerRequestContext) {
 
-        if (resource == null) {
-            // JAX-RS could not find a mapping this response, probably due to HTTP 404 (not found),
-            // 405 (method not allowed), 415 (unsupported media type), etc. with a message in Web server log
-
-            if (LOGGER.isDebugEnabled()) {
-                LOGGER.debug("No matching resource was found for URI '{}' and method '{}'",
-                        containerRequestContext.getUriInfo().getPath(), containerRequestContext.getMethod());
-            }
-
-            return;
-        }
+        LoggingContext.clear();
 
         containerRequestContext.setProperty(START_TIME_KEY, System.currentTimeMillis());
 
-        LoggingContext.clear();
-
         ContextData.ContextDataBuilder contextData = ContextData.builder();
         contextData.serviceName(getServiceName());
 
@@ -131,48 +125,12 @@ public class LoggingRequestFilter implements ContainerRequestFilter {
     }
 
     private String getServiceName() {
-
-        Class<?> resourceClass = resource.getResourceClass();
-        Method resourceMethod = resource.getResourceMethod();
-
-        if (Proxy.isProxyClass(resourceClass)) {
-            LOGGER.debug("Proxy class injected for JAX-RS resource");
-            return getServiceNameFromJavaProxy(resourceClass, resourceMethod);
-        }
-
-        return formatServiceName(resourceClass, resourceMethod);
+        return includeHttpMethod
+                       ? formatServiceName(this.httpRequest.getMethod(), this.httpRequest.getRequestURI())
+                       : this.httpRequest.getRequestURI();
     }
 
-    private String getServiceNameFromJavaProxy(Class<?> proxyType, Method resourceMethod) {
-
-        for (Class<?> interfaceType : proxyType.getInterfaces()) {
-
-            if (isMatchingInterface(interfaceType, resourceMethod)) {
-                return formatServiceName(interfaceType, resourceMethod);
-            }
-        }
-
-        LOGGER.debug("Failed to find method '{}' in interfaces implemented by injected Java proxy", resourceMethod);
-        return formatServiceName(proxyType, resourceMethod);
-    }
-
-    private String formatServiceName(Class<?> resourceClass, Method resourceMethod) {
-        return resourceClass.getName() + "#" + resourceMethod.getName();
-    }
-
-    private boolean isMatchingInterface(Class<?> candidateType, Method requestedMethod) {
-
-        try {
-
-            Method candidate = candidateType.getDeclaredMethod(requestedMethod.getName(),
-                    requestedMethod.getParameterTypes());
-            return candidate != null;
-
-        } catch (NoSuchMethodException ignored) {
-            // ignore and move on to the next
-            LOGGER.debug("Failed to find method '{}' in interface '{}'", requestedMethod, candidateType);
-        }
-
-        return false;
+    static String formatServiceName(String httpMethod, String requestUri) {
+        return httpMethod + " " + requestUri;
     }
 }
diff --git a/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/servlet/jaxrs/LoggingResponseFilter.java b/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/servlet/jaxrs/LoggingResponseFilter.java
index fbe28a79eb..e0353a43ca 100644
--- a/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/servlet/jaxrs/LoggingResponseFilter.java
+++ b/openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main/java/org/openecomp/sdc/logging/servlet/jaxrs/LoggingResponseFilter.java
@@ -23,6 +23,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.container.ContainerResponseContext;
 import javax.ws.rs.container.ContainerResponseFilter;
+import javax.ws.rs.container.ResourceInfo;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.ext.Provider;
@@ -53,7 +54,9 @@ import org.openecomp.sdc.logging.api.StatusCode;
 @Provider
 public class LoggingResponseFilter implements ContainerResponseFilter {
 
-    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+    private static final int UNKNOWN_START_TIME = 0;
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(LoggingResponseFilter.class);
 
     /**
      * Tracks reporting configuration problems to the log. We want to report them only once, and not to write to log
@@ -63,6 +66,8 @@ public class LoggingResponseFilter implements ContainerResponseFilter {
 
     private HttpServletRequest httpRequest;
 
+    private ResourceInfo resource;
+
     /**
      * Injection of HTTP request object from JAX-RS context.
      *
@@ -73,12 +78,29 @@ public class LoggingResponseFilter implements ContainerResponseFilter {
         this.httpRequest = httpRequest;
     }
 
+    /**
+     * Injection of a resource that matches the request from JAX-RS context.
+     *
+     * @param resource automatically injected by JAX-RS container
+     */
+    @Context
+    public void setResource(ResourceInfo resource) {
+        this.resource = resource;
+    }
+
     @Override
     public void filter(ContainerRequestContext containerRequestContext,
             ContainerResponseContext containerResponseContext) {
 
         try {
+
+            if ((resource == null) || (resource.getResourceClass() == null)) {
+                LOGGER.debug("No matching resource, skipping audit.");
+                return;
+            }
+
             writeAudit(containerRequestContext, containerResponseContext);
+
         } finally {
             LoggingContext.clear();
         }
@@ -87,7 +109,8 @@ public class LoggingResponseFilter implements ContainerResponseFilter {
     private void writeAudit(ContainerRequestContext containerRequestContext,
             ContainerResponseContext containerResponseContext) {
 
-        if (!logger.isAuditEnabled()) {
+        Logger resourceLogger = LoggerFactory.getLogger(resource.getResourceMethod().getDeclaringClass());
+        if (!resourceLogger.isAuditEnabled()) {
             return;
         }
 
@@ -102,7 +125,7 @@ public class LoggingResponseFilter implements ContainerResponseFilter {
                                        .responseCode(Integer.toString(responseCode))
                                        .responseDescription(statusInfo.getReasonPhrase())
                                        .clientIpAddress(httpRequest.getRemoteAddr()).build();
-        logger.audit(auditData);
+        resourceLogger.audit(auditData);
     }
 
     private boolean isSuccess(int responseCode) {
@@ -122,7 +145,7 @@ public class LoggingResponseFilter implements ContainerResponseFilter {
     private long handleMissingStartTime() {
         reportConfigProblem("{} key was not found in JAX-RS request context. "
                 + "Make sure you configured a request filter", LoggingRequestFilter.START_TIME_KEY);
-        return 0;
+        return UNKNOWN_START_TIME;
     }
 
     private long parseStartTime(Object startTime) {
@@ -131,7 +154,7 @@ public class LoggingResponseFilter implements ContainerResponseFilter {
             return Long.class.cast(startTime);
         } catch (ClassCastException e) {
             reportConfigProblem("{} key in JAX-RS request context contains an object of type '{}', but 'java.lang.Long'"
-                    + " is expected", LoggingRequestFilter.START_TIME_KEY, startTime.getClass().getName());
+                    + " is expected", LoggingRequestFilter.START_TIME_KEY, startTime.getClass().getName(), e);
             return 0;
         }
     }
@@ -140,7 +163,7 @@ public class LoggingResponseFilter implements ContainerResponseFilter {
 
         if (reportBadConfiguration) {
             reportBadConfiguration = false;
-            logger.error(message, arguments);
+            LOGGER.error(message, arguments);
         }
     }
 }