From c6a769eff2930fcf4f4dadb8106937abf86d8638 Mon Sep 17 00:00:00 2001 From: vempo Date: Wed, 27 Jun 2018 20:36:49 +0300 Subject: 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 --- .../openecomp/sdc/logging/api/LoggerFactory.java | 12 ++-- .../openecomp/sdc/logging/api/LoggingContext.java | 15 ++-- .../servlet/jaxrs/LoggingRequestFilter.java | 82 ++++++---------------- .../servlet/jaxrs/LoggingResponseFilter.java | 35 +++++++-- 4 files changed, 66 insertions(+), 78 deletions(-) (limited to 'openecomp-be/lib/openecomp-sdc-logging-lib/openecomp-sdc-logging-api/src/main') 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); } } } -- cgit 1.2.3-korg