arp242 / goatcounter

Easy web analytics. No tracking of personal data.
https://www.goatcounter.com
Other
4.46k stars 182 forks source link

Support config from env vars #227

Closed ItalyPaleAle closed 2 years ago

ItalyPaleAle commented 4 years ago

Would be really useful if we could pass configuration options (e.g. for serve) via environmental variables. For example, setting a -listen address with LISTEN=0.0.0.0:8081, etc.

Reason for this is that it makes deploying the app much simpler in certain scenarios, such as using Docker (you can define the LISTEN variable in the container), certain PaaS platforms, etc.

arp242 commented 4 years ago

My thinking is that you can use goatcounter serve -listen ${LISTEN:-perhaps_a_default} to pass stuff from the environment explicitly. I actually wrote a thing about this a while ago (specifically, the second section).

ItalyPaleAle commented 4 years ago

Thanks for the quick response. The problem with that is that you still need to manually pass the flags.

For example, if you're running this in Docker, I can set the default command to be goatcounter serve, with the path to the database set as environmental variable within the container. This way, a normal docker run would just suffice.

But, what if users want to run goatcounter create? They still need to specify -db themselves unless the value is read from the environment.

arp242 commented 4 years ago

Looks like you deleted your comment on #34 btw? Here is some additional discussion surrounding Docker, FYI: https://github.com/zgoat/goatcounter/issues/130#issuecomment-585874776

ItalyPaleAle commented 4 years ago

I did, because I first wanted to try running it on Docker :) I got it to build and run with a container, but I am having some issues. One of them is the user experience, hence this issue about env vars

arp242 commented 4 years ago

But, what if users want to run goatcounter create? They still need to specify -db themselves unless the value is read from the environment.

Actually, how does that work in a generic container the first place? Just docker sh and then /goatcounter create [..]?

Like I mentioned in the article, I've solved this with wrapper scripts, which act as a sort of config file. It's a bit idiosyncratic perhaps 😅, but found it actually works quite well.

ItalyPaleAle commented 4 years ago

Two ways:

  1. Like you said, docker exec <container_name> sh and then running /goatcounter create, assuming the container has a shell inside
  2. Or, just running the binary in a different container: docker run --rm --volume <path>:<path> goatcounter /goatcounter create (as long as the volume with the SQLite database is mounted, or the connection string to Postgres is set)

Nothing against wrapper scripts, but I have seen env vars used as the more standard approach usually.

arp242 commented 4 years ago

but I have seen env vars used as the more standard approach usually.

Yeah, but that would be boring 😆

I don't mind adding env support if that solves real problems for people; it's not my preferred way, but I am above all pragmatical in these kind of things. It's just that there's a lot of stuff to do, and more code is more to maintain, test, and stuff that can go wrong, so I'd prefer to keep it to just flags; but like I said, if it solves otherwise hard to solve problems then yeah, it's probably better to add it.

The way I'd probably do it is read all env vars with a certain prefix (GC_ maybe), match that to the flags (GC_LISTEN → -listen, GC_DEBUG → -debug, etc.) and error out if the flag is unknown to catch typos etc, and also print Using -listen and -debug from environment to stdout. That way it's a bit more transparent than just "read from env if it exists"; I've seen quite a few situations where people are confused and don't even know if a flag is being applied or not, only to discover they made a typo or something. I'd probably modify or wrap Go's flag package to do that automatically (maybe something like that already exists?)

That said, I'm just this guy you know, so not sure when I have time for this 😅 PRs are always welcome, of course.

ItalyPaleAle commented 4 years ago

I think your approach makes sense.

I was inspecting the code and I was thinking another way might be to change the cfg package and add an init method that sets a default value if the env var is found? This way, flags can still be used to override the env vars. But, it seemed that this would conflict more with the flag package.

Or maybe just modify the invocation of the flag to use os.Getenv to provide the default value!

There are options. I can look into them.

First, however, I need to figure out why login isn't working for me (after pasting the link, I'm redirected back to the login page). But that's another issue (literally)

arp242 commented 4 years ago

The cfg package isn't used for everything though, and by using init() you won't be able to to override he environment with flags. Also, I'd ideally like to get rid of that package as much as possible, and just pass stuff to the backend{} struct as a field. To be honest, this part is a bit messy.1

For now, I'd just recommend trying to go with wrapper scripts and see how far that gets you; a generic one which should work for all commands would be something like:

#!/bin/sh

set -eu

cmd=${1:-}
[ -z "$cmd" ] && goatcounter  # No command: run help.
[ -n "${1:-}" ] && shift

goatcounter $cmd -db "${GC_DB}" $@

It might be worth adding that to the repo actually, or at least documenting it somewhere.

The login issue sounds like a problem with the cookie not being set correctly? But ya, make a new issue with the command you used and the value of the Cookie: header you get back from the server.


1: Aside: there is a bit of history with this, GoatCounter started life as GoatLetter, a very small MailChimp-like newsletter platform for my personal website, I never quite finished that, mostly because email is hard and annoying, and also something I spent 3 years doing at my last job so I was a bit tired of it. I recycled the GoatLetter codebase for GoatCounter, and while a global cfg package was okay for a very small app, I think it makes a bit less sense for larger ones like GoatCounter. Actually, even GoatLetter was recycled from a recipe platform I built before that 😆 I still want to finish that as it has some neat ideas IMO, but kinda hard to monetize, so something for the long term...