Add new Servicegraph visualization. #3318
Conversation
Adds a new viz under /force/forcegraph.html. New serialization needed in d3graph.go. Update README.
|
Hi @jeffmendoza. Thanks for your PR. I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
addons/servicegraph/README.md
Outdated
| ## Services | ||
| - `/force/forcegraph.html` is an interactive | ||
| [D3.js](https://d3js.org/) visualization. This graph only displays | ||
| services that are currently receiving traffic. |
There was a problem hiding this comment.
Is there a way to use a time_horizon argument on this visualization? It is a bit strange to have a graph disappear dynamically when playing around with it simply because traffic stopped for a bit (this is especially true for testing).
There was a problem hiding this comment.
Yes, the js code queries the backend with those parameters. The default I chose is time_horizon=10s&filter_empty=true, as this leads to good demos of adding services and/or changing route rules.
My idea was to have UI elements to toggle those things, but I don't have those now. I think some js to pass those parameters through would be an easy interim solution.
| All endpoints also take an optional arugment of `filter_empty=true`, which will | ||
| restrict the nodes and edges shown to only those that reflect non-zero traffic | ||
| levels during the specified `time_horizon`. | ||
| - `/d3graph` provides a JSON serialization for D3 visualization. |
There was a problem hiding this comment.
i wonder: do we need two JSON serializations for the same graph?
There was a problem hiding this comment.
I was more comfortable doing the transformation in Go. I think the core Static/Dynamic graph structure is better for internal representation than the one that D3 wants, so I didn't want to change that. The /graph endpoint could be removed, as it is not used, but It is trivial to keep it, and someone might find a way to use it.
addons/servicegraph/README.md
Outdated
|
|
||
| ## Query Parameters | ||
|
|
||
| All endpoints except for `/force/forcegraph.html` take these query parameters: |
There was a problem hiding this comment.
as above: it would be really nice if there wasn't a behavior difference here.
| stroke: black; | ||
| stroke-width: 1.5px; | ||
| cursor: move; | ||
| fill: #466BB0; |
There was a problem hiding this comment.
this is way way down in the weeds, but I think we might want to consider a different color (non-red). would using istio blue make sense?
There was a problem hiding this comment.
It is Istio blue now, I had it as "lightcoral" before.
There was a problem hiding this comment.
ha. ok. i need to learn hex colors ;).
| <link rel="stylesheet" href="forcegraph.css"> | ||
| </head> | ||
| <body> | ||
| <div id="total"> |
There was a problem hiding this comment.
Can we add some text to this page that describes what it is and how to use (a la "click on nodes to see metrics")?
jeffmendoza
force-pushed
the
servicegraph-work
branch
from
48f41ca to
b93ed7d
Compare
|
@douglas-reid Comments addressed. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: douglas-reid The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue. |
|
in the future any chance to move those large js files to contrib/ or use CDN urls ? |
|
No plans at the moment, I think it is better to serve them from the container. I do think all of addons/ should be moved out to the contrib repo at some point. Right now build/release uses only istio/istio, and it's best to keep the addons versioned and released with the rest of Istio for now. I imagine the release process will grow to encompass more repos again at some point. |
Adds a new viz under /force/forcegraph.html. New serialization needed
in d3graph.go. Update README.