search

ajvondrak / remote_ip

A plug to rewrite the Plug.Conn's remote_ip based on forwarding headers.
MIT License
252 stars 31 forks source link

Make debug logging optional #18

Closed hpopp closed 4 years ago

hpopp commented 4 years ago

Debug logs are a bit verbose in local development, but they currently can't be disabled without upping the logger level (and losing phoenix debug logs, etc).

This PR allows you to enable it via an application env variable (default false)

config :remote_ip, debug: true

I'd be more than happy to rework it into a plug option, but it would require passing the Config struct into RemoteIp.Headers functions, which felt rather dirty.

ajvondrak commented 4 years ago

Hey, thanks for taking the time!

However, I'm going to close this because the implementation here is really what I don't want to do. It imposes an extra Application.get_env lookup at runtime for every log statement, even if we disable debugging. The Logger module's design is a lot nicer, where it uses macros that will get compiled into nothingness if configured the right way, thereby eliminating extra runtime checking.

In fact, since Elixir v1.7, they've added the terribly useful :compile_time_purge_matching option to the configuration. It seems to be relatively unknown, which is unfortunate. I think people are probably sleeping on it because the docs don't do it enough justice. 😉

Under the hood, Logger is adding a bunch of metadata to every Logger.debug/2 (+ info, warning, etc) call made by anyone. You could supply your own custom metadata (which is pretty cool in itself), but all the caller information is automatically gathered as metadata: application, module, function, file name, line number, ...

The point being that the :compile_time_purge_matching option can match against any of this, letting you pick & choose with great precision which log statements you want to purge. And all of that at compile time! 😎

With all this functionality, I don't see a need to reinvent the wheel with configurations on the remote_ip end. I'd rather leave the logging configuration to the Logger module.

But it's still pretty invisible. The first time this ask came up was in a (pretty invisible! 😬) commit comment. I think I'll add a section to the README to point people to the :compile_time_purge_matching option & help spread the word.

This blog post was written prior to v1.7, so this feature basically just gets a footnote, but it might still be useful for further reading about Elixir logging. :book:

hpopp commented 4 years ago

Thanks for the heads up!