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

escape meta-characters to fix dashboard #152

Merged
merged 1 commit into from
Jul 24, 2013
Merged

escape meta-characters to fix dashboard #152

merged 1 commit into from
Jul 24, 2013

Conversation

mrhooray
Copy link
Contributor

I want to solve #100 and #125 in a proper way rather than directly replacing 'invalid characters'.

By looking at the code, diagram flooding is caused by:
https://github.com/Netflix/Hystrix/blob/master/hystrix-dashboard/src/main/webapp/components/hystrixCommand/hystrixCommand.js#L211
https://github.com/Netflix/Hystrix/blob/master/hystrix-dashboard/src/main/webapp/components/hystrixThreadPool/hystrixThreadPool.js#L170

The selectors do not work as expected due to meta-characters. Although replacing those meta-characters would work to prevent flooding, it will cause further bug since different meta-characters on the same position of two different commands lead to the same command name (e.g. both my:command and my-command will reduce to my_command).

A clean approach is to escape those meta-characters in command name when used in selectors. Another advantage of this way is that original command/thread pool name can be displayed on the web page.

This patch creates escapedName in data object and replaces occurrences of original name in selectors.

@cloudbees-pull-request-builder

Hystrix-pull-requests #23 ABORTED

@benjchristensen
Copy link
Contributor

Nice! Thanks @mrhooray

benjchristensen added a commit that referenced this pull request Jul 24, 2013
escape meta-characters to fix dashboard
@benjchristensen benjchristensen merged commit 6860356 into Netflix:master Jul 24, 2013
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.

3 participants