Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-17724][WebUI][Streaming] Unevaluated new lines in tooltip in DAG Visualization of a job #15353

Closed
wants to merge 8 commits into from

Conversation

keypointt
Copy link
Contributor

https://issues.apache.org/jira/browse/SPARK-17724

What changes were proposed in this pull request?

Replace \n of RDD Node name with a space

  1. Since this graph node name is escaped in scala source file, I choose to change it at JavaScript level.
  2. I didn't change scala source file, which I'm afraid could have side effects.

Also, I removed some unused imports and semicolons.

How was this patch tested?

Tested locally

Before fix, \n is part of the popup tooltip:
screen shot 2016-10-03 at 5 28 16 pm

After fix, replace \n with a space:
screen shot 2016-10-04 at 10 17 58 am

@jaceklaskowski
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Oct 5, 2016

Test build #66347 has finished for PR 15353 at commit e50dc17.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Oct 5, 2016

Hm, it does seem to me like this should be fixed at the source. I'm not sure when it would be desirable to render a newline as literally \n -- where is this escaped?

The rest of the changes are not bad but not related. I think touching up surrounding code is OK but this is touching unrelated code. Neutral on that.

@andrewor14
Copy link
Contributor

But this isn't the original intention, which is to actually add a line break where \n is today. IIRC this works correctly on Chrome but not on Safari (or the other way round?). If you can make it work across all browsers then I will merge it. :)

@andrewor14
Copy link
Contributor

Also this is a more general problem, not just for streaming

@keypointt
Copy link
Contributor Author

I just gave it a quick manually visual check and it works for me on both Chome and Safari, and @andrewor14 could you explain more how it's not working?

And I'll dig more to find the root cause (maybe the string escape part), to solve it more generally

@felixcheung
Copy link
Member

it looks like the "after" image still has a \n in socket text stream?
https://cloud.githubusercontent.com/assets/3925641/19097318/a8edc9de-8a58-11e6-8c37-30271b761284.png

@keypointt
Copy link
Contributor Author

@felixcheung nice catch, you are right and a more general fix is needed then, working on it

@andrewor14
Copy link
Contributor

@keypointt by "working" I mean it should be replaced by a line break, not a space

@keypointt
Copy link
Contributor Author

Got it, thanks Andrew

@tdas
Copy link
Contributor

tdas commented Oct 25, 2016

Any updates on this?

@keypointt
Copy link
Contributor Author

sorry I was on vacation last 2 weeks. working on it now, please allow me some time to get it done :)

@keypointt keypointt closed this Jan 20, 2017
asfgit pushed a commit that referenced this pull request Jan 21, 2017
…AG Visualization of a job

https://issues.apache.org/jira/browse/SPARK-17724

