From e5ec5b40f69fe0e4ba5c9a59a0b573e2d99c8367 Mon Sep 17 00:00:00 2001 From: Seoyoung Park Date: Fri, 21 Feb 2025 19:38:14 +0900 Subject: [PATCH] [#12021] Replace ErrorAttributes with ProblemDetail --- .../web/starter/multi/PinpointWebStarter.java | 2 +- .../src/main/resources/application.yml | 5 +- .../pinpoint/web/CustomExceptionHandler.java | 111 +++++++++++++++++ .../pinpoint/web/PinpointWebModule.java | 1 + .../pinpoint/web/ProblemSpringWebConfig.java | 34 ++++++ .../NonWhiteLabelErrorController.java | 114 ------------------ .../web/problem/StackTraceProcessor.java | 33 +++++ .../view/error/PinpointErrorAttributes.java | 42 ------- .../web/view/error/PinpointErrorData.java | 97 --------------- web/src/main/resources/application.yml | 3 + 10 files changed, 187 insertions(+), 255 deletions(-) create mode 100644 web/src/main/java/com/navercorp/pinpoint/web/CustomExceptionHandler.java create mode 100644 web/src/main/java/com/navercorp/pinpoint/web/ProblemSpringWebConfig.java delete mode 100644 web/src/main/java/com/navercorp/pinpoint/web/controller/NonWhiteLabelErrorController.java create mode 100644 web/src/main/java/com/navercorp/pinpoint/web/problem/StackTraceProcessor.java delete mode 100644 web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorAttributes.java delete mode 100644 web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorData.java diff --git a/web-starter/src/main/java/com/navercorp/pinpoint/web/starter/multi/PinpointWebStarter.java b/web-starter/src/main/java/com/navercorp/pinpoint/web/starter/multi/PinpointWebStarter.java index 385afd6c2d4b..2646a0991fe9 100644 --- a/web-starter/src/main/java/com/navercorp/pinpoint/web/starter/multi/PinpointWebStarter.java +++ b/web-starter/src/main/java/com/navercorp/pinpoint/web/starter/multi/PinpointWebStarter.java @@ -49,7 +49,7 @@ SpringDataWebAutoConfiguration.class, RedisAutoConfiguration.class, RedisRepositoriesAutoConfiguration.class, - RedisReactiveAutoConfiguration.class + RedisReactiveAutoConfiguration.class, }) @Import({ PinpointWebModule.class, diff --git a/web-starter/src/main/resources/application.yml b/web-starter/src/main/resources/application.yml index 8894fe0befb6..65ce4835950a 100644 --- a/web-starter/src/main/resources/application.yml +++ b/web-starter/src/main/resources/application.yml @@ -6,6 +6,9 @@ spring: default-view-inclusion: true profiles: active: release, metric + mvc: + problemdetails: + enabled: true server: port: 8080 @@ -30,7 +33,7 @@ pinpoint: enabled: true otlpmetric: enabled: true - + pinpoint.web.websocket: async-send-timeout: max-session-idle-timeout: 10800000 # 3 hours diff --git a/web/src/main/java/com/navercorp/pinpoint/web/CustomExceptionHandler.java b/web/src/main/java/com/navercorp/pinpoint/web/CustomExceptionHandler.java new file mode 100644 index 000000000000..b40db0137578 --- /dev/null +++ b/web/src/main/java/com/navercorp/pinpoint/web/CustomExceptionHandler.java @@ -0,0 +1,111 @@ +/* + * Copyright 2025 NAVER Corp. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.navercorp.pinpoint.web; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.ProblemDetail; +import org.springframework.http.ResponseEntity; +import org.springframework.lang.Nullable; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.context.request.WebRequest; +import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; +import com.fasterxml.jackson.databind.ser.std.ToStringSerializer; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.Collection; + +import static com.navercorp.pinpoint.web.problem.StackTraceProcessor.COMPOUND; +import static java.util.Arrays.asList; + +/** + * @author intr3p1d + */ +@ControllerAdvice +final class CustomExceptionHandler extends ResponseEntityExceptionHandler { + + private static final ToStringSerializer serializer = new ToStringSerializer(); + private final String hostname; + + CustomExceptionHandler() { + this.hostname = getHostName(); + } + + static private String getHostName() { + try { + return InetAddress.getLocalHost().getHostName(); + } catch (UnknownHostException e) { + return "unknown"; + } + } + + @ExceptionHandler({Exception.class}) + public ResponseEntity handleGeneralException( + Exception ex, + WebRequest request + ) { + HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR; + ProblemDetail problemDetail = ProblemDetail.forStatus(status.value()); + problemDetail.setTitle("Internal Server Error"); + problemDetail.setDetail(ex.getMessage()); + addProperties(problemDetail, request); + addStackTraces(problemDetail, ex); + + return new ResponseEntity<>(problemDetail, status); + } + + @Override + protected ResponseEntity handleExceptionInternal( + Exception ex, + @Nullable Object body, + HttpHeaders headers, + HttpStatusCode statusCode, + WebRequest request + ) { + ResponseEntity response = super.handleExceptionInternal(ex, body, headers, statusCode, request); + + if (response.getBody() instanceof ProblemDetail problemDetail) { + addProperties(problemDetail, request); + addStackTraces(problemDetail, ex); + return this.createResponseEntity(problemDetail, headers, statusCode, request); + } + return response; + } + + public void addProperties(ProblemDetail problemDetail, WebRequest request) { + if (request instanceof ServletWebRequest servletWebRequest) { + problemDetail.setProperty("method", servletWebRequest.getRequest().getMethod()); + } else { + problemDetail.setProperty("method", "UNKNOWN"); + } + problemDetail.setProperty("parameters", request.getParameterMap()); + problemDetail.setProperty("hostname", hostname); + } + + public void addStackTraces(ProblemDetail problemDetail, Throwable th) { + final Collection stackTrace = COMPOUND.process(asList(th.getStackTrace())); + String[] trace = traceToStringArray(stackTrace); + problemDetail.setProperty("trace", trace); + } + + private String[] traceToStringArray(Collection stackTrace) { + return stackTrace.stream().map(serializer::valueToString).toArray(String[]::new); + } +} diff --git a/web/src/main/java/com/navercorp/pinpoint/web/PinpointWebModule.java b/web/src/main/java/com/navercorp/pinpoint/web/PinpointWebModule.java index e454c0d35295..983b7ad14382 100644 --- a/web/src/main/java/com/navercorp/pinpoint/web/PinpointWebModule.java +++ b/web/src/main/java/com/navercorp/pinpoint/web/PinpointWebModule.java @@ -36,6 +36,7 @@ WebServiceConfig.class, RealtimeConfig.class, MainDataSourceConfiguration.class, + ProblemSpringWebConfig.class, WebPinpointIdCacheConfiguration.class, CacheConfiguration.class, diff --git a/web/src/main/java/com/navercorp/pinpoint/web/ProblemSpringWebConfig.java b/web/src/main/java/com/navercorp/pinpoint/web/ProblemSpringWebConfig.java new file mode 100644 index 000000000000..e29403c73117 --- /dev/null +++ b/web/src/main/java/com/navercorp/pinpoint/web/ProblemSpringWebConfig.java @@ -0,0 +1,34 @@ +/* + * Copyright 2025 NAVER Corp. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.navercorp.pinpoint.web; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; +import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + +/** + * @author intr3p1d + */ +@Configuration +public class ProblemSpringWebConfig { + + @Bean + @Primary + public ResponseEntityExceptionHandler pinpointExceptionHandling() { + return new CustomExceptionHandler(); + } +} diff --git a/web/src/main/java/com/navercorp/pinpoint/web/controller/NonWhiteLabelErrorController.java b/web/src/main/java/com/navercorp/pinpoint/web/controller/NonWhiteLabelErrorController.java deleted file mode 100644 index 7690038c86eb..000000000000 --- a/web/src/main/java/com/navercorp/pinpoint/web/controller/NonWhiteLabelErrorController.java +++ /dev/null @@ -1,114 +0,0 @@ -/* - * Copyright 2022 NAVER Corp. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.navercorp.pinpoint.web.controller; - -import jakarta.servlet.http.HttpServletRequest; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.web.ErrorProperties; -import org.springframework.boot.autoconfigure.web.ServerProperties; -import org.springframework.boot.autoconfigure.web.servlet.error.AbstractErrorController; -import org.springframework.boot.web.error.ErrorAttributeOptions; -import org.springframework.boot.web.servlet.error.ErrorAttributes; -import org.springframework.http.HttpStatus; -import org.springframework.http.ProblemDetail; -import org.springframework.validation.annotation.Validated; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RestController; - -import java.net.URI; -import java.net.URISyntaxException; - -@RestController -@RequestMapping(value={"/api-public/error"}) -@Validated -public class NonWhiteLabelErrorController extends AbstractErrorController { - private final ErrorProperties errorProperties; - - public NonWhiteLabelErrorController( - @Autowired ErrorAttributes errorAttributes, - @Autowired ServerProperties serverProperties - ) { - super(errorAttributes); - this.errorProperties = serverProperties.getError(); - } - - @RequestMapping - public ProblemDetail error(HttpServletRequest request) { - final HttpStatus status = this.getStatus(request); - ProblemDetail ret = ProblemDetail.forStatus(status.value()); - Object uri = request.getAttribute("jakarta.servlet.error.request_uri"); - if (uri != null) { - try { - ret.setInstance(new URI(uri.toString())); - } catch (URISyntaxException e) { - ret.setInstance(null); - } - } - - ret.setProperties(this.getErrorAttributes(request, this.getErrorAttributeOptions(request))); - return ret; - } - - private ErrorAttributeOptions getErrorAttributeOptions(HttpServletRequest request) { - ErrorAttributeOptions options = ErrorAttributeOptions.defaults(); - if (this.errorProperties.isIncludeException()) { - options = options.including(ErrorAttributeOptions.Include.EXCEPTION); - } - - if (this.isIncludeStackTrace(request)) { - options = options.including(ErrorAttributeOptions.Include.STACK_TRACE); - } - - if (this.isIncludeMessage(request)) { - options = options.including(ErrorAttributeOptions.Include.MESSAGE); - } - - if (this.isIncludeBindingErrors(request)) { - options = options.including(ErrorAttributeOptions.Include.BINDING_ERRORS); - } - - return options; - } - - private boolean isIncludeStackTrace(HttpServletRequest request) { - return switch (this.getErrorProperties().getIncludeStacktrace()) { - case ALWAYS -> true; - case ON_PARAM -> this.getTraceParameter(request); - default -> false; - }; - } - - private boolean isIncludeMessage(HttpServletRequest request) { - return switch (this.getErrorProperties().getIncludeMessage()) { - case ALWAYS -> true; - case ON_PARAM -> this.getMessageParameter(request); - default -> false; - }; - } - - private boolean isIncludeBindingErrors(HttpServletRequest request) { - return switch (this.getErrorProperties().getIncludeBindingErrors()) { - case ALWAYS -> true; - case ON_PARAM -> this.getErrorsParameter(request); - default -> false; - }; - } - - private ErrorProperties getErrorProperties() { - return this.errorProperties; - } -} diff --git a/web/src/main/java/com/navercorp/pinpoint/web/problem/StackTraceProcessor.java b/web/src/main/java/com/navercorp/pinpoint/web/problem/StackTraceProcessor.java new file mode 100644 index 000000000000..ca228b60e626 --- /dev/null +++ b/web/src/main/java/com/navercorp/pinpoint/web/problem/StackTraceProcessor.java @@ -0,0 +1,33 @@ +/* + * Copyright 2025 NAVER Corp. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.navercorp.pinpoint.web.problem; + +import static java.util.ServiceLoader.load; +import static java.util.stream.StreamSupport.stream; + +import java.util.Collection; +/** + * ... + */ +public interface StackTraceProcessor { + + StackTraceProcessor DEFAULT = elements -> elements; + StackTraceProcessor COMPOUND = stream(load(StackTraceProcessor.class).spliterator(), false) + .reduce(DEFAULT, (first, second) -> elements -> second.process(first.process(elements))); + + Collection process(final Collection elements); + +} diff --git a/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorAttributes.java b/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorAttributes.java deleted file mode 100644 index 8a67540bf50f..000000000000 --- a/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorAttributes.java +++ /dev/null @@ -1,42 +0,0 @@ -package com.navercorp.pinpoint.web.view.error; - -import org.apache.commons.lang3.SystemUtils; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.web.error.ErrorAttributeOptions; -import org.springframework.boot.web.servlet.error.DefaultErrorAttributes; -import org.springframework.stereotype.Component; -import org.springframework.web.context.request.WebRequest; - -import java.util.Map; - -@Component -public class PinpointErrorAttributes extends DefaultErrorAttributes { - private final String hostname; - - @Value("${server.error.include-cookies:true}") - private boolean includeCookies; - - public PinpointErrorAttributes() { - this.hostname = SystemUtils.getHostName(); - } - - @Override - public Map getErrorAttributes(WebRequest webRequest, ErrorAttributeOptions options) { - Map errorAttributes = super.getErrorAttributes(webRequest, options); - this.removeDuplicateData(errorAttributes); - this.addCustomData(webRequest, errorAttributes); - return errorAttributes; - } - - // removes attributes already present with ProblemDetail - private void removeDuplicateData(Map errorAttributes) { - errorAttributes.remove("status"); - errorAttributes.remove("error"); // ProblemDetail already has "title" field - errorAttributes.remove("path"); // ProblemDetail already has "instance" field - } - - private void addCustomData(WebRequest webRequest, Map errorAttributes) { - PinpointErrorData pinpointErrorData = new PinpointErrorData(this.hostname, webRequest, includeCookies); - errorAttributes.put("data", pinpointErrorData); - } -} \ No newline at end of file diff --git a/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorData.java b/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorData.java deleted file mode 100644 index 82af70000ab4..000000000000 --- a/web/src/main/java/com/navercorp/pinpoint/web/view/error/PinpointErrorData.java +++ /dev/null @@ -1,97 +0,0 @@ -package com.navercorp.pinpoint.web.view.error; - -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonInclude; -import org.springframework.web.context.request.ServletWebRequest; -import org.springframework.web.context.request.WebRequest; - -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; - -@JsonInclude(JsonInclude.Include.NON_NULL) -public class PinpointErrorData { - private final String hostName; - private final RequestInfo requestInfo; - - public PinpointErrorData(String hostName, WebRequest request, boolean includeCookies) { - this.hostName = hostName; - this.requestInfo = new RequestInfo(request, includeCookies); - } - - public String getHostName() { - return hostName; - } - - public RequestInfo getRequestInfo() { - return requestInfo; - } - - public static class RequestInfo { - private static final String UNKNOWN = "UNKNOWN"; - private final String method; - private final Map> headers; - private final Map parameters; - - @JsonIgnore - private boolean includeCookies = true; - - public RequestInfo(WebRequest request, boolean includeCookies) { - this.includeCookies = includeCookies; - if (request instanceof ServletWebRequest webRequest) { - this.method = webRequest.getRequest().getMethod(); - this.headers = getRequestHeader(webRequest); - this.parameters = request.getParameterMap(); - } else { - this.method = UNKNOWN; - this.headers = null; - this.parameters = null; - } - } - - public String getMethod() { - return method; - } - - public Map> getHeaders() { - return headers; - } - - public Map getParameters() { - return parameters; - } - - private Map> getRequestHeader(ServletWebRequest webRequest) { - Iterator keys = webRequest.getHeaderNames(); - if (keys == null) { - return Collections.emptyMap(); - } - - Map> result = new HashMap<>(); - while (keys.hasNext()) { - String key = keys.next(); - if (key == null) { - continue; - } - if (key.equals("cookie") && !includeCookies) { - continue; - } - result.put(key, List.of(webRequest.getHeaderValues(key))); - } - - return result; - - } - - @Override - public String toString() { - return "RequestInfo{" + - "method='" + method + '\'' + - ", headers=" + headers + - ", parameters=" + parameters + - '}'; - } - } -} \ No newline at end of file diff --git a/web/src/main/resources/application.yml b/web/src/main/resources/application.yml index d41b6e240974..a1f8ea15d210 100644 --- a/web/src/main/resources/application.yml +++ b/web/src/main/resources/application.yml @@ -4,6 +4,9 @@ spring: jackson: mapper: default-view-inclusion: true + mvc: + problemdetails: + enabled: true server: port: 8080