From 242fb14eac82f65c537cac440c40f59c5ab3628b Mon Sep 17 00:00:00 2001 From: evernat Date: Tue, 15 May 2018 23:41:34 +0200 Subject: [PATCH] #76 When using Spring MVC, use path in RequestMapping annotation as http request name, in order to reduce the need of http-transform-pattern --- .../java/net/bull/javamelody/JdbcWrapper.java | 2 +- .../net/bull/javamelody/MonitoringFilter.java | 8 +++-- .../javamelody/internal/model/Counter.java | 34 +++++++++++++------ .../internal/model/CounterRequestContext.java | 34 ++++++++++++++----- .../javamelody/internal/web/RumInjector.java | 18 ++++++---- .../web/html/HtmlJobInformationsReport.java | 2 +- .../web/pdf/PdfJobInformationsReport.java | 2 +- 7 files changed, 70 insertions(+), 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 index 16a6e2806..840926dd5 100644 --- 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 index 78470aa2a..24662cdc2 100644 --- 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 index c8014f492..cb829a249 100644 --- 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 index 2e633a02c..6bffdcdb8 100644 --- 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 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 index 6d903968e..a22e1b5fd 100644 --- 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\n\n"; } + private String getHttpRequestName() { + return CounterRequestContext.getHttpRequestName(httpRequest, requestName); + } + @Override public String getBeforeTag() { // we suppose lowercase diff --git a/javamelody-core/src/main/java/net/bull/javamelody/internal/web/html/HtmlJobInformationsReport.java b/javamelody-core/src/main/java/net/bull/javamelody/internal/web/html/HtmlJobInformationsReport.java index bb3be605a..fe13d06c8 100644 --- a/javamelody-core/src/main/java/net/bull/javamelody/internal/web/html/HtmlJobInformationsReport.java +++ b/javamelody-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/javamelody-core/src/main/java/net/bull/javamelody/internal/web/pdf/PdfJobInformationsReport.java b/javamelody-core/src/main/java/net/bull/javamelody/internal/web/pdf/PdfJobInformationsReport.java index 72ee6b5dd..ac64a8c5a 100644 --- a/javamelody-core/src/main/java/net/bull/javamelody/internal/web/pdf/PdfJobInformationsReport.java +++ b/javamelody-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;