You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ya...@apache.org on 2023/08/28 05:52:17 UTC
[spark] branch master updated: [SPARK-44960][UI] Unescape and consist error summary across UI pages
This is an automated email from the ASF dual-hosted git repository.
yao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new b7788f9b552 [SPARK-44960][UI] Unescape and consist error summary across UI pages
b7788f9b552 is described below
commit b7788f9b552dbcef723fd35a744f332c73b2d196
Author: Kent Yao <ya...@apache.org>
AuthorDate: Mon Aug 28 13:52:03 2023 +0800
[SPARK-44960][UI] Unescape and consist error summary across UI pages
### What changes were proposed in this pull request?
This pull request eliminates the unnecessary use of escape for error summary cells.
Previously, all the error details and some of the error summaries, such as the Task list on the stage page, did not call escapeHtml4. The rest of the error summaries, such as Job list, Stage list, Query list, etc., will do an extra escapeHtml4 that is unnecessary.
### Why are the changes needed?
UI improvement
### Does this PR introduce _any_ user-facing change?
UI changes
### How was this patch tested?
#### new tests
#### Local test
##### Before
##### After
### Was this patch authored or co-authored using generative AI tooling?
no
Closes #42674 from yaooqinn/SPARK-44960.
Authored-by: Kent Yao <ya...@apache.org>
Signed-off-by: Kent Yao <ya...@apache.org>
---
.../sql/connect/ui/SparkConnectServerPage.scala | 18 +--------------
.../main/scala/org/apache/spark/ui/UIUtils.scala | 6 ++---
.../sql/streaming/ui/StreamingQueryPage.scala | 17 ++++----------
.../spark/sql/execution/ui/UISeleniumSuite.scala | 27 +++++++++++++++++++++-
.../org/apache/spark/streaming/ui/UIUtils.scala | 11 +--------
5 files changed, 35 insertions(+), 44 deletions(-)
diff --git a/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/ui/SparkConnectServerPage.scala b/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/ui/SparkConnectServerPage.scala
index 57535cc06a2..bc92014b12b 100644
--- a/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/ui/SparkConnectServerPage.scala
+++ b/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/ui/SparkConnectServerPage.scala
@@ -23,8 +23,6 @@ import javax.servlet.http.HttpServletRequest
import scala.xml.Node
-import org.apache.commons.text.StringEscapeUtils
-
import org.apache.spark.internal.Logging
import org.apache.spark.sql.connect.ui.ToolTips._
import org.apache.spark.ui._
@@ -330,24 +328,10 @@ private[ui] class SqlStatsPagedTable(
<td>
{sqlStatsTableRow.sparkSessionTags.mkString(", ")}
</td>
- {errorMessageCell(Option(info.detail))}
+ {UIUtils.errorMessageCell(Option(info.detail).getOrElse(""))}
</tr>
}
- private def errorMessageCell(errorMessageOption: Option[String]): Seq[Node] = {
- val errorMessage = errorMessageOption.getOrElse("")
- val isMultiline = errorMessage.indexOf('\n') >= 0
- val errorSummary = StringEscapeUtils.escapeHtml4(if (isMultiline) {
- errorMessage.substring(0, errorMessage.indexOf('\n'))
- } else {
- errorMessage
- })
- val details = detailsUINode(isMultiline, errorMessage)
- <td>
- {errorSummary}{details}
- </td>
- }
-
private def jobURL(request: HttpServletRequest, jobId: String): String =
"%s/jobs/job/?id=%s".format(UIUtils.prependBaseUri(request, parent.basePath), jobId)
diff --git a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
index 286c0a16251..8874563d76f 100644
--- a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
+++ b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
@@ -31,7 +31,6 @@ import scala.util.control.NonFatal
import scala.xml._
import scala.xml.transform.{RewriteRule, RuleTransformer}
-import org.apache.commons.text.StringEscapeUtils
import org.glassfish.jersey.internal.util.collection.MultivaluedStringMap
import org.apache.spark.internal.Logging
@@ -709,7 +708,7 @@ private[spark] object UIUtils extends Logging {
private final val ERROR_CLASS_REGEX = """\[(?<errorClass>[A-Z][A-Z_.]+[A-Z])]""".r
- private def errorSummary(errorMessage: String): (String, Boolean) = {
+ def errorSummary(errorMessage: String): (String, Boolean) = {
var isMultiline = true
val maybeErrorClass =
ERROR_CLASS_REGEX.findFirstMatchIn(errorMessage).map(_.group("errorClass"))
@@ -724,8 +723,7 @@ private[spark] object UIUtils extends Logging {
errorMessage
}
- val errorSummary = StringEscapeUtils.escapeHtml4(errorClassOrBrief)
- (errorSummary, isMultiline)
+ (errorClassOrBrief, isMultiline)
}
def errorMessageCell(errorMessage: String): Seq[Node] = {
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryPage.scala b/sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryPage.scala
index 7cd7db4088a..e16a09b3874 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryPage.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/streaming/ui/StreamingQueryPage.scala
@@ -24,8 +24,6 @@ import javax.servlet.http.HttpServletRequest
import scala.collection.mutable
import scala.xml.Node
-import org.apache.commons.text.StringEscapeUtils
-
import org.apache.spark.internal.Logging
import org.apache.spark.sql.streaming.ui.UIUtils._
import org.apache.spark.ui.{PagedDataSource, PagedTable, UIUtils => SparkUIUtils, WebUIPage}
@@ -178,17 +176,12 @@ private[ui] class StreamingQueryPagedTable(
.format(SparkUIUtils.prependBaseUri(request, parent.basePath), parent.prefix,
streamingQuery.summary.runId)
- def details(detail: Any): Seq[Node] = {
+ def details: Seq[Node] = {
if (isActive) {
- return Seq.empty[Node]
+ Seq.empty[Node]
+ } else {
+ SparkUIUtils.errorMessageCell(streamingQuery.summary.exception.getOrElse("-"))
}
- val detailString = detail.asInstanceOf[String]
- val isMultiline = detailString.indexOf('\n') >= 0
- val summary = StringEscapeUtils.escapeHtml4(
- if (isMultiline) detailString.substring(0, detailString.indexOf('\n')) else detailString
- )
- val details = SparkUIUtils.detailsUINode(isMultiline, detailString)
- <td>{summary}{details}</td>
}
<tr>
@@ -201,7 +194,7 @@ private[ui] class StreamingQueryPagedTable(
<td>{withNoProgress(streamingQuery, {"%.2f".format(query.avgInput)}, "NaN")}</td>
<td>{withNoProgress(streamingQuery, {"%.2f".format(query.avgProcess)}, "NaN")}</td>
<td>{withNoProgress(streamingQuery, {streamingQuery.lastProgress.batchId}, "NaN")}</td>
- {details(streamingQuery.summary.exception.getOrElse("-"))}
+ {details}
</tr>
}
}
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala
index ca4c1f4007c..f25c150259f 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala
@@ -17,14 +17,17 @@
package org.apache.spark.sql.execution.ui
+import scala.collection.JavaConverters._
import scala.concurrent.duration.DurationInt
+import org.apache.commons.text.StringEscapeUtils.escapeJava
+import org.apache.commons.text.translate.EntityArrays._
import org.openqa.selenium.htmlunit.HtmlUnitDriver
import org.scalatest.concurrent.Eventually.eventually
import org.scalatest.concurrent.Futures.{interval, timeout}
import org.scalatestplus.selenium.WebBrowser
-import org.apache.spark.SparkFunSuite
+import org.apache.spark.{SparkException, SparkFunSuite}
import org.apache.spark.sql.SparkSession
import org.apache.spark.ui.SparkUICssErrorHandler
@@ -50,6 +53,13 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser {
findAll(cssSelector("""#failed-table td .stacktrace-details""")).map(_.text).toList
}
+ private def findErrorSummaryOnSQLUI(): String = {
+ val webUrl = spark.sparkContext.uiWebUrl
+ assert(webUrl.isDefined, "please turn on spark.ui.enabled")
+ go to s"${webUrl.get}/SQL"
+ findAll(cssSelector("""#failed-table td""")).map(_.text).toList.last
+ }
+
private def findExecutionIDOnSQLUI(): Int = {
val webUrl = spark.sparkContext.uiWebUrl
assert(webUrl.isDefined, "please turn on spark.ui.enabled")
@@ -103,4 +113,19 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser {
assert(planDetails.head.contains("TABLE_OR_VIEW_NOT_FOUND"))
}
}
+
+ test("SPARK-44960: Escape html is not necessary for Spark UI") {
+ spark = creatSparkSessionWithUI
+ val escape = (BASIC_ESCAPE.keySet().asScala.toSeq ++ ISO8859_1_ESCAPE.keySet().asScala ++
+ HTML40_EXTENDED_ESCAPE.keySet().asScala).mkString
+ val errorMsg = escapeJava(escape.mkString)
+ val e1 = intercept[SparkException](spark.sql(s"SELECT raise_error('$errorMsg')").collect())
+ val e2 = e1.getCause
+ assert(e2.isInstanceOf[RuntimeException])
+ assert(e2.getMessage === escape)
+ eventually(timeout(10.seconds), interval(100.milliseconds)) {
+ val summary = findErrorSummaryOnSQLUI()
+ assert(!summary.contains("&"))
+ }
+ }
}
diff --git a/streaming/src/main/scala/org/apache/spark/streaming/ui/UIUtils.scala b/streaming/src/main/scala/org/apache/spark/streaming/ui/UIUtils.scala
index dc1af0a940e..57ec162b0d1 100644
--- a/streaming/src/main/scala/org/apache/spark/streaming/ui/UIUtils.scala
+++ b/streaming/src/main/scala/org/apache/spark/streaming/ui/UIUtils.scala
@@ -21,8 +21,6 @@ import java.util.concurrent.TimeUnit
import scala.xml.Node
-import org.apache.commons.text.StringEscapeUtils
-
import org.apache.spark.ui.{ UIUtils => SparkUIUtils }
private[streaming] object UIUtils {
@@ -96,14 +94,7 @@ private[streaming] object UIUtils {
failureReason: String,
rowspan: Int = 1,
includeFirstLineInExpandDetails: Boolean = true): Seq[Node] = {
- val isMultiline = failureReason.indexOf('\n') >= 0
- // Display the first line by default
- val failureReasonSummary = StringEscapeUtils.escapeHtml4(
- if (isMultiline) {
- failureReason.substring(0, failureReason.indexOf('\n'))
- } else {
- failureReason
- })
+ val (failureReasonSummary, isMultiline) = SparkUIUtils.errorSummary(failureReason)
val failureDetails =
if (isMultiline && !includeFirstLineInExpandDetails) {
// Skip the first line
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org