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

Allow to set custom logger #9

Closed
wants to merge 2 commits into from

Conversation

sashazykov
Copy link
Contributor

Use logger to log errors and allow to set a custom logger. Also, fixes a bug that resulted in writing new line characters to stdout on successful responses from zipkin.

@@ -38,11 +39,11 @@ def emit_batch(spans)
request.body = JSON.dump(spans)
response = http.request(request)

if response.code != 202
STDERR.puts(response.body)
if response.code.to_s != '202'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we convering response code to string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Response code is a string in my environment, not sure about the other, so decided to convert it to string before checking.

This was the reason of random new line characters in logs, because the gem logged empty body on successful responses.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://ruby-doc.org/stdlib-2.4.0/libdoc/net/http/rdoc/Net/HTTPResponse.html

The HTTP result code string. For example, ‘302’. You can also determine the response type by examining which response subclass the response object is an instance of.

Looks like you're right. Can you please remove #to_s from response.code but lets keep 202 as a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://ruby-doc.org/stdlib-2.5.0/libdoc/net/http/rdoc/Net/HTTPResponse.html

The HTTP result code string. For example, '302'. You can also determine the response type by examining which response subclass the response object is an instance of.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I've also seen these blank lines, but didn't have time to look much into it. Thanks for the fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it.

@indrekj indrekj closed this in 33002b5 Apr 4, 2018
@indrekj
Copy link
Member

indrekj commented Apr 4, 2018

Split it into two commits for better visibility c81294c and 2e69154 . Also released it as 1.1.0.

Merged. Thank you.

@sashazykov
Copy link
Contributor Author

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants