cratedb-prometheus-adapter icon indicating copy to clipboard operation
cratedb-prometheus-adapter copied to clipboard

Improve user experience on first invocation

Open amotl opened this issue 3 years ago • 4 comments

Hi there,

when downloading/using a release package/image from [1] and invoking ./cratedb-prometheus-adapter, it croaks blatantly:

FATA[0000] Error loading configuration file "config.yml": error reading configuration file: open config.yml: no such file or directory  source="server.go:406"

In order to guide the user better, we might

a) add a link to the configuration blueprint at https://raw.githubusercontent.com/crate/cratedb-prometheus-adapter/main/config.yml. b) provide an option to create a configuration blueprint using the program itself.

With kind regards, Andreas.

P.S.: @chaudum recently improved the situation on this matter when invoking the program using Docker [2] (thanks!). Follwing that, recent Docker images include the configuration blueprint config.yml, essentially resolving #29. Of course, this will only work well on installations where the default password has not been changed.

[1] https://cdn.crate.io/downloads/dist/prometheus/ [2] https://ghcr.io/crate/cratedb-prometheus-adapter

amotl avatar May 04 '21 17:05 amotl

I have to agree that the user experience could be better. Given the fact, that a valid config.yml only requires at minimum a single endpoint with a host field. Everything else is optional, meaning that

cratedb_endpoints:
- host: localhost

would be a valid configuration. So, I argue that such a minimum config could also be used as a fallback in case there is no -config.file provided or the config.yml in the working directory of the cratedb-prometheus-adapter is missing.

Another option would be to make the -config.file parameter required and fail with a better error message.

chaudum avatar May 07 '21 12:05 chaudum

I think this sentence in the README.md is a bit misleading in that context

The CrateDB endpoints are provided in a configuration file, which defaults to config.yml (-config.file flag).

So maybe we should

  • include the config.yml also in the native release archives
  • make it optional as suggested by @chaudum
  • change the text in Usage in the README.md to only refer to the docker setup

What do you think @amotl ?

proddata avatar Jan 05 '22 12:01 proddata

Dear @chaudum and @proddata,

thank you for your replies. There is clearly a need to improve the situation and documentation for initial installations, both with Docker and with native deployment variants. In this context, I also would like to reference a recent discussion at CrateDB Prometheus adapter service not starting where we confirmed a working recipe to install and configure the systemd-based setup variant. I will look into bringing those insights back to the repository next week.

With kind regards, Andreas.

/cc @jayeff

amotl avatar Jan 14 '22 19:01 amotl

Hi again,

while answering a question on the forum at ^1, it just occurred to me that we might be better off configuring this service by using environment variables instead of command line options or a dedicated configuration file. This will improve usability in container environments, e.g. when following the tutorial at ^2.

I think the nine configuration variables currently employed by the program ^3 can be wrapped into environment variables instead. In this way, the program would not need a dedicated configuration file.

With kind regards, Andreas.

/cc @rafaelasantana, @jayeff

amotl avatar Apr 06 '22 15:04 amotl

[...] it just occurred to me that we might be better off configuring this service by using environment variables instead of command line options or a dedicated configuration file.

When revisiting this topic, I discovered that it will not be so easy, because the data model of config.yml represents a list of dictionary items, for configuring multiple endpoints. List structures are always unwieldly to map to environment variables, right?

While ^1 shows an example how to represent lists or objects containing scalar values,

export GITHUB_REPOS=webargs,konch,ped
export GITHUB_REPO_PRIORITY="webargs=2,konch=3"

I believe there is no standard or sane way to represent a list of objects within environment variables? [^2] So, let's skip this idea, and keep the configuration file.

[^2]: Please let me know if you do!

amotl avatar Jan 17 '23 20:01 amotl

GH-59 implements some suggestions from this discussion. Please let me know about any changes you would like to see.

amotl avatar Jan 17 '23 20:01 amotl

Hi again,

following up on https://github.com/crate/cratedb-prometheus-adapter/issues/49#issuecomment-1013388632, GH-60 finally pulls the improved documentation section from ^1 about how to invoke the service using systemd into the README.

With kind regards, Andreas.

amotl avatar Jan 18 '23 18:01 amotl

GH-61 was needed to fix up some flaws introduced with GH-59.

amotl avatar Jan 20 '23 10:01 amotl

I think what has been outlined and suggested here has been implemented already. Please let me know if you think something is still missing, and re-open the ticket.

amotl avatar Jan 17 '24 00:01 amotl