Set user/Event IP addresses more accurately.

[nateberkopec]

Using X-Forwarded-For and a few other HTTP headers, we can more
accurately determine the client's IP address than just reporting
REMOTE_ADDR. This will help setups where the client is deployed beyond
a proxy like nginx.

---

This change is

[naw]

@nateberkopec Thanks for your initiative on this!

I'm totally in support of the overall change.

Since the logic for guessing the client ip from various headers isn't specific to Raven (right?), I'm wondering if there is any way to extract the logic into a separate library (perhaps a separate Rack middleware) that is automatically wrapped around Raven middleware? Or, maybe there is a way to piggyback on Rails' remote_ip middleware? (although perhaps that's not feasible without making Rails a dependency, which wouldn't be good, of course).

I'm not sure this is feasible, just throwing it out as an idea.

[nateberkopec]

Since the logic for guessing the client ip from various headers isn't specific to Raven (right?)

It really is, actually. Most people, when they want "the real ip of the client", want "the real ip of the client which I can trust". ActionDispatch::RemoteIp does the latter, and if you need that behavior, you should just use that. The behavior of this PR is a little different, since it will return the oldest IP address in the chain, not the first IP address after all trusted proxies and local addresses have been removed.

[naw]

Yes, thanks for correcting my misconception on the similarities between ActionDispatch::RemoteIp and your approach.

I don't have an opinion on which approach is a better default for Raven, but both seem a lot more useful than the current REMOTE_ADDR approach, which is why I'm in support of the change.

The primary point I was trying to make was about modularity. In other words, it seems to be a ripe opportunity for a somewhat generic Rack middleware that examines headers and makes an educated guess for the client's ip. Rather than embedding the logic directly into the Raven repo, I was imagining it could have its own life outside of Raven, and Raven could consume it. Of course, that's extra work, and I don't mean to impose that on you, nor do I mean to imply that modularity is always best.

Thanks again for your work on this.

[nateberkopec]

Yeah no problem, the differences are subtle but important. It took me a few hours of thinking just to come up with the right solution to all of this.

I'm against a modular approach here just because a) I think most people want the "secure" approach to this problem, when solving it in other contexts b) I don't want some to accidentally use my modular solution for insecure purposes. Of course, this lib is Apache licensed so anyone is free to take this work and modularize it for their own purposes.