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

Improve ztunnel dashboard UI #638

Open
hanxiaop opened this issue Aug 4, 2023 · 9 comments
Open

Improve ztunnel dashboard UI #638

hanxiaop opened this issue Aug 4, 2023 · 9 comments
Labels
area/debuggability Area: Debuggability (logs, utils, admin endpoints, etc) area/dev-exp Area: Developer Experience

Comments

@hanxiaop
Copy link
Member

hanxiaop commented Aug 4, 2023

Ref: istio/istio#45695 (comment)

  1. ztunnel /config_dump does not return a pretty printed json object
  2. ztunnel /logging is unusable since it expects a POST and a level, this should be similar to envoy logging API.
  3. ztunnel currently doesn't have a /stats API on 15000, but it does have one on :15020/metrics
@hanxiaop hanxiaop added area/dev-exp Area: Developer Experience area/debuggability Area: Debuggability (logs, utils, admin endpoints, etc) labels Aug 4, 2023
@howardjohn
Copy link
Member

Doesn't logging in envoy use a POST as well? Modifications should require a POST? Or are we just talking about viewing the current level?

For stats, we don't need to copy envoy IMO.

@hanxiaop
Copy link
Member Author

hanxiaop commented Aug 7, 2023

Doesn't logging in envoy use a POST as well? Modifications should require a POST? Or are we just talking about viewing the current level?

We are talking about viewing the current level, currently it's not viewable directly like envoy.

For stats, we don't need to copy envoy IMO.

+1, I think 15020/metrics is good.

@Stevenjin8
Copy link
Contributor

Stevenjin8 commented Aug 10, 2023

I can do the pretty print

and log level

@Stevenjin8
Copy link
Contributor

Stevenjin8 commented Aug 11, 2023

@hanxiaop

ztunnel /logging is unusable since it expects a POST and a level, this should be similar to envoy logging API.

I don't think this is completely true. I can get the logging level of ztunnel through an empty post:

root@ztunnel-4v22b:/# curl -d "" localhost:15000/logging
current log level is info

Though, it is much less descriptive and formatted differently than envoy:

istio-proxy@my-pod:/$ curl -d "" localhost:15000/logging
active loggers:
  admin: warning
  alternate_protocols_cache: warning
...

Neither ztunnel nor enovy support GET requests for /logging.

So I wouldn't say that " /logging is unusable since it expects a POST and a level," but the ztunnel api is definitely different than the envoy API.

@hanxiaop
Copy link
Member Author

@Stevenjin8 Thanks for testing this. We are talking about the /logging API in the dashboard. Now, you can try to see the ztunnel dashboard using a command like istioctl d envoy ztunnel-bg4b7.istio-system. Once you click on the /logging API, it shows

Invalid HTTP method
 
usage: POST /logging						(To list current level)
usage: POST /logging?level=<level>				(To change global levels)
usage: POST /logging?level={mod1}:{level1},{mod2}:{level2}	(To change specific mods' logging level)

hint: loglevel:	error|warn|info|debug|trace|off
hint: mod_name:	the module name, i.e. ztunnel::proxy

I'm not sure, but I think we may need to modify the dashboard page, and perhaps the API a little, to make it work?

@Stevenjin8
Copy link
Contributor

@hanxiaop Thank you for that. I was able to reproduce.

From a brief look at the HTML, the difference lies in the dashboard page rather than the API.

The issue with the current ztunnel dashboard page is that it uses a hyperlink to reference the logging endpoint.

<a href="logging">logging</a>

This means that when you click on the endpoint, the browser will send a GET request to the logging endpoint rather than an empty POST request (I couldn't find a way to change this behavior without javascript)

The envoy logging dashboard uses buttons instead of hyperlinks which allows it to send post requests. This is kinda hacky thought because if you go to the logging endpoint and then refresh, the page, you'll get an error. It also looks kinda janky because there are a mix of buttons and hyperlinks

I think the better solution is to have envoy and ztunnel accept GET requests. If we just want to mirror behavior, we can replace the hyperlink with a button.

@hanxiaop
Copy link
Member Author

I think the better solution is to have envoy and ztunnel accept GET requests. If we just want to mirror behavior, we can replace the hyperlink with a button.

@Stevenjin8 I think adding a GET request to Envoy may not be acceptable since POST has been used for a while and may cause confusion. However, since we have full control of ztunnel, it may be fine to have a different behavior than Envoy? @GregHanson Do you have any suggestions?

@howardjohn
Copy link
Member

howardjohn commented Aug 15, 2023 via email

@GregHanson
Copy link
Member

@hanxiaop @Stevenjin8 I don't like the idea of modifying the API to accept GET requests. I envisioned this one would require some html magic to get a page more similar to what envoy has (i.e. buttons for POST etc).

This issue encompassed a few items of varying degrees of importance IMO.

  • pretty printing the json to something human-readable: HIGH
  • including the stats API: design/implementation question
  • fixing the html for the dashboard: post-beta

Agreeing with John that the remaining item is lower priority. This issue could be used for future tracking - or we can mark it as community/help-wanted/community/good-first-issue for any savvy html devs that want to take a stab at it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/debuggability Area: Debuggability (logs, utils, admin endpoints, etc) area/dev-exp Area: Developer Experience
Projects
None yet
Development

No branches or pull requests

4 participants