Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GeoIP resolution fails to work when running GoatCounter without a reverse proxy #294

Closed
mcginty opened this issue Jun 11, 2020 · 11 comments · Fixed by #297
Closed

GeoIP resolution fails to work when running GoatCounter without a reverse proxy #294

mcginty opened this issue Jun 11, 2020 · 11 comments · Fixed by #297
Labels

Comments

@mcginty
Copy link

mcginty commented Jun 11, 2020

It would be great to have a section in the README or somewhere else that explained how to enable location support in the self-hosted version of goatcounter.

Right now, all my visits using the binary from the releases tab here are showing "(unknown)" as the location, and the same thing happens if I drop my own GeoLite2-Country.mmdb into the root directory of goatcounter and do my own static build.

@arp242
Copy link
Owner

arp242 commented Jun 11, 2020

This should already be enabled by default. The GeoDB is compiled in the binary: https://github.com/zgoat/goatcounter/blob/master/pack/geodb.go

I think you might not be forwarding the IP address correct if you're running stuff through a proxy? Try setting X-Real-IP or adding it to X-Forwarded-For.

@arp242 arp242 added the support label Jun 11, 2020
@mcginty
Copy link
Author

mcginty commented Jun 11, 2020

Thanks so much for the fast response! I see, I'm not doing any funny business, running it on a Google Cloud compute instance without any proxy or middleman software. I'll try from more IPs, maybe my ranges I tested from are simply not recognized by GeoIP's Country DB.

@arp242
Copy link
Owner

arp242 commented Jun 11, 2020

I'm not really familiar with Google cloud, but if you add something like:

fmt.Printf("ra: %q; xrip: %q; xff: %q\n", r.RemoteAddr, r.Header.Get("X-Real-IP"),
    r.Header.Get("X-Forwarded-For"))

Over here: https://github.com/zgoat/goatcounter/blob/master/handlers/backend.go#L209

And recompile, you can check the output to see which IP address is being used, just to make sure that's correct. I'm reasonably sure this is your problem, but can never be 100% sure 😅

Should probably add a -debug flag for this btw.

@mcginty
Copy link
Author

mcginty commented Jun 11, 2020

ra: "<my actual external IP>:50296"; xrip: ""; xff: ""

and https://www.maxmind.com/en/geoip-demo identifies the IP correctly, so I don't think it's that either. This change was made on the release-1.3 branch, FWIW. Thanks again for the fast response (and lovely project, I love the simplicity of the UI).

@arp242
Copy link
Owner

arp242 commented Jun 11, 2020

I think that port number in there may be the issue. If you do use a proxy then there won't be any port number I think, so I never noticed. Removing that should probably fix it. I'll take a look later.

@arp242
Copy link
Owner

arp242 commented Jun 11, 2020

Looks like this is a reported issue already: go-chi/chi#453

@mcginty
Copy link
Author

mcginty commented Jun 11, 2020

Ah, I see! It seems like maybe using a proxy is the right way to go for now anyway. I also noticed that GoatCounter fails to successfully negotiate an ACME certificate unless I manually specify the listen address to 0.0.0.0 as well, a la ./goatcounter serve -automigrate -listen 0.0.0.0:443, which I didn't find documented elsewhere either.

Thanks again for the help :).

@arp242
Copy link
Owner

arp242 commented Jun 11, 2020

Yeah, there's #285 for the acme thing. To be honest, my experience with implementing acme in GoatCounter has left me quite disappointed with the entire thing; there are so many assumptions with how you've set up your server that it completely breaks the "just run a dev server on :8080" (... and don't even get me started about wildcard certificates...)

@mcginty
Copy link
Author

mcginty commented Jun 11, 2020

With Caddy being so easy to setup, maybe you could not need to maintain the ACME code from GoatCounter and instead recommend people self-hosting to use Caddy with an example Caddyfile? The configuration is literally just two lines to get it working with GoatCounter. And now country reporting works for me too!

analytics.example.com

reverse_proxy http://localhost:8081

@arp242
Copy link
Owner

arp242 commented Jun 11, 2020

Yeah, but then you need to run Caddy, haha. Right now GoatCounter can run without any external dependencies (other than a place to store the SQLite database file – no way around that) which is a property I rather like. Also, Caddy has pretty much the same problems as GoatCounter; from memory it just refuses to run without root or setcap [..] permissions. So basically, GoatCounter just needs to copy that behaviour to make it clearer, and then it's pretty much identical to Caddy (except without having to run Caddy).

The only reason I use hitch as a TLS proxy on goatcounter.com is because I run some other stuff on the server as well, but other than that there is no real reason to do so (no longer, anyway). Ideally, I'd like to simplify things a bit at some point but it's not a huge priority.

@mcginty
Copy link
Author

mcginty commented Jun 12, 2020

Respect on the no-dependency philosophy.

Yep, it's true things should work fine without Caddy and with a few small changes. I'll rename the title of this issue and keep it open, but feel free to close it if this is tracked elsewhere. Thanks again for your support and for making such a nice straightforward analytics project.

@mcginty mcginty changed the title Instructions on GeoIP support in self-hosted version missing GeoIP resolution fails to work when running GoatCounter without a reverse proxy Jun 12, 2020
arp242 added a commit that referenced this issue Jun 12, 2020
arp242 added a commit that referenced this issue Jun 12, 2020
arp242 added a commit that referenced this issue Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants