Adding IP Addresses in log entries

[husobee]

Addressing this feature: #112

Looking forward to hearing your thoughts.

[vishr]

Can't we just display what's coming in the request?

[husobee]

Sure, we could, lots of times I find myself proxy-ing from another web server (nginx, apache), or a load balancer, and sometimes using headers can be helpful in those cases. Otherwise it will look like the request is coming from the proxy host or load balancer instead of who it is really coming from.

[axdg]

IMHO, this logger, and the other core middleware (except maybe compression) aren't necessarily meant to be used for every production situation. The logs it produces don't contain several values that might be needed by some users. It should be kept as simple as possible.

@husobee, out of curiosity, why not use net.SplitHostPort

[vishr]

I understand, getting real IP should be the case. As @axdg mentioned, we should be using net.SplitHostPort.

[vishr]

I agree with @axdg, there could many possible values to put in the logs. In future we should be capable of specifying the format (with fields) for logger (low priority).

[vishr]

I am okay with the proxy headers, that's how it should be, I was more specific about calling net.ParseIP do we really need it, it's just a logger? We should just get the IP, split and display. What do you guys think?

[vishr]

@husobee I think, you should bring back the header logic.

[husobee]

will do, thanks vishr.

[axdg]

Header is fine, it obviously useful if your running behind a proxy. But as it stands, the values that are being logged would appear to have been chosen arbitrarily. At some point it’ll need a refactor, as vishr said.

P.S. do you guys have some time to discuss #205 in gitter?

[vishr]

I am outside, you guys can discuss send me the transcript.

Sent from my iPhone

On Sep 18, 2015, at 7:54 AM, axdg notifications@github.com wrote:

In middleware/logger.go:

@@ -34,7 +64,7 @@ func Logger() echo.MiddlewareFunc {
code = color.Cyan(n)
}

•       log.Printf("%s %s %s %s %d", method, path, code, stop.Sub(start), size)
  

•       log.Printf("%s %s %s %s %s %d", parseIP(remoteAddr), method, path, code, stop.Sub(start), size)
  

  Header is fine, it obviously useful if your running behind a proxy. But as it stands, the values that are being logged would appear to have been chosen arbitrarily. At some point it’ll need a refactor, as vishr said. P.S. do you guys have some time to discuss RFC 2616 10.4.6 Compliance - Method Not Allowed On Resources Method isn't Specified #205 in glitter? On 19 September 2015 at 12:50:50 am, husobee (notifications@github.com) wrote: In middleware/logger.go:
  @@ -34,7 +64,7 @@ func Logger() echo.MiddlewareFunc { code = color.Cyan(n) } - log.Printf("%s %s %s %s %d", method, path, code, stop.Sub(start), size) + log.Printf("%s %s %s %s %s %d", parseIP(remoteAddr), method, path, code, stop.Sub(start), size)
  will do, thanks vishr. — Reply to this email directly or view it on GitHub.
  —
  Reply to this email directly or view it on GitHub.

[vishr]

Closed #214