Skip to content

Commit

Permalink
PAYARA-4123 Refactored server JAXRS Request Tracing
Browse files Browse the repository at this point in the history 
- removed filter + exception mapper solution, which failed
  TCK's completitioncallback tests
- introduced EventListener solution, depends on Jersey
- removed trailing whitespaces in the module
- added logging and several generics
- reduced nesting of blocks in favor of readibility
- EnvironmentConfigSource uses AccessController to avoid security exceptions
  • Loading branch information
David Matejcek committed Sep 16, 2019
1 parent aa770aa commit 0512c51
Show file tree
Hide file tree
Showing 15 changed files with 961 additions and 916 deletions.
155 changes: 79 additions & 76 deletions ...jaxws/src/main/java/fish/payara/opentracing/jaxws/JaxwsContainerRequestTracingFilter.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/*
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
*
*
* Copyright (c) [2018-2019] Payara Foundation and/or its affiliates. All rights reserved.
*
*
* The contents of this file are subject to the terms of either the GNU
* General Public License Version 2 only ("GPL") or the Common Development
* and Distribution License("CDDL") (collectively, the "License"). You
Expand All @@ -11,20 +11,20 @@
* https://github.com/payara/Payara/blob/master/LICENSE.txt
* See the License for the specific
* language governing permissions and limitations under the License.
*
*
* When distributing the software, include this License Header Notice in each
* file and include the License file at glassfish/legal/LICENSE.txt.
*
*
* GPL Classpath Exception:
* The Payara Foundation designates this particular file as subject to the "Classpath"
* exception as provided by the Payara Foundation in the GPL Version 2 section of the License
* file that accompanied this code.
*
*
* Modifications:
* If applicable, add the following below the License Header, with the fields
* enclosed by brackets [] replaced by your own identifying information:
* "Portions Copyright [year] [name of copyright owner]"
*
*
* Contributor(s):
* If you wish your version of this file to be governed by only the CDDL or
* only the GPL Version 2, indicate your decision by adding "[Contributor]
Expand All @@ -41,22 +41,24 @@

import com.sun.xml.ws.api.message.Message;
import com.sun.xml.ws.api.message.Packet;

import fish.payara.nucleus.requesttracing.RequestTracingService;
import fish.payara.opentracing.OpenTracingService;

import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.SpanContext;
import io.opentracing.Tracer;
import io.opentracing.Tracer.SpanBuilder;
import io.opentracing.propagation.TextMap;
import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.opentracing.Traced;
import org.glassfish.api.invocation.InvocationManager;
import org.glassfish.hk2.api.ServiceLocator;
import org.glassfish.webservices.monitoring.MonitorContext;
import org.glassfish.webservices.monitoring.MonitorFilter;
import org.jvnet.hk2.annotations.Service;

import java.util.AbstractMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.logging.Logger;

import javax.enterprise.inject.spi.BeanManager;
import javax.enterprise.inject.spi.CDI;
Expand All @@ -69,13 +71,15 @@
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
import javax.xml.soap.SOAPException;
import java.util.AbstractMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.logging.Logger;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.opentracing.Traced;
import org.glassfish.api.invocation.InvocationManager;
import org.glassfish.hk2.api.ServiceLocator;
import org.glassfish.webservices.monitoring.MonitorContext;
import org.glassfish.webservices.monitoring.MonitorFilter;
import org.jvnet.hk2.annotations.Service;

import static io.opentracing.propagation.Format.Builtin.HTTP_HEADERS;
import static io.opentracing.tag.Tags.COMPONENT;
Expand Down Expand Up @@ -105,14 +109,14 @@ public class JaxwsContainerRequestTracingFilter implements MonitorFilter {

@Inject
private ServiceLocator serviceLocator;

@Inject
private RequestTracingService requestTracing;

@Inject
private OpenTracingService openTracing;


// Before method invocation
@Override
public void filterRequest(Packet pipeRequest, MonitorContext monitorContext) {
Expand All @@ -121,16 +125,16 @@ public void filterRequest(Packet pipeRequest, MonitorContext monitorContext) {

// Get the Traced annotation from the target method if CDI is initialised
Traced tracedAnnotation = getTraceAnnotation(monitorContext);

// If there is no matching skip pattern and no traced annotation, or if there is there is no matching skip
// pattern and a traced annotation set to true (via annotation or config override)
if (shouldTrace(pipeRequest) && shouldTrace(monitorContext, tracedAnnotation)) {

// Get the application's tracer instance
Tracer tracer = getTracer();

HttpServletRequest httpRequest = (HttpServletRequest) pipeRequest.get(SERVLET_REQUEST);

// Create a Span and instrument it with details about the request
SpanBuilder spanBuilder = tracer.buildSpan(determineOperationName(pipeRequest, monitorContext, tracedAnnotation))
.withTag(SPAN_KIND.getKey(), SPAN_KIND_SERVER)
Expand All @@ -156,7 +160,7 @@ public void filterRequest(Packet pipeRequest, MonitorContext monitorContext) {
}
}
}

// After method invocation
@Override
public void filterResponse(Packet pipeRequest, Packet pipeResponse, MonitorContext monitorContext) {
Expand All @@ -175,10 +179,10 @@ public void filterResponse(Packet pipeRequest, Packet pipeResponse, MonitorConte
if (activeScope == null) {
return;
}

// Get and add the response status to the active span
Response.StatusType statusInfo = getResponseStatus(pipeRequest, pipeRequest);

Span activeSpan = activeScope.span();
activeSpan.setTag(HTTP_STATUS.getKey(), statusInfo.getStatusCode());

Expand All @@ -188,7 +192,7 @@ public void filterResponse(Packet pipeRequest, Packet pipeResponse, MonitorConte
activeSpan.log(singletonMap("event", "error"));

// If there's an attached exception, add it to the span

Message message = pipeResponse.getMessage();
if (message != null && message.isFault()) {
activeSpan.log(singletonMap("error.object", getErrorObject(message)));
Expand All @@ -200,20 +204,20 @@ public void filterResponse(Packet pipeRequest, Packet pipeResponse, MonitorConte
} finally {
// If there's an attached error on the response, log the exception
Message message = pipeResponse.getMessage();

if (message != null && message.isFault()) {
Object errorObject = getErrorObject(message);

if (errorObject != null) {
// TODO: have an actual detail formatter for fault
logger.log(SEVERE, getErrorObject(message).toString());
}
}

}

}

private Object getErrorObject(Message message) {
try {
return message.copy().readAsSOAPMessage().getSOAPBody().getFault();
Expand All @@ -231,9 +235,9 @@ private Object getErrorObject(Message message) {
*/
private String determineOperationName(Packet pipeRequest, MonitorContext monitorContext, Traced tracedAnnotation) {
HttpServletRequest httpRequest = (HttpServletRequest) pipeRequest.get(SERVLET_REQUEST);

if (tracedAnnotation != null) {
String operationName = (String)
String operationName =
OpenTracingJaxwsCdiUtils
.getConfigOverrideValue(
Traced.class, "operationName", monitorContext, String.class)
Expand All @@ -248,7 +252,7 @@ private String determineOperationName(Packet pipeRequest, MonitorContext monitor

return operationName;
}

// If there is no @Traced annotation
Config config = getConfig();

Expand All @@ -263,7 +267,7 @@ private String determineOperationName(Packet pipeRequest, MonitorContext monitor

// If the provider is set to "http-path" and the class-level @WebService annotation is actually present
if (operationNameProvider.equals("http-path") && classLevelAnnotation != null) {

String operationName = httpRequest.getMethod() + ":";

operationName += "/" + classLevelAnnotation.name();
Expand All @@ -276,37 +280,36 @@ private String determineOperationName(Packet pipeRequest, MonitorContext monitor
return operationName;
}
}

// TODO: consider the WSDL info in MonitorContext?


// If we haven't returned by now, just go with the default ("class-method")
return createFallbackName(httpRequest, monitorContext);
}

private String createFallbackName(HttpServletRequest httpRequest, MonitorContext monitorContext) {
return
httpRequest.getMethod() + ":" +
return
httpRequest.getMethod() + ":" +
monitorContext.getImplementationClass().getCanonicalName() + "." +
monitorContext.getCallInfo().getMethod().getName();
}

private boolean shouldTrace(Packet pipeRequest) {
HttpServletRequest httpRequest = (HttpServletRequest) pipeRequest.get(SERVLET_REQUEST);

return shouldTrace(httpRequest.getContextPath());
}

private boolean shouldTrace(MonitorContext monitorContext, Traced tracedAnnotation) {
if (tracedAnnotation == null) {
// No annotation present means we trace
return true;
}

return (Boolean)
OpenTracingJaxwsCdiUtils
.getConfigOverrideValue(Traced.class, "value", monitorContext, boolean.class)
.orElse(tracedAnnotation.value());

return OpenTracingJaxwsCdiUtils
.getConfigOverrideValue(Traced.class, "value", monitorContext, boolean.class)
.orElse(tracedAnnotation.value());
}

/**
Expand Down Expand Up @@ -343,31 +346,31 @@ private boolean shouldTrace(String path) {

return true;
}

private boolean isTraceInProgress() {
return requestTracing != null && requestTracing.isRequestTracingEnabled() && requestTracing.isTraceInProgress();
}

private Response.StatusType getResponseStatus(Packet pipeRequest, Packet pipeResponse) {

Integer statusCode = (Integer) pipeResponse.get(HTTP_RESPONSE_CODE);

if (statusCode == null || statusCode.equals(0)) {
HttpServletResponse httpResponse = (HttpServletResponse) pipeRequest.get(SERVLET_RESPONSE);
statusCode = httpResponse.getStatus();
}

return Response.Status.fromStatusCode(statusCode);
}

private Throwable getResponseException(MonitorContext monitorContext, Packet pipeResponse) {
return monitorContext
.getSeiModel()
.getDatabinding()
.deserializeResponse(pipeResponse.copy(true), monitorContext.getCallInfo())
.getException();
}

private Traced getTraceAnnotation(MonitorContext monitorContext) {
// Check if CDI has been initialised by trying to get the BeanManager
BeanManager beanManager = getBeanManager();
Expand All @@ -376,44 +379,44 @@ private Traced getTraceAnnotation(MonitorContext monitorContext) {
if (beanManager != null) {
return OpenTracingJaxwsCdiUtils.getAnnotation(beanManager, Traced.class, monitorContext);
}

return null;
}

private BeanManager getBeanManager() {
try {
return CDI.current().getBeanManager();
} catch (IllegalStateException ise) {
// *Should* only get here if CDI hasn't been initialised, indicating that the app isn't using it
logger.log(FINE, "Error getting Bean Manager, presumably due to this application not using CDI",
logger.log(FINE, "Error getting Bean Manager, presumably due to this application not using CDI",
ise);
}

return null;
}

private Config getConfig() {
try {
return ConfigProvider.getConfig();
} catch (IllegalArgumentException ex) {
logger.log(INFO, "No config could be found", ex);
}

return null;
}

private Tracer getTracer() {
return openTracing.getTracer(openTracing.getApplicationName(serviceLocator.getService(InvocationManager.class)));
}

private MultivaluedMap<String, String> getHeaders(HttpServletRequest httpRequest) {

MultivaluedMap<String, String> headerMap = new MultivaluedHashMap<>();

for (String headerName : list(httpRequest.getHeaderNames())) {
headerMap.addAll(headerName, list(httpRequest.getHeaders(headerName)));
}

return headerMap;
}

Expand All @@ -426,7 +429,7 @@ private class MultivaluedMapToTextMap implements TextMap {

/**
* Initialises this object with the MultivaluedMap to wrap.
*
*
* @param map The MultivaluedMap to convert to a TextMap
*/
public MultivaluedMapToTextMap(MultivaluedMap<String, String> map) {
Expand All @@ -448,7 +451,7 @@ public void put(String key, String value) {

/**
* Helper Class used for iterating over the MultivaluedMapToTextMap class.
*
*
* @param <K> The map key class
* @param <V> The map value class
*/
Expand All @@ -461,7 +464,7 @@ private class MultivaluedMapIterator<K, V> implements Iterator<Map.Entry<K, V>>

/**
* Initialise the iterator to use on this map.
*
*
* @param multiValuesEntrySet The map to initialise the iterator from.
*/
public MultivaluedMapIterator(Set<Map.Entry<K, List<V>>> multiValuesEntrySet) {
Expand Down Expand Up @@ -496,5 +499,5 @@ public void remove() {
}

}

}

0 comments on commit 0512c51

Please sign in to comment.