Skip to content

Commit 28a5c30

Browse files
rstoyanchevrstoyanchev
committed
Improve DEBUG/TRACE logging for Spring MVC
Issue: SPR-16898
1 parent 003d643 commit 28a5c30

File tree

69 files changed

+584
-601
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+584
-601
lines changed

‎spring-web/src/main/java/org/springframework/http/CacheControl.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,4 +304,8 @@ private void appendDirective(StringBuilder builder, String value) {
304304
builder.append(value);
305305
}
306306

307+
@Override
308+
public String toString() {
309+
return "CacheControl [" + getHeaderValue() + "]";
310+
}
307311
}

‎spring-web/src/main/java/org/springframework/http/HttpStatus.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ public Series series() {
504504
*/
505505
@Override
506506
public String toString() {
507-
return Integer.toString(this.value);
507+
return Integer.toString(this.value) + " " + name();
508508
}
509509

510510

‎spring-web/src/main/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategy.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,6 @@ public List<MediaType> resolveMediaTypeKey(NativeWebRequest webRequest, @Nullabl
138138
* {@link #lookupMediaType}.
139139
*/
140140
protected void handleMatch(String key, MediaType mediaType) {
141-
if (logger.isTraceEnabled()) {
142-
logger.trace("Requested MediaType='" + mediaType + "' based on key='" + key + "'.");
143-
}
144141
}
145142

146143
/**

‎spring-web/src/main/java/org/springframework/web/accept/FixedContentNegotiationStrategy.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@ public List<MediaType> getContentTypes() {
6969

7070
@Override
7171
public List<MediaType> resolveMediaTypes(NativeWebRequest request) {
72-
if (logger.isDebugEnabled()) {
73-
logger.debug("Requested media types: " + this.contentTypes);
74-
}
7572
return this.contentTypes;
7673
}
7774

‎spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ public void setUseJaf(boolean useJaf) {
8787
protected String getMediaTypeKey(NativeWebRequest webRequest) {
8888
HttpServletRequest request = webRequest.getNativeRequest(HttpServletRequest.class);
8989
if (request == null) {
90-
logger.warn("An HttpServletRequest is required to determine the media type key");
9190
return null;
9291
}
9392
String path = this.urlPathHelper.getLookupPathForRequest(request);

‎spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,13 @@ public BindingResult getBindingResult() {
6161

6262
@Override
6363
public String getMessage() {
64-
StringBuilder sb = new StringBuilder("Validation failed for argument at index ")
65-
.append(this.parameter.getParameterIndex()).append(" in method: ")
66-
.append(this.parameter.getExecutable().toGenericString())
67-
.append(", with ").append(this.bindingResult.getErrorCount()).append(" error(s): ");
64+
StringBuilder sb = new StringBuilder("Validation failed for argument [")
65+
.append(this.parameter.getParameterIndex()).append("] in ")
66+
.append(this.parameter.getExecutable().toGenericString());
67+
if (this.bindingResult.getErrorCount() > 1) {
68+
sb.append(" with ").append(this.bindingResult.getErrorCount()).append(" errors");
69+
}
70+
sb.append(": ");
6871
for (ObjectError error : this.bindingResult.getAllErrors()) {
6972
sb.append("[").append(error).append("] ");
7073
}

‎spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ public Object applyPostProcess(NativeWebRequest request, Callable<?> task, Objec
7272
try {
7373
this.interceptors.get(i).postProcess(request, task, concurrentResult);
7474
}
75-
catch (Throwable t) {
75+
catch (Throwable ex) {
7676
// Save the first exception but invoke all interceptors
7777
if (exceptionResult != null) {
78-
logger.error("postProcess error", t);
78+
logger.warn("Unhandled error from interceptor postProcess method", ex);
7979
}
8080
else {
81-
exceptionResult = t;
81+
exceptionResult = ex;
8282
}
8383
}
8484
}
@@ -140,8 +140,8 @@ public void triggerAfterCompletion(NativeWebRequest request, Callable<?> task) {
140140
try {
141141
this.interceptors.get(i).afterCompletion(request, task);
142142
}
143-
catch (Throwable t) {
144-
logger.error("afterCompletion error", t);
143+
catch (Throwable ex) {
144+
logger.error("Unhandled error from interceptor afterCompletion method", ex);
145145
}
146146
}
147147
}

‎spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ public Object applyPostProcess(NativeWebRequest request, DeferredResult<?> defe
6565
this.interceptors.get(i).postProcess(request, deferredResult, concurrentResult);
6666
}
6767
}
68-
catch (Throwable t) {
69-
return t;
68+
catch (Throwable ex) {
69+
return ex;
7070
}
7171
return concurrentResult;
7272
}
@@ -105,8 +105,8 @@ public void triggerAfterCompletion(NativeWebRequest request, DeferredResult<?> d
105105
try {
106106
this.interceptors.get(i).afterCompletion(request, deferredResult);
107107
}
108-
catch (Throwable t) {
109-
logger.error("afterCompletion error", t);
108+
catch (Throwable ex) {
109+
logger.error("Unhandled error from interceptor afterCompletion method", ex);
110110
}
111111
}
112112
}

‎spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -290,15 +290,15 @@ public void startCallableProcessing(final WebAsyncTask<?> webAsyncTask, Object..
290290
final CallableInterceptorChain interceptorChain = new CallableInterceptorChain(interceptors);
291291

292292
this.asyncWebRequest.addTimeoutHandler(() -> {
293-
logger.debug("Processing timeout");
293+
logger.debug("Async request timeout for " + formatRequestUri());
294294
Object result = interceptorChain.triggerAfterTimeout(this.asyncWebRequest, callable);
295295
if (result != CallableProcessingInterceptor.RESULT_NONE) {
296296
setConcurrentResultAndDispatch(result);
297297
}
298298
});
299299

300300
this.asyncWebRequest.addErrorHandler(ex -> {
301-
logger.debug("Processing error");
301+
logger.debug("Async request error for " + formatRequestUri() + ": " + ex);
302302
Object result = interceptorChain.triggerAfterError(this.asyncWebRequest, callable, ex);
303303
result = (result != CallableProcessingInterceptor.RESULT_NONE ? result : ex);
304304
setConcurrentResultAndDispatch(result);
@@ -333,6 +333,11 @@ public void startCallableProcessing(final WebAsyncTask<?> webAsyncTask, Object..
333333
}
334334
}
335335

336+
private String formatRequestUri() {
337+
HttpServletRequest request = this.asyncWebRequest.getNativeRequest(HttpServletRequest.class);
338+
return request != null ? urlPathHelper.getRequestUri(request) : "servlet container";
339+
}
340+
336341
private void setConcurrentResultAndDispatch(Object result) {
337342
synchronized (WebAsyncManager.this) {
338343
if (this.concurrentResult != RESULT_NONE) {
@@ -347,8 +352,7 @@ private void setConcurrentResultAndDispatch(Object result) {
347352
}
348353

349354
if (logger.isDebugEnabled()) {
350-
logger.debug("Concurrent result value [" + this.concurrentResult +
351-
"] - dispatching request to resume processing");
355+
logger.debug("Async result ready, dispatch to " + formatRequestUri());
352356
}
353357
this.asyncWebRequest.dispatch();
354358
}
@@ -432,11 +436,7 @@ private void startAsyncProcessing(Object[] processingContext) {
432436
this.asyncWebRequest.startAsync();
433437

434438
if (logger.isDebugEnabled()) {
435-
HttpServletRequest request = this.asyncWebRequest.getNativeRequest(HttpServletRequest.class);
436-
if (request != null) {
437-
String requestUri = urlPathHelper.getRequestUri(request);
438-
logger.debug("Concurrent handling starting for " + request.getMethod() + " [" + requestUri + "]");
439-
}
439+
logger.debug("Started async request");
440440
}
441441
}
442442

‎spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,13 @@ public boolean processRequest(@Nullable CorsConfiguration config, HttpServletReq
6868

6969
ServletServerHttpResponse serverResponse = new ServletServerHttpResponse(response);
7070
if (responseHasCors(serverResponse)) {
71-
logger.debug("Skip CORS processing: response already contains \"Access-Control-Allow-Origin\" header");
71+
logger.trace("Skip CORS processing: response already contains \"Access-Control-Allow-Origin\" header");
7272
return true;
7373
}
7474

7575
ServletServerHttpRequest serverRequest = new ServletServerHttpRequest(request);
7676
if (WebUtils.isSameOrigin(serverRequest)) {
77-
logger.debug("Skip CORS processing: request is from same origin");
77+
logger.trace("Skip CORS processing: request is from same origin");
7878
return true;
7979
}
8080

‎spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.springframework.lang.Nullable;
2727
import org.springframework.util.ClassUtils;
2828
import org.springframework.util.ReflectionUtils;
29+
import org.springframework.util.StringUtils;
2930
import org.springframework.web.bind.WebDataBinder;
3031
import org.springframework.web.bind.support.SessionStatus;
3132
import org.springframework.web.bind.support.WebDataBinderFactory;
@@ -163,24 +164,26 @@ private Object[] getMethodArgumentValues(NativeWebRequest request, @Nullable Mod
163164
continue;
164165
}
165166
catch (Exception ex) {
166-
if (logger.isDebugEnabled()) {
167-
logger.debug(getArgumentResolutionErrorMessage("Failed to resolve", i), ex);
167+
// Leave stack trace for later, e.g. AbstractHandlerExceptionResolver
168+
String message = ex.getMessage();
169+
if (!message.contains(parameter.getExecutable().toGenericString())) {
170+
if (logger.isDebugEnabled()) {
171+
logger.debug(formatArgumentError(parameter, message));
172+
}
168173
}
169174
throw ex;
170175
}
171176
}
172177
if (args[i] == null) {
173-
throw new IllegalStateException("Could not resolve method parameter at index " +
174-
parameter.getParameterIndex() + " in " + parameter.getExecutable().toGenericString() +
175-
": " + getArgumentResolutionErrorMessage("No suitable resolver for", i));
178+
throw new IllegalStateException(formatArgumentError(parameter, "No suitable resolver"));
176179
}
177180
}
178181
return args;
179182
}
180183

181-
private String getArgumentResolutionErrorMessage(String text, int index) {
182-
Class<?> paramType = getMethodParameters()[index].getParameterType();
183-
return text + " argument " + index + " of type '" + paramType.getName() + "'";
184+
private static String formatArgumentError(MethodParameter param, String message) {
185+
return "Could not resolve parameter [" + param.getParameterIndex() + "] in " +
186+
param.getExecutable().toGenericString() + (StringUtils.hasText(message) ? ": " + message : "");
184187
}
185188

186189
/**

‎spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsFileUploadSupport.java

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -281,10 +281,15 @@ protected MultipartParsingResult parseFileItems(List<FileItem> fileItems, String
281281
// multipart file field
282282
CommonsMultipartFile file = createMultipartFile(fileItem);
283283
multipartFiles.add(file.getName(), file);
284-
if (logger.isDebugEnabled()) {
285-
logger.debug("Found multipart file [" + file.getName() + "] of size " + file.getSize() +
286-
" bytes with original filename [" + file.getOriginalFilename() + "], stored " +
287-
file.getStorageDescription());
284+
if (logger.isDebugEnabled() || logger.isTraceEnabled()) {
285+
String message = "Part '" + file.getName() + "', " +
286+
"size " + file.getSize() + " bytes, filename='" + file.getOriginalFilename() + "'";
287+
if (logger.isTraceEnabled()) {
288+
logger.trace(message + ", storage=" + file.getStorageDescription());
289+
}
290+
else {
291+
logger.debug(message);
292+
}
288293
}
289294
}
290295
}
@@ -318,9 +323,15 @@ protected void cleanupFileItems(MultiValueMap<String, MultipartFile> multipartFi
318323
if (file instanceof CommonsMultipartFile) {
319324
CommonsMultipartFile cmf = (CommonsMultipartFile) file;
320325
cmf.getFileItem().delete();
321-
if (logger.isDebugEnabled()) {
322-
logger.debug("Cleaning up multipart file [" + cmf.getName() + "] with original filename [" +
323-
cmf.getOriginalFilename() + "], stored " + cmf.getStorageDescription());
326+
if (logger.isDebugEnabled() || logger.isTraceEnabled()) {
327+
String filename = cmf.getOriginalFilename();
328+
String message = "Cleaning up part '" + cmf.getName() + "', filename '" + filename + "'";
329+
if (logger.isTraceEnabled()) {
330+
logger.trace(message + ", stored " + cmf.getStorageDescription());
331+
}
332+
else {
333+
logger.debug(message);
334+
}
324335
}
325336
}
326337
}

‎spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsMultipartFile.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,14 +165,19 @@ public void transferTo(File dest) throws IOException, IllegalStateException {
165165

166166
try {
167167
this.fileItem.write(dest);
168-
if (logger.isDebugEnabled()) {
168+
if (logger.isDebugEnabled() || logger.isTraceEnabled()) {
169169
String action = "transferred";
170170
if (!this.fileItem.isInMemory()) {
171171
action = (isAvailable() ? "copied" : "moved");
172172
}
173-
logger.debug("Multipart file '" + getName() + "' with original filename [" +
174-
getOriginalFilename() + "], stored " + getStorageDescription() + ": " +
175-
action + " to [" + dest.getAbsolutePath() + "]");
173+
String message = "Part '" + getName() + "', filename '" + getOriginalFilename() + "'";
174+
if (logger.isTraceEnabled()) {
175+
logger.trace(message + ", stored " + getStorageDescription() + ": " + action +
176+
" to [" + dest.getAbsolutePath() + "]");
177+
}
178+
else {
179+
logger.debug(message + ": " + action + " to [" + dest.getAbsolutePath() + "]");
180+
}
176181
}
177182
}
178183
catch (FileUploadException ex) {

‎spring-web/src/main/java/org/springframework/web/multipart/support/MultipartFilter.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,15 @@ protected void doFilterInternal(
106106

107107
HttpServletRequest processedRequest = request;
108108
if (multipartResolver.isMultipart(processedRequest)) {
109-
if (logger.isDebugEnabled()) {
110-
logger.debug("Resolving multipart request [" + processedRequest.getRequestURI() +
111-
"] with MultipartFilter");
109+
if (logger.isTraceEnabled()) {
110+
logger.trace("Resolving multipart request");
112111
}
113112
processedRequest = multipartResolver.resolveMultipart(processedRequest);
114113
}
115114
else {
116115
// A regular request...
117-
if (logger.isDebugEnabled()) {
118-
logger.debug("Request [" + processedRequest.getRequestURI() + "] is not a multipart request");
116+
if (logger.isTraceEnabled()) {
117+
logger.trace("Not a multipart request");
119118
}
120119
}
121120

‎spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public void cannotResolveArg() throws Exception {
9393
fail("Expected exception");
9494
}
9595
catch (IllegalStateException ex) {
96-
assertTrue(ex.getMessage().contains("No suitable resolver for argument 0 of type 'java.lang.Integer'"));
96+
assertTrue(ex.getMessage().contains("Could not resolve parameter [0]"));
9797
}
9898
}
9999

0 commit comments

Comments
 (0)