api client examples: avoid shadowing package with variable

[cben]

Hi 👋. I'm coming from reading this example at https://godoc.org/github.com/prometheus/client_golang/api/prometheus/v1#example-API--Query. (thanks #630)

In that rendered doc, one only sees the example body, package imports and their aliases are invisible.
It'd be reasonable to guess that api refers to github.com/prometheus/client_golang/api, it's the only directory of that name here.
However, below it becomes shadowed by a variable of same name api := v1.NewAPI(client), which makes the whole example very confusing to read! (I didn't even know you can do that in Go – access an import in same function before a variable of same name...)

This is an attempt to rename the variable, leaving package names at their default so one can still guess what they refer to.

cc @krasi-georgiev

[cben]

I'm actually not trying to use this client yet, just learn from it for refactoring/naming a tiny internal client we have in our app, so let me bounce some feedback on your naming, without knowing if you're interested in discussing that or already settled 😉

Currently to use this one is supposed to import both the versioned api/prometheus/v1 and the unversioned api.
What will you do if in v2 you'll want to change the interface of api.NewClient or api.Config?

Hmm:

Package api provides clients for the HTTP APIs.

Package v1 provides bindings to the Prometheus HTTP API v1: http://prometheus.io/docs/querying/api/

Ah, so the v1 refers to version of the prometheus API, not versioning of this package? OK that makes more sense 👍.

Anyway it'd generally be easier to use, and to find docs, if one only had to deal with just 1 package...

Naming-wise, the separation of concerns between api.NewClient() and v1.NewAPI() sounds unclear, kinda circular — ask the api package for a "client", but then wrap a "client" in an "API" (which is what you'll actually use) 🔃 ...

• Is api package expected to ever contain clients for things other than prometheus? In that case, api.Config having a field called just Address won't scale, maybe it should be say PromethesAddress?

  [EDIT: To answer myself:
  #333 shed some light on that.
  Yes, it was considered that api/alertmanager/v1 and similar packages could grow here, if other people are willing to maintain them.
  For alertmanager specifically, the client code ended up in alertmanager repo as that avoided some circular-import problem.
  """We decided against api/v1/{alertmanager,prometheus} on purpose as these are unrelated APIs and we didn't want to give the impression their versioning is in any way related."""]

• Maybe calling that package apis would be better (and avoid aliasing which NewAPI() begs for)?

• I'm not sure a user cares for a shared api.Client object. Its .Do() and .URL() methods feel more like implementation helpers, do you consider them public?

  • api.DoGetFallback makes sense for the POST body / GET url params equivalence of specifically Prometheus v1 api, probably doesn't belong as public api function.

• What if we keep api.Config but fed it directly into v1.NewAPI() directly, and/or renamed that v1.NewClient()?

• What if we moved Config under prometheus/v1/ so user doesn't need to touch api package at all?

I'm happy to work on PRs if any of these ideas sound good to you...
I'm even happier to learn why current design is right and I'm wrong 😁
(In the long term, I'd like to get off our private code and start using this official client, and start contributing here)

[UPDATE 2021: I have less time to contribute now.]

[beorn7]

I guess these are all good points. @joe-elliott, this seems related to the discussion we had recently.
I wouldn't be opposed to a general clean-up / refactoring to make the whole API client easier to use / more consistent and concise.

@joe-elliott and @cben perhaps the issues ran into here are a great starting point to come up with an improvement?

(Historical remark: The API client was never properly maintained. It got dropped here in an experimental stage, but then nobody really picked it up. @krasi-georgiev gave it a try, but he doesn't have enough time anymore. While I am the maintainer of this GH repo, I don't really have a lot of context or opinions about the API client.)

[beorn7]

The renaming in this PR makes sense in any case. Thank you very much.