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

Ability to adjust log levels? #27

Open
osyoyu opened this issue Jan 30, 2019 · 4 comments
Open

Ability to adjust log levels? #27

osyoyu opened this issue Jan 30, 2019 · 4 comments

Comments

@osyoyu
Copy link

osyoyu commented Jan 30, 2019

I'm working on a webapp which utilizes garage_client to communicate with another backend server, and would like to see garage_client requests in the logs.

GarageClient::Client#new takes the :verbose option, which is passed down to the underlying Faraday object, but its output is a bit noisy for the above usage.

So, my question is: could garage_client have an option to adjust the log level?

I'm thinking of something like aws-sdk-ruby, which has a configurable logger (e.g. AWS.config(:log_level => :debug)), which could even take a format string like this (docs):

pattern = '[REQUEST :http_status_code] :service :operation :duration'
formatter = AWS::Core::LogFormatter.new(pattern)

@cookpad/dev-infra What do you think?

@osyoyu
Copy link
Author

osyoyu commented Jan 30, 2019

Newer versions of Faraday::Response::Logger accept Logger instances (https://github.com/lostisland/faraday/blob/c80cf86c5a09be0f0cdc3dd00d1c85390fe55d5b/lib/faraday/response/logger.rb#L9).
Maybe this issue could be partially solved by just allowing GarageClient::Client#new to accept logger instances (which have logger levels set), and just pass it down to Faraday::Response::Logger (which is already used in verbose mode).


Printing bodies of POST requests could also be useful, and Faraday::Response::Logger provides that when {bodies: true} is passed to its options. Although, accepting that option hash through GarageClient::Client#new would be bit complex, and destroy encapsulation. No good ideas for that in the current implementation.

@taiki45
Copy link
Contributor

taiki45 commented Jan 31, 2019

GarageClient::Client#new takes the :verbose option, which is passed down to the underlying Faraday object, but its output is a bit noisy for the above usage.

Could you give us more detail of the trouble point? I'm not sure why that verbose option doesn't work for your requirement.

@osyoyu
Copy link
Author

osyoyu commented Jan 31, 2019

The verbose option prints something like this:

[INFO] get https://example.com/v1/users?fields=id
[DEBUG] request: Accept: "application/json"
User-Agent: "garage_client 2.4.2 myapp"
[INFO] Status: 200
[DEBUG] response: date: "Mon, 28 Jan 2019 23:40:27 GMT"
connection: "close"
server: "h2o/2.0.6"
content-type: "application/json; charset=utf-8"
vary: "Accept-Encoding"
status: "200 OK"
link: "</v1/users?fields=id&page=2&per_page=20>; rel=\"next\"; page=\"2\""
cache-control: "public, max-age=0, s-maxage=600"
x-unicorn-pid: "29900"
x-amzn-trace-id: "Root=1-xxxxxxx-xxxxxxxxxx;Sampled=1"
x-chst: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
etag: "W/\"xxxxxxxxxxxxxxxxxxxxxxxxxxx\""
x-request-id: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
x-runtime: "0.068630"
transfer-encoding: "chunked"

While this information is "verbose", we don't need all of this in our logs; so, I'd like to have control over this.

We have patched GarageClient in our application to make it show compact logs like this:

[GarageClient 200] POST https://example.com/v1/users

@taiki45
Copy link
Contributor

taiki45 commented Jan 31, 2019

Got it, I also think current "verbose option" is too much for most of usec ases, so cloud we just change current GarageClient's "verbose option" behavior instead of adding more extensible API above? I don't think current "verbose option" is not good default behavior, so we can change this (IMO).

If we face additional more verbose requirement later, we can add that with extensible API. I think default behaviors should be simple and fit to most of use cases or basic requirements.

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

2 participants