From 64b7648f0dfe0fe9dcfadfc239582ce8227fca80 Mon Sep 17 00:00:00 2001 From: Seoyoung Park Date: Wed, 12 Feb 2025 16:05:54 +0900 Subject: [PATCH] [#12021] Replace ErrorAttributes with Problem --- pom.xml | 7 ++ .../web/starter/multi/PinpointWebStarter.java | 4 +- .../src/main/resources/application.yml | 1 - web/pom.xml | 4 + .../pinpoint/web/ExceptionHandling.java | 73 +++++++++++ .../pinpoint/web/PinpointWebModule.java | 1 + .../pinpoint/web/ProblemSpringWebConfig.java | 61 ++++++++++ .../com/navercorp/pinpoint/web/WebApp.java | 2 + .../NonWhiteLabelErrorController.java | 114 ------------------ .../pinpoint/web/problem/ProblemWrapper.java | 85 +++++++++++++ .../view/error/PinpointErrorAttributes.java | 42 ------- .../web/view/error/PinpointErrorData.java | 97 --------------- web/src/main/resources/application.yml | 1 - 13 files changed, 236 insertions(+), 256 deletions(-) create mode 100644 web/src/main/java/com/navercorp/pinpoint/web/ExceptionHandling.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/ProblemWrapper.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/pom.xml b/pom.xml index d85dd323823b..32b6658f3c76 100644 --- a/pom.xml +++ b/pom.xml @@ -201,6 +201,8 @@ 4.0.0 3.0.2 + 0.29.1 + 2.0.16 2.24.2 @@ -764,6 +766,11 @@ spring-boot-starter-tomcat ${spring-boot.version} + + org.zalando + problem-spring-web-starter + ${problem-spring-web.version} + org.springframework.boot spring-boot-starter-validation 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..e2d632d1b4e0 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 @@ -38,6 +38,7 @@ import org.springframework.boot.autoconfigure.data.web.SpringDataWebAutoConfiguration; import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration; import org.springframework.boot.autoconfigure.transaction.TransactionAutoConfiguration; +import org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration; import org.springframework.context.annotation.Import; /** * @author minwoo.jung @@ -49,7 +50,8 @@ SpringDataWebAutoConfiguration.class, RedisAutoConfiguration.class, RedisRepositoriesAutoConfiguration.class, - RedisReactiveAutoConfiguration.class + RedisReactiveAutoConfiguration.class, + ErrorMvcAutoConfiguration.class, }) @Import({ PinpointWebModule.class, diff --git a/web-starter/src/main/resources/application.yml b/web-starter/src/main/resources/application.yml index 8894fe0befb6..de544bdfb66d 100644 --- a/web-starter/src/main/resources/application.yml +++ b/web-starter/src/main/resources/application.yml @@ -10,7 +10,6 @@ spring: server: port: 8080 error: - path: /api-public/error include-exception: true include-message: always include-binding-errors: always diff --git a/web/pom.xml b/web/pom.xml index 057b643a5a80..de9bf10c9ee8 100644 --- a/web/pom.xml +++ b/web/pom.xml @@ -201,6 +201,10 @@ org.springframework.boot spring-boot-starter-tomcat + + org.zalando + problem-spring-web-starter + diff --git a/web/src/main/java/com/navercorp/pinpoint/web/ExceptionHandling.java b/web/src/main/java/com/navercorp/pinpoint/web/ExceptionHandling.java new file mode 100644 index 000000000000..d8b3f0009932 --- /dev/null +++ b/web/src/main/java/com/navercorp/pinpoint/web/ExceptionHandling.java @@ -0,0 +1,73 @@ +/* + * 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 com.navercorp.pinpoint.web.problem.ProblemWrapper; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.context.request.NativeWebRequest; +import org.zalando.problem.Problem; +import org.zalando.problem.spring.web.advice.ProblemHandling; + +import java.net.InetAddress; +import java.net.UnknownHostException; + +/** + * @author intr3p1d + */ +@ControllerAdvice +final class ExceptionHandling implements ProblemHandling { + + private final String hostname; + + ExceptionHandling() { + this.hostname = getHostName(); + } + + static private String getHostName() { + try { + return InetAddress.getLocalHost().getHostName(); + } catch (UnknownHostException e) { + return "unknown"; + } + } + + @Override + public ResponseEntity process(ResponseEntity entity, NativeWebRequest request) { + Problem originalProblem = entity.getBody(); + if (originalProblem == null) { + return entity; + } + + HttpServletRequest httpRequest = request.getNativeRequest(HttpServletRequest.class); + HttpServletResponse httpResponse = request.getNativeResponse(HttpServletResponse.class); + + String path = (httpRequest != null) ? httpRequest.getRequestURI() : "unknown"; + String method = (httpRequest != null) ? httpRequest.getMethod() : "unknown"; + int statusCode = (httpResponse != null) ? httpResponse.getStatus() : entity.getStatusCode().value(); + + Problem modifiedProblem = new ProblemWrapper(originalProblem, hostname, path, method); + return ResponseEntity.status(statusCode).body(modifiedProblem); + } + + @Override + public boolean isCausalChainsEnabled() { + return true; + } + +} 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 37074c972af4..87eeaf3fb5df 100644 --- a/web/src/main/java/com/navercorp/pinpoint/web/PinpointWebModule.java +++ b/web/src/main/java/com/navercorp/pinpoint/web/PinpointWebModule.java @@ -35,6 +35,7 @@ WebServiceConfig.class, RealtimeConfig.class, MainDataSourceConfiguration.class, + ProblemSpringWebConfig.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..c17818803276 --- /dev/null +++ b/web/src/main/java/com/navercorp/pinpoint/web/ProblemSpringWebConfig.java @@ -0,0 +1,61 @@ +/* + * 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 com.navercorp.pinpoint.web.problem.ProblemWrapper; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.autoconfigure.jackson.Jackson2ObjectMapperBuilderCustomizer; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; +import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder; +import org.zalando.problem.Problem; +import org.zalando.problem.jackson.ProblemModule; +import org.zalando.problem.spring.web.advice.AdviceTrait; +import org.zalando.problem.violations.ConstraintViolationProblemModule; + +/** + * @author intr3p1d + */ +@Configuration +public class ProblemSpringWebConfig { + @Bean + public Jackson2ObjectMapperBuilderCustomizer addCustomBigDecimalDeserialization( + @Qualifier("pinpointWebProblemModule") ProblemModule problemModule, + @Qualifier("pinpointConstraintViolationProblemModule") ConstraintViolationProblemModule constraintViolationProblemModule + ) { + return (Jackson2ObjectMapperBuilder jacksonObjectMapperBuilder) -> + jacksonObjectMapperBuilder.modules( + problemModule, constraintViolationProblemModule + ).mixIn(Problem.class, ProblemWrapper.class); + } + + @Bean + public ProblemModule pinpointWebProblemModule() { + return new ProblemModule().withStackTraces(true); + } + + @Bean + public ConstraintViolationProblemModule pinpointConstraintViolationProblemModule() { + return new ConstraintViolationProblemModule(); + } + + @Bean + @Primary + public AdviceTrait pinpointExceptionHandling() { + return new ExceptionHandling(); + } +} diff --git a/web/src/main/java/com/navercorp/pinpoint/web/WebApp.java b/web/src/main/java/com/navercorp/pinpoint/web/WebApp.java index b7d1a0396325..1b77c9201b07 100644 --- a/web/src/main/java/com/navercorp/pinpoint/web/WebApp.java +++ b/web/src/main/java/com/navercorp/pinpoint/web/WebApp.java @@ -29,6 +29,7 @@ import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration; import org.springframework.boot.autoconfigure.sql.init.SqlInitializationAutoConfiguration; import org.springframework.boot.autoconfigure.transaction.TransactionAutoConfiguration; +import org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration; import org.springframework.context.annotation.Import; @SpringBootConfiguration @@ -40,6 +41,7 @@ RedisAutoConfiguration.class, RedisRepositoriesAutoConfiguration.class, RedisReactiveAutoConfiguration.class, + ErrorMvcAutoConfiguration.class, }) @Import({ PinpointWebModule.class, 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/ProblemWrapper.java b/web/src/main/java/com/navercorp/pinpoint/web/problem/ProblemWrapper.java new file mode 100644 index 000000000000..5faadf19de4d --- /dev/null +++ b/web/src/main/java/com/navercorp/pinpoint/web/problem/ProblemWrapper.java @@ -0,0 +1,85 @@ +/* + * 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 com.fasterxml.jackson.annotation.JsonUnwrapped; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.zalando.problem.Problem; +import org.zalando.problem.StatusType; + +import java.net.URI; +import java.util.Map; + +/** + * @author intr3p1d + */ +public class ProblemWrapper implements Problem { + + @JsonUnwrapped + private final Problem originalProblem; + private final String hostName; + private final String path; + private final String method; + + public ProblemWrapper(Problem originalProblem, String hostName, String path, String method) { + this.originalProblem = originalProblem; + this.hostName = hostName; + this.path = path; + this.method = method; + } + + @Override + public URI getType() { + return originalProblem.getType(); + } + + @Override + public @Nullable String getTitle() { + return originalProblem.getTitle(); + } + + @Override + public @Nullable StatusType getStatus() { + return originalProblem.getStatus(); + } + + @Override + public @Nullable String getDetail() { + return originalProblem.getDetail(); + } + + @Override + public @Nullable URI getInstance() { + return originalProblem.getInstance(); + } + + @Override + public Map getParameters() { + return originalProblem.getParameters(); + } + + public String getHostName() { + return hostName; + } + + public String getPath() { + return path; + } + + public String getMethod() { + return method; + } +} 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..9ad067414857 100644 --- a/web/src/main/resources/application.yml +++ b/web/src/main/resources/application.yml @@ -8,7 +8,6 @@ spring: server: port: 8080 error: - path: /api/error include-exception: true include-message: always include-binding-errors: always