## What changes were proposed in this pull request?
For unevaluated `\n`, evaluate it and enable line break, for Streaming WebUI `stages` page and `job` page.
(I didn't change Scala source file, since Jetty server has to somehow indicate line break and js to code display it.)
(This PR is a continue from previous PR #15353 for the same issue, sorry being so long time)

Two changes:

1. RDD Node tooltipText is actually showing the `<circle>`  `title` property, so I set extra attribute in `spark-dag-viz.js`: `.attr("data-html", "true")`

`<circle x="-5" y="-5" r="5" data-toggle="tooltip" data-placement="bottom" title="" data-original-title="ParallelCollectionRDD [9]\nmakeRDD at QueueStream.scala:49"></circle>`

2. Static `<tspan>` text of each stage, split by `/n`, and append an extra `<tspan>` element to its parentNode

`<text><tspan xml:space="preserve" dy="1em" x="1">reduceByKey</tspan><tspan xml:space="preserve" dy="1em" x="1">reduceByKey/n 23:34:49</tspan></text>
`

## UI changes
Screenshot **before fix**, `\n` is not evaluated in both circle tooltipText and static text:
![screen shot 2017-01-19 at 12 21 54 am](https://cloud.githubusercontent.com/assets/3925641/22098829/53c7f49c-dddd-11e6-9daa-b3ddb6044114.png)

Screenshot **after fix**:
![screen shot 2017-01-19 at 12 20 30 am](https://cloud.githubusercontent.com/assets/3925641/22098806/294910d4-dddd-11e6-9948-d942e09f545e.png)

## How was this patch tested?
Tested locally. For Streaming WebUI `stages` page and `job` page, on multiple browsers:
- Chrome
- Firefox
- Safari

Author: Xin Ren <renxin.ubc@gmail.com>

Closes #16643 from keypointt/SPARK-17724-2nd.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…AG Visualization of a job

https://issues.apache.org/jira/browse/SPARK-17724

## What changes were proposed in this pull request?
For unevaluated `\n`, evaluate it and enable line break, for Streaming WebUI `stages` page and `job` page.
(I didn't change Scala source file, since Jetty server has to somehow indicate line break and js to code display it.)
(This PR is a continue from previous PR apache#15353 for the same issue, sorry being so long time)

Two changes:

1. RDD Node tooltipText is actually showing the `<circle>`  `title` property, so I set extra attribute in `spark-dag-viz.js`: `.attr("data-html", "true")`

`<circle x="-5" y="-5" r="5" data-toggle="tooltip" data-placement="bottom" title="" data-original-title="ParallelCollectionRDD [9]\nmakeRDD at QueueStream.scala:49"></circle>`

2. Static `<tspan>` text of each stage, split by `/n`, and append an extra `<tspan>` element to its parentNode

`<text><tspan xml:space="preserve" dy="1em" x="1">reduceByKey</tspan><tspan xml:space="preserve" dy="1em" x="1">reduceByKey/n 23:34:49</tspan></text>
`

## UI changes
Screenshot **before fix**, `\n` is not evaluated in both circle tooltipText and static text:
![screen shot 2017-01-19 at 12 21 54 am](https://cloud.githubusercontent.com/assets/3925641/22098829/53c7f49c-dddd-11e6-9daa-b3ddb6044114.png)

Screenshot **after fix**:
![screen shot 2017-01-19 at 12 20 30 am](https://cloud.githubusercontent.com/assets/3925641/22098806/294910d4-dddd-11e6-9948-d942e09f545e.png)

## How was this patch tested?
Tested locally. For Streaming WebUI `stages` page and `job` page, on multiple browsers:
- Chrome
- Firefox
- Safari

Author: Xin Ren <renxin.ubc@gmail.com>

Closes apache#16643 from keypointt/SPARK-17724-2nd.
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
…AG Visualization of a job

https://issues.apache.org/jira/browse/SPARK-17724

## What changes were proposed in this pull request?
For unevaluated `\n`, evaluate it and enable line break, for Streaming WebUI `stages` page and `job` page.
(I didn't change Scala source file, since Jetty server has to somehow indicate line break and js to code display it.)
(This PR is a continue from previous PR apache#15353 for the same issue, sorry being so long time)

Two changes:

1. RDD Node tooltipText is actually showing the `<circle>`  `title` property, so I set extra attribute in `spark-dag-viz.js`: `.attr("data-html", "true")`

`<circle x="-5" y="-5" r="5" data-toggle="tooltip" data-placement="bottom" title="" data-original-title="ParallelCollectionRDD [9]\nmakeRDD at QueueStream.scala:49"></circle>`

2. Static `<tspan>` text of each stage, split by `/n`, and append an extra `<tspan>` element to its parentNode

`<text><tspan xml:space="preserve" dy="1em" x="1">reduceByKey</tspan><tspan xml:space="preserve" dy="1em" x="1">reduceByKey/n 23:34:49</tspan></text>
`

## UI changes
Screenshot **before fix**, `\n` is not evaluated in both circle tooltipText and static text:
![screen shot 2017-01-19 at 12 21 54 am](https://cloud.githubusercontent.com/assets/3925641/22098829/53c7f49c-dddd-11e6-9daa-b3ddb6044114.png)

Screenshot **after fix**:
![screen shot 2017-01-19 at 12 20 30 am](https://cloud.githubusercontent.com/assets/3925641/22098806/294910d4-dddd-11e6-9948-d942e09f545e.png)

## How was this patch tested?
Tested locally. For Streaming WebUI `stages` page and `job` page, on multiple browsers:
- Chrome
- Firefox
- Safari

Author: Xin Ren <renxin.ubc@gmail.com>

Closes apache#16643 from keypointt/SPARK-17724-2nd.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants