observatorium / obsctl

A cli to interact with Observatorium instances.
Apache License 2.0
10 stars 14 forks source link

UX comments from new user #14

Open esnible opened 2 years ago

esnible commented 2 years ago

I attempted to try out obsctl and failed.

(I believe I failed because I configured Observatorium-API to use dex. I will try again with Keycloak later.)

This is how I really tried to use it; I didn't purposely make mistakes.

obsctl metrics get labels
Error: getting current client: getting current context: current context is empty
// confusing/distracting usage message -- my flags were correct.  Showing the flags obscures the actual error message.

// Would be better to say there are no contexts, rather than saying it is an error to list them.
obsctl context list
Error: current context is empty
// confusing usage message -- my flags were correct.
// Should probably say "none" and perhaps suggest the user do obsctl login --tenant <tenant> --api <name>

obsctl context current
Error: current context is empty
// confusing usage message -- my flags were correct.  Should probably say "none", and perhaps suggest how to create one.

obsctl context api add
Error: required flag(s) "url" not set

obsctl context api add --url localhost:8080
Error: localhost:8080 is not a valid URL (scheme: localhost,host: )
// distracting usage message -- my flags were correct

// Gives no output; a confirmation message would be nice.  A suggestion to do "obsctl login" would also be nice.
obsctl context api add --url http://localhost:8080

// I would assumed the above would have set this; the error message should probably suggest doing "obsctl login"
obsctl context current                            
Error: current context is empty

obsctl login
Error: required flag(s) "api", "tenant" not set
// What's confusing here is that the name is optional for `api add`, but the `--api` is not optional.  It is also confusing that the "name" is called `--name" in `obsctl context api add` but called `--api` in `obsctl login`.

// The error message makes me wonder if the nameless `obsctl context api add` above had any effect.
obsctl login --tenant test-oidc --api "" 
Error: api name  does not exist, please add it in via 'context api add'

obsctl context api add --url http://localhost:8080 --name localhost

// (It might be good to disallow "" for --oidc.issuer-url)
obsctl login --tenant test-oidc --api localhost
Error: creating authenticated client: constructing oidc provider: Get "/.well-known/openid-configuration": unsupported protocol scheme ""

// This message is confusing because there is no grant-type option.  (I realize it comes from dex; others may not)
obsctl login --tenant test-oidc --api localhost --oidc.issuer-url http://localhost:5556/dex
Error: creating authenticated client: fetching token: oauth2: cannot fetch token: 400 Bad Request
Response: {"error":"unsupported_grant_type"}

obsctl login --tenant test-oidc --api dummy     
Error: api name dummy does not exist, please add it in via 'context api add'
// It would be helpful if the error message included the valid values for --api

You might also consider a obsctl context api list command.

esnible commented 2 years ago

After I configured Observatorium to use a Keycloak that I manage I was able to proceed:

// Worked? (no error; yet no confirmation)
obsctl login --tenant test-keycloak --api localhost --oidc.issuer-url https://keycloak-snible.apps.observability-d.p3ao.p1.openshiftapps.com/auth/realms/redhat-external --oidc.client-id="$OBS_CLIENT_ID" --oidc.client-secret="$OBS_CLIENT_SECRET" --oidc.audience=observatorium

// ???
obsctl metrics get labels                                    
Error: indent JSON invalid character 'p' after top-level value

// not enough info to debug
obsctl metrics get labels --log.level=debug
debug: read and parsed config file
debug: fetched token: test-keycloak
debug: saved config in config file
debug: updated token in config file: test-keycloak
debug: made GET request: /api/v1/labels: 404
Error: indent JSON invalid character 'p' after top-level value

obsctl metrics get labels --log.level=trace
panic: unexpected log level

goroutine 19 [running]:
github.com/observatorium/obsctl/pkg/cmd.setupLogger(0xc0001c9680, {0x13a0452, 0x1, 0x1})
    /Users/snible/go/src/github.com/observatorium/obsctl/pkg/cmd/cmd.go:38 +0x617
...

obsctl context list

// The tree is nice; but it might be better to output a flat list of values acceptable to 
// `obsctl context switch` and to put a `*` in front of the current one similar to `kubectl config get-contexts`.
localhost:8080
    - no tenants yet, currently not usable
localhost
    - test-keycloak

obsctl context current                     
The current context is: localhost/test-keycloak

// The usage message does not show the positional argument
obsctl context switch
Error: accepts 1 arg(s), received 0
Usage:
  obsctl context switch [flags]
// It might be easier to use `set-context` instead of `switch` to be more like `kubectl` and friends

obsctl context switch localhost/test-keycloak
// no output; perhaps better to have confirmation by outputting the current context
saswatamcode commented 2 years ago

Thanks for trying this out and giving valuable feedback! Yup, the UX needs to be worked upon, as currently there is only very basic functionality. Also, will add some docs soon too! 🙂

esnible commented 2 years ago

After a successful obsctl login I expected the context to be set to the login context. It was unchanged.

esnible commented 2 years ago

I'd like to use obsctl against the interactive Observatorium created by make test-interactive.

That test case outputs a token with 24 hour life to stdout. One possibility is to add a --bearerToken <token> (or similar) to obsctl login. It might also be valuable to have the test case print the obsctl command to log in to the interactive test.

I tried to log in with obsctl login --tenant test-oidc --api localhost --oidc.issuer-url https://localhost:62908/dex --oidc.client-id=test --oidc.client-secret=<...>. For me this failed because the CLI tries to contact "localhost", and is presented with a cert for "e2e_interactive-dex". An option to bypass the cert check or check for a different name than the host name would help.

esnible commented 2 years ago

Allow obsctl to take individual username/password.

esnible commented 2 years ago

After login, consider having the system test that the login is valid by attempt to fetch something controlled by authorization.

There are several ways the OIDC provider can be screwed up. For example, I set one up for password grants rather than client credentials. I could log in, but

obsctl metrics get rules
Error: 403 Forbidden response: ""

(Observatorium logged level=debug name=observatorium ts=2022-04-15T16:41:29.062300748Z caller=rbac.go:114 msg="authorization: \"53e55dbc-6ff4-4442-9155-68fda4c219f8\" unknown; groups [] unknown")

saswatamcode commented 2 years ago

Thanks! I plan to address a lot of your suggestions in future PRs! For the time being, I'm adding some docs for obsctl in https://github.com/observatorium/obsctl/pull/16 which can help new users a bit. 🙂

esnible commented 2 years ago

The help text for obsctl metrics get series needs examples. Please add an Example to the cobra.Command. I couldn't figure it out.

Please add a clue to the flag usage for what is needed.

I cannot figure out the value needed for --match. The --start and --end need timestamps but I don't know what timestamp format.