This is an automated email from the ASF dual-hosted git repository. rickyma pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push: new db357cabc [#1927] fix(dashboard): Fix NPE issues in dashboard (#1965) db357cabc is described below commit db357cabc4d393dbb3f82832789095672b92112f Author: yl09099 <33595968+yl09...@users.noreply.github.com> AuthorDate: Wed Jul 31 17:14:16 2024 +0800 [#1927] fix(dashboard): Fix NPE issues in dashboard (#1965) ### What changes were proposed in this pull request? 1. Modify NPE bugs; 2. Modify the Vue3's CompositionAPI. 3. Handling the request failed. 4. Optimize page display. ![image](https://github.com/user-attachments/assets/b0395f00-3a00-4092-91c6-e7fb121dca57) ![image](https://github.com/user-attachments/assets/068d5b09-cf4b-43ec-a706-61e51a2371b9) ### Why are the changes needed? Fix: #1954 ### Does this PR introduce _any_ user-facing change? Yes. ### How was this patch tested? UT. --- .../dashboard/web/proxy/WebProxyServlet.java | 69 ++++++----- .../dashboard/web/utils/DashboardUtils.java | 13 +- dashboard/src/main/webapp/src/api/api.js | 102 ++++------------ .../webapp/src/pages/CoordinatorServerPage.vue | 88 +++++++++----- .../webapp/src/pages/serverstatus/NodeListPage.vue | 133 +++++++++++++++++---- dashboard/src/main/webapp/src/utils/http.js | 15 ++- 6 files changed, 257 insertions(+), 163 deletions(-) diff --git a/dashboard/src/main/java/org/apache/uniffle/dashboard/web/proxy/WebProxyServlet.java b/dashboard/src/main/java/org/apache/uniffle/dashboard/web/proxy/WebProxyServlet.java index 084478030..15d0772e7 100644 --- a/dashboard/src/main/java/org/apache/uniffle/dashboard/web/proxy/WebProxyServlet.java +++ b/dashboard/src/main/java/org/apache/uniffle/dashboard/web/proxy/WebProxyServlet.java @@ -22,8 +22,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import com.google.common.base.Preconditions; -import org.eclipse.jetty.client.api.Request; -import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.proxy.ProxyServlet; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,7 +30,14 @@ import org.slf4j.LoggerFactory; public class WebProxyServlet extends ProxyServlet { private static final Logger LOG = LoggerFactory.getLogger(WebProxyServlet.class); + /** The key of the request header. */ + private static final String HEADER_TARGET_ADDRESS = "targetAddress"; + private static final String HEADER_REQUEST_SERVER_TYPE = "requestServerType"; + /** The value of the request header. */ + private static final String REQUEST_SERVER_TYPE_COORDINATOR = "coordinator"; + + private static final String REQUEST_SERVER_TYPE_SERVER = "server"; private Map<String, String> coordinatorServerAddressesMap; public WebProxyServlet(Map<String, String> coordinatorServerAddressesMap) { @@ -46,47 +52,56 @@ public class WebProxyServlet extends ProxyServlet { return null; } String targetAddress; - if (clientRequest.getHeader("serverType").equals("coordinator")) { - targetAddress = coordinatorServerAddressesMap.get(clientRequest.getHeader("targetAddress")); - if (targetAddress == null) { - // Get random one from coordinatorServerAddressesMap - targetAddress = coordinatorServerAddressesMap.values().iterator().next(); - } + String requestServerType = + clientRequest.getHeader(HEADER_REQUEST_SERVER_TYPE) != null + && REQUEST_SERVER_TYPE_COORDINATOR.equalsIgnoreCase( + clientRequest.getHeader(HEADER_REQUEST_SERVER_TYPE)) + ? REQUEST_SERVER_TYPE_COORDINATOR + : REQUEST_SERVER_TYPE_SERVER; + if (requestServerType.equalsIgnoreCase(REQUEST_SERVER_TYPE_COORDINATOR)) { + targetAddress = + coordinatorServerAddressesMap.get(clientRequest.getHeader(HEADER_TARGET_ADDRESS)); } else { - targetAddress = clientRequest.getHeader("targetAddress"); + targetAddress = clientRequest.getHeader(HEADER_TARGET_ADDRESS); } StringBuilder target = new StringBuilder(); - if (targetAddress.endsWith("/")) { - targetAddress = targetAddress.substring(0, targetAddress.length() - 1); - } target.append(targetAddress).append("/api").append(clientRequest.getPathInfo()); String query = clientRequest.getQueryString(); if (query != null) { target.append("?").append(query); } + LOG.info(target.toString()); return target.toString(); } - @Override - protected void onProxyRewriteFailed( - HttpServletRequest clientRequest, HttpServletResponse clientResponse) {} - + /** + * If the proxy address fails to be requested, 403 is returned and the front-end handles the + * exception. + * + * @param clientRequest + * @param proxyResponse + * @param serverResponse + * @param failure + */ @Override protected void onProxyResponseFailure( HttpServletRequest clientRequest, HttpServletResponse proxyResponse, - Response serverResponse, - Throwable failure) {} - - @Override - protected String filterServerResponseHeader( - HttpServletRequest clientRequest, - Response serverResponse, - String headerName, - String headerValue) { - return null; + org.eclipse.jetty.client.api.Response serverResponse, + Throwable failure) { + sendProxyResponseError(clientRequest, proxyResponse, HttpStatus.FORBIDDEN_403); } + /** + * If the proxy address fails to be rewritten, 403 is returned and the front-end handles the + * exception. + * + * @param clientRequest the client request + * @param proxyResponse the client response + */ @Override - protected void addXForwardedHeaders(HttpServletRequest clientRequest, Request proxyRequest) {} + protected void onProxyRewriteFailed( + HttpServletRequest clientRequest, HttpServletResponse proxyResponse) { + sendProxyResponseError(clientRequest, proxyResponse, HttpStatus.FORBIDDEN_403); + } } diff --git a/dashboard/src/main/java/org/apache/uniffle/dashboard/web/utils/DashboardUtils.java b/dashboard/src/main/java/org/apache/uniffle/dashboard/web/utils/DashboardUtils.java index 1b60a6b27..72a5b446f 100644 --- a/dashboard/src/main/java/org/apache/uniffle/dashboard/web/utils/DashboardUtils.java +++ b/dashboard/src/main/java/org/apache/uniffle/dashboard/web/utils/DashboardUtils.java @@ -24,6 +24,7 @@ import java.util.Map; import com.google.common.base.Preconditions; import com.google.common.collect.Maps; +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,11 +38,21 @@ public class DashboardUtils { for (String coordinator : coordinators) { try { URL coordinatorURL = new URL(coordinator); - coordinatorAddressMap.put(coordinatorURL.getHost(), coordinator); + coordinatorAddressMap.put(coordinatorURL.getHost(), removeBackslash(coordinator)); } catch (MalformedURLException e) { LOG.error("The coordinator address is abnormal.", e); } } + LOG.info("Monitored the Coordinator of {}.", String.join(",", coordinatorAddressMap.values())); return coordinatorAddressMap; } + + /** Remove the trailing backslash. */ + public static String removeBackslash(String addressStr) { + if (StringUtils.isNotEmpty(addressStr) && addressStr.endsWith("/")) { + return addressStr.substring(0, addressStr.length() - 1); + } else { + return addressStr; + } + } } diff --git a/dashboard/src/main/webapp/src/api/api.js b/dashboard/src/main/webapp/src/api/api.js index b837658ce..928be9afb 100644 --- a/dashboard/src/main/webapp/src/api/api.js +++ b/dashboard/src/main/webapp/src/api/api.js @@ -26,82 +26,32 @@ export function getCoordinatorConf(params, headers) { return http.get('/coordinator/conf', params, headers, 0) } -export async function getShuffleServerConf(address, params, headers) { - if (typeof headers === 'undefined') { - headers = {}; - } - headers.targetAddress = address; - const response = await http.get('/shuffleServer/conf', params, headers, 0); - const newWindow = window.open('', '_blank'); - let tableHTML = ` - <style> - table { - width: 100%; - } - th, td { - padding: 0 20px; - text-align: left; - } - </style> - <table> - <tr> - <th>Key</th> - <th>Value</th> - </tr> - `; - for (const item of response.data.data) { - tableHTML += `<tr><td>${item.argumentKey}</td><td>${item.argumentValue}</td></tr>`; - } - tableHTML += '</table>'; - newWindow.document.write(tableHTML); -} - -export async function getCoordinatorMetrics(params, headers) { - const response = await http.get('/coordinator/metrics', params, headers, 0) - const newWindow = window.open('', '_blank'); - newWindow.document.write('<pre>' + JSON.stringify(response.data, null, 2) + '</pre>'); -} - -export async function getShuffleServerMetrics(address, params, headers) { - if (typeof headers === 'undefined') { - headers = {} - } - headers.targetAddress = address - const response = await http.get('/shuffleServer/metrics', params, headers, 0) - const newWindow = window.open('', '_blank'); - newWindow.document.write('<pre>' + JSON.stringify(response.data, null, 2) + '</pre>'); -} - -export async function getCoordinatorPrometheusMetrics(params, headers) { - const response = await http.get('/coordinator/prometheus/metrics/all', params, headers, 0) - const newWindow = window.open('', '_blank'); - newWindow.document.write('<pre>' + response.data + '</pre>'); -} - -export async function getShuffleServerPrometheusMetrics(address, params, headers) { - if (typeof headers === 'undefined') { - headers = {} - } - headers.targetAddress = address - const response = await http.get('/shuffleServer/prometheus/metrics/all', params, headers, 0) - const newWindow = window.open('', '_blank'); - newWindow.document.write('<pre>' + response.data + '</pre>'); -} - -export async function getCoordinatorStacks(params, headers) { - const response = await http.get('/coordinator/stacks', params, headers, 0) - const newWindow = window.open('', '_blank'); - newWindow.document.write('<pre>' + response.data + '</pre>'); -} - -export async function getShuffleServerStacks(address, params, headers) { - if (typeof headers === 'undefined') { - headers = {} - } - headers.targetAddress = address - const response = await http.get('/shuffleServer/stacks', params, headers, 0) - const newWindow = window.open('', '_blank'); - newWindow.document.write('<pre>' + response.data + '</pre>'); +export function getShuffleServerConf(params, headers) { + return http.get('/shuffleServer/conf', params, headers, 0) +} + +export function getCoordinatorMetrics(params, headers) { + return http.get('/coordinator/metrics', params, headers, 0) +} + +export function getShuffleServerMetrics(params, headers) { + return http.get('/shuffleServer/metrics', params, headers, 0) +} + +export function getCoordinatorPrometheusMetrics(params, headers) { + return http.get('/coordinator/prometheus/metrics/all', params, headers, 0) +} + +export function getShuffleServerPrometheusMetrics(params, headers) { + return http.get('/shuffleServer/prometheus/metrics/all', params, headers, 0) +} + +export function getCoordinatorStacks(params, headers) { + return http.get('/coordinator/stacks', params, headers, 0) +} + +export function getShuffleServerStacks(params, headers) { + return http.get('/shuffleServer/stacks', params, headers, 0) } // Create an interface for the total number of nodes diff --git a/dashboard/src/main/webapp/src/pages/CoordinatorServerPage.vue b/dashboard/src/main/webapp/src/pages/CoordinatorServerPage.vue index aac7cf317..4e9efbd2a 100644 --- a/dashboard/src/main/webapp/src/pages/CoordinatorServerPage.vue +++ b/dashboard/src/main/webapp/src/pages/CoordinatorServerPage.vue @@ -65,6 +65,21 @@ </template> {{ pageData.serverInfo.serverWebPort }} </el-descriptions-item> + <el-descriptions-item> + <template #label> + <div class="cell-item"> + <el-icon :style="iconStyle"> + <Wallet /> + </el-icon> + Monitoring Information + </div> + </template> + <div class="mb-4"> + <el-button type="primary" @click="handlerMetrics">Metrics</el-button> + <el-button type="success" @click="handlerPromMetrics">Prometheus Metrics</el-button> + <el-button type="info" @click="handlerStacks">Stacks</el-button> + </div> + </el-descriptions-item> </el-descriptions> </div> </el-collapse-item> @@ -74,36 +89,13 @@ <el-table-column prop="argumentValue" label="Value" min-width="380" /> </el-table> </el-collapse-item> - <el-collapse-item title="Coordinator Metrics" name="3"> - <el-link @click="getCoordinatorMetrics" target="_blank"> - <el-icon :style="iconStyle"> - <Link /> - </el-icon> - metrics - </el-link> - </el-collapse-item> - <el-collapse-item title="Coordinator Prometheus Metrics" name="4"> - <el-link @click="getCoordinatorPrometheusMetrics" target="_blank"> - <el-icon :style="iconStyle"> - <Link /> - </el-icon> - prometheus metrics - </el-link> - </el-collapse-item> - <el-collapse-item title="Coordinator Stacks" name="5"> - <el-link @click="getCoordinatorStacks" target="_blank"> - <el-icon :style="iconStyle"> - <Link /> - </el-icon> - stacks - </el-link> - </el-collapse-item> </el-collapse> </div> </template> <script> import { ref, reactive, computed, onMounted } from 'vue' +import { ElMessage } from 'element-plus' import { getCoordinatorConf, getCoordinatorMetrics, @@ -114,10 +106,9 @@ import { import { useCurrentServerStore } from '@/store/useCurrentServerStore' export default { - methods: {getCoordinatorMetrics, getCoordinatorPrometheusMetrics, getCoordinatorStacks}, setup() { const pageData = reactive({ - activeNames: ['1', '2', '3', '4', '5'], + activeNames: ['1', '2'], tableData: [], serverInfo: {} }) @@ -132,6 +123,46 @@ export default { pageData.serverInfo = res.data.data } + async function handlerMetrics() { + try { + const response = await getCoordinatorMetrics() + if (response.status >= 200 && response.status < 300) { + const newWindow = window.open('', '_blank') + newWindow.document.write('<pre>' + JSON.stringify(response.data, null, 2) + '</pre>') + } else { + ElMessage.error('Request failed.') + } + } catch (err) { + ElMessage.error('Internal error.') + } + } + async function handlerPromMetrics() { + try { + const response = await getCoordinatorPrometheusMetrics() + if (response.status >= 200 && response.status < 300) { + const newWindow = window.open('', '_blank') + newWindow.document.write('<pre>' + response.data + '</pre>') + } else { + ElMessage.error('Request failed.') + } + } catch (err) { + ElMessage.error('Internal error.') + } + } + async function handlerStacks() { + try { + const response = await getCoordinatorStacks() + if (response.status >= 200 && response.status < 300) { + const newWindow = window.open('', '_blank') + newWindow.document.write('<pre>' + response.data + '</pre>') + } else { + ElMessage.error('Request failed.') + } + } catch (err) { + ElMessage.error('Internal error.') + } + } + /** * The system obtains data from global variables and requests the interface to obtain new data after data changes. */ @@ -176,7 +207,10 @@ export default { pageData, iconStyle, blockMargin, - size + size, + handlerMetrics, + handlerPromMetrics, + handlerStacks } } } diff --git a/dashboard/src/main/webapp/src/pages/serverstatus/NodeListPage.vue b/dashboard/src/main/webapp/src/pages/serverstatus/NodeListPage.vue index 41c7a19c4..bc6e87ef4 100644 --- a/dashboard/src/main/webapp/src/pages/serverstatus/NodeListPage.vue +++ b/dashboard/src/main/webapp/src/pages/serverstatus/NodeListPage.vue @@ -68,42 +68,32 @@ /> <el-table-column label="Conf"> <template v-slot="{ row }"> - <el-link @click="getShuffleServerConf('http://' + row.ip + ':' + row.jettyPort)" target="_blank"> - <el-icon :style="iconStyle"> - <Link /> - </el-icon> - conf - </el-link> + <div class="mb-4"> + <el-button type="warning" @click="handlerServerConf(row)">conf</el-button> + </div> </template> </el-table-column> <el-table-column label="Metrics"> <template v-slot="{ row }"> - <el-link @click="getShuffleServerMetrics('http://' + row.ip + ':' + row.jettyPort)" target="_blank"> - <el-icon :style="iconStyle"> - <Link /> - </el-icon> - metrics - </el-link> + <div class="mb-4"> + <el-button type="primary" @click="handlerServerMetrics(row)">metrics</el-button> + </div> </template> </el-table-column> <el-table-column label="PrometheusMetrics" min-width="150"> <template v-slot="{ row }"> - <el-link @click="getShuffleServerPrometheusMetrics('http://' + row.ip + ':' + row.jettyPort)" target="_blank"> - <el-icon :style="iconStyle"> - <Link /> - </el-icon> - prometheus metrics - </el-link> + <div class="mb-4"> + <el-button type="success" @click="handlerServerPrometheusMetrics(row)" + >prometheus metrics</el-button + > + </div> </template> </el-table-column> <el-table-column label="Stacks"> <template v-slot="{ row }"> - <el-link @click="getShuffleServerStacks('http://' + row.ip + ':' + row.jettyPort)" target="_blank"> - <el-icon :style="iconStyle"> - <Link /> - </el-icon> - stacks - </el-link> + <div class="mb-4"> + <el-button type="info" @click="handlerServerStacks(row)">stacks</el-button> + </div> </template> </el-table-column> <el-table-column prop="tags" label="Tags" min-width="140" /> @@ -137,7 +127,6 @@ import { } from '@/api/api' export default { - methods: {getShuffleServerConf, getShuffleServerMetrics, getShuffleServerPrometheusMetrics, getShuffleServerStacks}, setup() { const router = useRouter() const currentServerStore = useCurrentServerStore() @@ -165,7 +154,7 @@ export default { async function deleteLostServer(row) { try { const params = { serverId: row.id } - const res = await deleteConfirmedLostServer(params, {}) + const res = await deleteConfirmedLostServer(params) // Invoke the interface to delete the lost server, prompting a message based on the result returned. if (res.data.data === 'success') { ElMessage.success('remove ' + row.id + ' sucess...') @@ -202,6 +191,94 @@ export default { listPageData.tableData = res.data.data } + function combinedRequestAddress(serverRow) { + return 'http://' + serverRow.ip + ':' + serverRow.jettyPort + } + + async function handlerServerConf(serverRow) { + try { + const headers = {} + headers.targetAddress = combinedRequestAddress(serverRow) + const response = await getShuffleServerConf({}, headers) + if (response.status >= 200 && response.status < 300) { + const newWindow = window.open('', '_blank') + let tableHTML = ` + <style> + table { + width: 100%; + } + th, td { + padding: 0 20px; + text-align: left; + } + </style> + <table> + <tr> + <th>Key</th> + <th>Value</th> + </tr> + ` + for (const item of response.data.data) { + tableHTML += `<tr><td>${item.argumentKey}</td><td>${item.argumentValue}</td></tr>` + } + tableHTML += '</table>' + newWindow.document.write(tableHTML) + } else { + ElMessage.error('Request failed.') + } + } catch (err) { + ElMessage.error('Internal error.') + } + } + + async function handlerServerMetrics(serverRow) { + try { + const headers = {} + headers.targetAddress = combinedRequestAddress(serverRow) + const response = await getShuffleServerMetrics({}, headers) + if (response.status >= 200 && response.status < 300) { + const newWindow = window.open('', '_blank') + newWindow.document.write('<pre>' + JSON.stringify(response.data, null, 2) + '</pre>') + } else { + ElMessage.error('Request failed.') + } + } catch (err) { + ElMessage.error('Internal error.') + } + } + + async function handlerServerPrometheusMetrics(serverRow) { + try { + const headers = {} + headers.targetAddress = combinedRequestAddress(serverRow) + const response = await getShuffleServerPrometheusMetrics({}, headers) + if (response.status >= 200 && response.status < 300) { + const newWindow = window.open('', '_blank') + newWindow.document.write('<pre>' + response.data + '</pre>') + } else { + ElMessage.error('Request failed.') + } + } catch (err) { + ElMessage.error('Internal error.') + } + } + + async function handlerServerStacks(serverRow) { + try { + const headers = {} + headers.targetAddress = combinedRequestAddress(serverRow) + const response = await getShuffleServerStacks({}, headers) + if (response.status >= 200 && response.status < 300) { + const newWindow = window.open('', '_blank') + newWindow.document.write('<pre>' + response.data + '</pre>') + } else { + ElMessage.error('Request failed.') + } + } catch (err) { + ElMessage.error('Internal error.') + } + } + const loadPageData = () => { isShowRemove.value = false listPageData.tableData = [ @@ -281,6 +358,10 @@ export default { isShowRemove, showDeleteConfirm, sortChangeEvent, + handlerServerConf, + handlerServerPrometheusMetrics, + handlerServerMetrics, + handlerServerStacks, memFormatter, dateFormatter } diff --git a/dashboard/src/main/webapp/src/utils/http.js b/dashboard/src/main/webapp/src/utils/http.js index 40efd1ed8..48ca73668 100644 --- a/dashboard/src/main/webapp/src/utils/http.js +++ b/dashboard/src/main/webapp/src/utils/http.js @@ -18,17 +18,18 @@ import request from '@/utils/request' import { useCurrentServerStore } from '@/store/useCurrentServerStore' +const requestServerType = { coordinator: 'coordinator', server: 'server' } const http = { get(url, params, headers, fontBackFlag) { if (fontBackFlag === 0) { + // The system obtains the address of the Coordinator to be accessed from global variables. + const currentServerStore = useCurrentServerStore() if (headers) { - headers.serverType = 'server' + headers.requestServerType = requestServerType.server } else { - // The system obtains the address of the Coordinator to be accessed from global variables. - const currentServerStore = useCurrentServerStore() headers = {} + headers.requestServerType = requestServerType.coordinator headers.targetAddress = currentServerStore.currentServer - headers.serverType = 'coordinator' } return request.getBackEndAxiosInstance().get(url, { params, headers }) } else { @@ -40,9 +41,10 @@ const http = { // The system obtains the address of the Coordinator to be accessed from global variables. const currentServerStore = useCurrentServerStore() if (headers) { - headers.targetAddress = currentServerStore.currentServer + headers.requestServerType = requestServerType.server } else { headers = {} + headers.requestServerType = requestServerType.coordinator headers.targetAddress = currentServerStore.currentServer } return request.getBackEndAxiosInstance().post(url, data, headers) @@ -55,9 +57,10 @@ const http = { // The system obtains the address of the Coordinator to be accessed from global variables. const currentServerStore = useCurrentServerStore() if (headers) { - headers.targetAddress = currentServerStore.currentServer + headers.requestServerType = requestServerType.server } else { headers = {} + headers.requestServerType = requestServerType.coordinator headers.targetAddress = currentServerStore.currentServer } return request.getBackEndAxiosInstance().delete(url, { params, headers })