crate / cratedb-prometheus-adapter

CrateDB Prometheus Adapter.
Apache License 2.0
60 stars 15 forks source link

Improve installation, configuration, and first invocation #49

Closed amotl closed 9 months ago

amotl commented 3 years ago

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

chaudum commented 3 years ago

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.

proddata commented 2 years ago

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

What do you think @amotl ?

amotl commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

[...] 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 commented 1 year ago

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

amotl commented 1 year ago

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 commented 1 year ago

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

amotl commented 9 months ago

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.