#76 When using Spring MVC, use path in RequestMapping annotation as h…

…ttp request name, in order to reduce the need of http-transform-pattern
javamelody · May 15, 2018 · 242fb14 · 242fb14
1 parent 896a0ae
commit 242fb14
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 30 deletions.
diff --git a/javamelody-core/src/main/java/net/bull/javamelody/JdbcWrapper.java b/javamelody-core/src/main/java/net/bull/javamelody/JdbcWrapper.java
@@ -407,7 +407,7 @@ Object doExecute(String requestName, Statement statement, Method method, Object[
 
 			// note perf: selon un paramètre current-sql(/requests)-disabled,
 			// on pourrait ici ne pas binder un nouveau contexte à chaque requête sql
-			sqlCounter.bindContext(requestName, requestName, null, -1);
+			sqlCounter.bindContext(requestName, requestName, null, null, -1);
 
 			final Object result = method.invoke(statement, args);
 			systemError = false;

diff --git a/javamelody-core/src/main/java/net/bull/javamelody/MonitoringFilter.java b/javamelody-core/src/main/java/net/bull/javamelody/MonitoringFilter.java
@@ -44,6 +44,7 @@
 import net.bull.javamelody.internal.model.Collector;
 import net.bull.javamelody.internal.model.Counter;
 import net.bull.javamelody.internal.model.CounterError;
+import net.bull.javamelody.internal.model.CounterRequestContext;
 import net.bull.javamelody.internal.model.LabradorRetriever;
 import net.bull.javamelody.internal.model.ThreadInformations;
 import net.bull.javamelody.internal.web.CounterServletResponseWrapper;
@@ -229,8 +230,8 @@ private void doFilter(FilterChain chain, HttpServletRequest httpRequest,
 		try {
 			JdbcWrapper.ACTIVE_THREAD_COUNT.incrementAndGet();
 			// on binde le contexte de la requête http pour les requêtes sql
-			httpCounter.bindContext(requestName, completeRequestName, httpRequest.getRemoteUser(),
-					startCpuTime);
+			httpCounter.bindContext(requestName, completeRequestName, httpRequest,
+					httpRequest.getRemoteUser(), startCpuTime);
 			// on binde la requête http (utilisateur courant et requête complète) pour les derniers logs d'erreurs
 			httpRequest.setAttribute(CounterError.REQUEST_KEY, completeRequestName);
 			CounterError.bindRequest(httpRequest);
@@ -283,6 +284,9 @@ private void doFilter(FilterChain chain, HttpServletRequest httpRequest,
 							null);
 				}
 
+				// prise en compte de Spring bestMatchingPattern s'il y a
+				requestName = CounterRequestContext.getHttpRequestName(httpRequest, requestName);
+
 				// taille du flux sortant
 				final int responseSize = wrappedResponse.getDataLength();
 				// nom identifiant la requête

diff --git a/javamelody-core/src/main/java/net/bull/javamelody/internal/model/Counter.java b/javamelody-core/src/main/java/net/bull/javamelody/internal/model/Counter.java
@@ -30,6 +30,8 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import javax.servlet.http.HttpServletRequest;
+
 import net.bull.javamelody.internal.common.LOG;
 
 /**
@@ -412,17 +414,18 @@ long getEstimatedMemorySize() {
 	}
 
 	public void bindContextIncludingCpu(String requestName) {
-		bindContext(requestName, requestName, null, ThreadInformations.getCurrentThreadCpuTime());
+		bindContext(requestName, requestName, null, null,
+				ThreadInformations.getCurrentThreadCpuTime());
 	}
 
-	public void bindContext(String requestName, String completeRequestName, String remoteUser,
-			long startCpuTime) {
+	public void bindContext(String requestName, String completeRequestName,
+			HttpServletRequest httpRequest, String remoteUser, long startCpuTime) {
 		// requestName est la même chose que ce qui sera utilisée dans addRequest,
 		// completeRequestName est la même chose éventuellement complétée
 		// pour cette requête à destination de l'affichage dans les requêtes courantes
 		// (sinon mettre 2 fois la même chose)
 		final CounterRequestContext context = new CounterRequestContext(this,
-				contextThreadLocal.get(), requestName, completeRequestName, remoteUser,
+				contextThreadLocal.get(), requestName, completeRequestName, httpRequest, remoteUser,
 				startCpuTime);
 		contextThreadLocal.set(context);
 		if (context.getParentContext() == null) {
@@ -722,34 +725,43 @@ void removeRequest(String requestName) {
 	 * @return CounterRequest
 	 */
 	public CounterRequest getCounterRequest(CounterRequestContext context) {
-		return getCounterRequestByName(context.getRequestName());
+		return getCounterRequestByName(context.getRequestName(), false);
 	}
 
 	/**
 	 * Retourne l'objet {@link CounterRequest} correspondant au nom sans agrégation en paramètre.
 	 * @param requestName Nom de la requête sans agrégation par requestTransformPattern
+	 * @param saveRequestIfAbsent true except for current requests because the requestName may not be yet bestMatchingPattern
 	 * @return CounterRequest
 	 */
-	public CounterRequest getCounterRequestByName(String requestName) {
+	public CounterRequest getCounterRequestByName(String requestName, boolean saveRequestIfAbsent) {
 		// l'instance de CounterRequest retournée est clonée
 		// (nécessaire pour protéger la synchronisation interne du counter),
 		// son état peut donc être lu sans synchronisation
 		// mais toute modification de cet état ne sera pas conservée
 		final String aggregateRequestName = getAggregateRequestName(requestName);
-		final CounterRequest request = getCounterRequestInternal(aggregateRequestName);
+		final CounterRequest request = getCounterRequestInternal(aggregateRequestName,
+				saveRequestIfAbsent);
 		synchronized (request) {
 			return request.clone();
 		}
 	}
 
 	private CounterRequest getCounterRequestInternal(String requestName) {
+		return getCounterRequestInternal(requestName, true);
+	}
+
+	private CounterRequest getCounterRequestInternal(String requestName,
+			boolean saveRequestIfAbsent) {
 		CounterRequest request = requests.get(requestName);
 		if (request == null) {
 			request = new CounterRequest(requestName, getName());
-			// putIfAbsent a l'avantage d'être garanti atomique, même si ce n'est pas indispensable
-			final CounterRequest precedentRequest = requests.putIfAbsent(requestName, request);
-			if (precedentRequest != null) {
-				request = precedentRequest;
+			if (saveRequestIfAbsent) {
+				// putIfAbsent a l'avantage d'être garanti atomique, même si ce n'est pas indispensable
+				final CounterRequest precedentRequest = requests.putIfAbsent(requestName, request);
+				if (precedentRequest != null) {
+					request = precedentRequest;
+				}
 			}
 		}
 		return request;

diff --git a/javamelody-core/src/main/java/net/bull/javamelody/internal/model/CounterRequestContext.java b/javamelody-core/src/main/java/net/bull/javamelody/internal/model/CounterRequestContext.java
@@ -25,6 +25,8 @@
 import java.util.List;
 import java.util.Map;
 
+import javax.servlet.http.HttpServletRequest;
+
 import net.bull.javamelody.internal.model.CounterRequest.ICounterRequestContext;
 
 /**
@@ -36,12 +38,14 @@
 public class CounterRequestContext implements ICounterRequestContext, Cloneable, Serializable {
 	private static final long serialVersionUID = 1L;
 	private static final Long ONE = 1L;
+	private static final String SPRING_BEST_MATCHING_PATTERN_ATTRIBUTE = "org.springframework.web.servlet.HandlerMapping.bestMatchingPattern";
 	// attention de ne pas sérialiser le counter d'origine vers le serveur de collecte, le vrai ayant été cloné
 	private Counter parentCounter;
 	private final CounterRequestContext parentContext;
 	private CounterRequestContext currentChildContext;
 	private final String requestName;
 	private final String completeRequestName;
+	private final transient HttpServletRequest httpRequest;
 	private final String remoteUser;
 	private final long threadId;
 	// attention, si sérialisation vers serveur de collecte, la durée peut être impactée s'il y a désynchronisation d'horloge
@@ -54,9 +58,11 @@ public class CounterRequestContext implements ICounterRequestContext, Cloneable,
 	private Map<String, Long> childRequestsExecutionsByRequestId;
 
 	public CounterRequestContext(Counter parentCounter, CounterRequestContext parentContext,
-			String requestName, String completeRequestName, String remoteUser, long startCpuTime) {
-		this(parentCounter, parentContext, requestName, completeRequestName, remoteUser,
-				Thread.currentThread().getId(), System.currentTimeMillis(), startCpuTime);
+			String requestName, String completeRequestName, HttpServletRequest httpRequest,
+			String remoteUser, long startCpuTime) {
+		this(parentCounter, parentContext, requestName, completeRequestName, httpRequest,
+				remoteUser, Thread.currentThread().getId(), System.currentTimeMillis(),
+				startCpuTime);
 		if (parentContext != null) {
 			parentContext.setCurrentChildContext(this);
 		}
@@ -65,8 +71,8 @@ public CounterRequestContext(Counter parentCounter, CounterRequestContext parent
 	// constructeur privé pour la méthode clone
 	// CHECKSTYLE:OFF
 	private CounterRequestContext(Counter parentCounter, CounterRequestContext parentContext,
-			String requestName, String completeRequestName, String remoteUser, long threadId,
-			long startTime, long startCpuTime) {
+			String requestName, String completeRequestName, HttpServletRequest httpRequest,
+			String remoteUser, long threadId, long startTime, long startCpuTime) {
 		// CHECKSTYLE:ON
 		super();
 		assert parentCounter != null;
@@ -78,6 +84,7 @@ private CounterRequestContext(Counter parentCounter, CounterRequestContext paren
 		this.parentContext = parentContext;
 		this.requestName = requestName;
 		this.completeRequestName = completeRequestName;
+		this.httpRequest = httpRequest;
 		this.remoteUser = remoteUser;
 		this.threadId = threadId;
 		this.startTime = startTime;
@@ -124,10 +131,21 @@ public CounterRequestContext getParentContext() {
 		return parentContext;
 	}
 
-	public String getRequestName() {
+	public static String getHttpRequestName(HttpServletRequest httpRequest, String requestName) {
+		if (httpRequest != null) {
+			final String bestMatchingPattern = (String) httpRequest
+					.getAttribute(SPRING_BEST_MATCHING_PATTERN_ATTRIBUTE);
+			if (bestMatchingPattern != null) {
+				return bestMatchingPattern;
+			}
+		}
 		return requestName;
 	}
 
+	public String getRequestName() {
+		return getHttpRequestName(httpRequest, requestName);
+	}
+
 	public String getCompleteRequestName() {
 		return completeRequestName;
 	}
@@ -291,8 +309,8 @@ private CounterRequestContext clone(CounterRequestContext parentContextClone) {
 		//		final Counter parentCounterClone = new Counter(counter.getName(), counter.getStorageName(),
 		//				counter.getIconName(), counter.getChildCounterName(), null);
 		final CounterRequestContext clone = new CounterRequestContext(counter, parentContextClone,
-				getRequestName(), getCompleteRequestName(), getRemoteUser(), getThreadId(),
-				startTime, startCpuTime);
+				getRequestName(), getCompleteRequestName(), httpRequest, getRemoteUser(),
+				getThreadId(), startTime, startCpuTime);
 		clone.childHits = getChildHits();
 		clone.childDurationsSum = getChildDurationsSum();
 		final CounterRequestContext childContext = getCurrentChildContext();

diff --git a/javamelody-core/src/main/java/net/bull/javamelody/internal/web/RumInjector.java b/javamelody-core/src/main/java/net/bull/javamelody/internal/web/RumInjector.java
@@ -22,6 +22,7 @@
 
 import net.bull.javamelody.internal.common.Parameters;
 import net.bull.javamelody.internal.model.Counter;
+import net.bull.javamelody.internal.model.CounterRequestContext;
 import net.bull.javamelody.internal.web.HtmlInjectorResponseStream.HtmlToInject;
 
 /**
@@ -34,20 +35,19 @@ public final class RumInjector implements HtmlToInject {
 	private static final String BOOMERANG_FILENAME = "boomerang.min.js";
 
 	private final long start = System.currentTimeMillis();
-	private final String rumUrl;
+	private final HttpServletRequest httpRequest;
 	private final String requestName;
 
-	private RumInjector(String rumUrl, String requestName) {
+	private RumInjector(HttpServletRequest httpRequest, String requestName) {
 		super();
-		this.rumUrl = rumUrl;
+		this.httpRequest = httpRequest;
 		this.requestName = requestName;
 	}
 
 	public static HttpServletResponse createRumResponseWrapper(HttpServletRequest httpRequest,
 			HttpServletResponse httpResponse, String requestName) {
 		if (HtmlInjectorServletResponseWrapper.acceptsRequest(httpRequest)) {
-			final String rumUrl = getRumUrlForBrowser(requestName);
-			final HtmlToInject htmlToInject = new RumInjector(rumUrl, requestName);
+			final HtmlToInject htmlToInject = new RumInjector(httpRequest, requestName);
 			return new HtmlInjectorServletResponseWrapper(httpRequest, httpResponse, htmlToInject);
 		}
 		return httpResponse;
@@ -85,14 +85,20 @@ private static String getRumUrlForBrowser(String requestName) {
 
 	@Override
 	public String getContent() {
+		final String httpRequestName = getHttpRequestName();
+		final String rumUrl = getRumUrlForBrowser(httpRequestName);
 		// approximation of server duration (may not be the real server duration, but not far in general)
 		final long serverTime = System.currentTimeMillis() - start;
 		return "\n<script src='" + rumUrl + "?resource=" + BOOMERANG_FILENAME
 				+ "'></script>\n<script>BOOMR.init({beacon_url: '" + rumUrl
-				+ "?part=rum', log: null});\nBOOMR.addVar('requestName', \"" + requestName
+				+ "?part=rum', log: null});\nBOOMR.addVar('requestName', \"" + httpRequestName
 				+ "\");\n" + "BOOMR.addVar('serverTime', " + serverTime + ");\n</script>\n";
 	}
 
+	private String getHttpRequestName() {
+		return CounterRequestContext.getHttpRequestName(httpRequest, requestName);
+	}
+
 	@Override
 	public String getBeforeTag() {
 		// we suppose lowercase

diff --git a/...y-core/src/main/java/net/bull/javamelody/internal/web/html/HtmlJobInformationsReport.java b/...y-core/src/main/java/net/bull/javamelody/internal/web/html/HtmlJobInformationsReport.java
@@ -223,7 +223,7 @@ private CounterRequest getCounterRequest(JobInformations jobInformations) {
 		final String jobFullName = jobInformations.getGroup() + '.' + jobInformations.getName();
 		// rq: la méthode getCounterRequestByName prend en compte l'éventuelle utilisation du paramètre
 		// job-transform-pattern qui peut faire que jobFullName != counterRequest.getName()
-		final CounterRequest result = jobCounter.getCounterRequestByName(jobFullName);
+		final CounterRequest result = jobCounter.getCounterRequestByName(jobFullName, true);
 		// getCounterRequestByName ne peut pas retourner null actuellement
 		assert result != null;
 		return result;

diff --git a/...ody-core/src/main/java/net/bull/javamelody/internal/web/pdf/PdfJobInformationsReport.java b/...ody-core/src/main/java/net/bull/javamelody/internal/web/pdf/PdfJobInformationsReport.java
@@ -183,7 +183,7 @@ private CounterRequest getCounterRequest(JobInformations jobInformations) {
 		final String jobFullName = jobInformations.getGroup() + '.' + jobInformations.getName();
 		// rq: la méthode getCounterRequestByName prend en compte l'éventuelle utilisation du paramètre
 		// job-transform-pattern qui peut faire que jobFullName != counterRequest.getName()
-		final CounterRequest result = jobCounter.getCounterRequestByName(jobFullName);
+		final CounterRequest result = jobCounter.getCounterRequestByName(jobFullName, true);
 		// getCounterRequestByName ne peut pas retourner null actuellement
 		assert result != null;
 		return result;