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

Logging #5

Open
TwP opened this issue Apr 20, 2013 · 6 comments
Open

Logging #5

TwP opened this issue Apr 20, 2013 · 6 comments

Comments

@TwP
Copy link
Contributor

TwP commented Apr 20, 2013

We need to add some logging to the Client class. For each request the client makes, we should output a detailed debug message to a logger of some sort. I really like how Tire does it; logging a curl command that you can copy/paste and use directly. Also, outputting the status code of the response would be good, too ...

[200] curl -XGET 'http://localhost:9200/index/_search' -d '...'

Not sure how to handle large bodies and what not. And I don't think we need to log responses.

cc @github/search

@wfarr
Copy link

wfarr commented Apr 20, 2013

IMO, http requests with bodies should be a level of logging but likely not the default.

As far as logging request bodies goes, strip new lines and compress multiple spaces into a single space.

On Sat, Apr 20, 2013 at 6:20 PM, Tim Pease [email protected]
wrote:

We need to add some logging to the Client class. For each request the client makes, we should output a detailed debug message to a logger of some sort. I really like how Tire does it; logging a curl command that you can copy/paste and use directly. Also, outputting the status code of the response would be good, too ...

[200] curl -XGET 'http://localhost:9200/index/_search' -d '...'

Not sure how to handle large bodies and what not. And I don't think we need to log responses.

cc @github/search

Reply to this email directly or view it on GitHub:
#5

@grantr
Copy link
Contributor

grantr commented Apr 22, 2013

Maybe the client can take a :verbose level or :trace => true option that increases the logging level to include requests and responses?

@grantr
Copy link
Contributor

grantr commented Apr 22, 2013

We can't require activesupport for the low level client, but at a higher level we should use ActiveSupport::Notifications and a LogSubscriber.

@wfarr
Copy link

wfarr commented Apr 22, 2013

I'd much prefer to keep it in UNIX convention and offer info, error, debug, etc log levels.

On Mon, Apr 22, 2013 at 11:05 AM, Grant Rodgers [email protected]
wrote:

Maybe the client can take a :verbose level or :trace => true option that increases the logging level to include requests and responses?

Reply to this email directly or view it on GitHub:
#5 (comment)

@grantr
Copy link
Contributor

grantr commented Aug 27, 2013

Picking this back up.

@TwP do you like the way tire has a module-level logger instance? Should we add Elastomer.logger? Since you are well versed in all things logging I figure you'll probably know best how to add lightweight logging to a library.

@TwP
Copy link
Contributor Author

TwP commented Aug 28, 2013

Given ∞ time, I would much rather use https://github.com/TwP/polylog

The idea behind polylog is that each library requests a logging instance from Polylog, and the application then configures Polylog to hand out the type of logger that it wants to use. So you only need to configure a logger in one location. Every library then grabs that logger and uses it.

This is akin to SLF4J, but with a much more pronounceable name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants