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

Backing off log.error() causes multiple issues on Sentry #88

Closed
henryl opened this issue Jun 1, 2018 · 7 comments
Closed

Backing off log.error() causes multiple issues on Sentry #88

henryl opened this issue Jun 1, 2018 · 7 comments

Comments

@henryl
Copy link

henryl commented Jun 1, 2018

Like many people, we use Sentry for exception logging. The backoff library you guys are using will log messages that look like:

Backing off _connect(...) for 0.7s (requests.exceptions.ReadTimeout: HTTPSConnectionPool(host='st...
Backing off _connect(...) for 1.0s (requests.exceptions.ReadTimeout: HTTPSConnectionPool(host='st...
...

Each of these log messages counts as separate issue. On a recent LD outage, we had thousands of these issues and events inside of Sentry, which was really annoying.

@eli-darkly
Copy link
Contributor

Do I understand correctly that the issue is that it's using logger.error() rather than a lower level, like logger.warning()?

@henryl
Copy link
Author

henryl commented Jun 4, 2018

@eli-darkly i think it's appropriate for it to log an error, but because the string includes the backoff interval (ie. 0.7s) it will generate unique issues on sentry. If there's a way to provide a static message, that would solve this.

@eli-darkly
Copy link
Contributor

Ah, got it. Yes, that should be doable - we can use a static message for the Error line, and add a Debug line that includes the backoff delay if someone wants to know about that.

@eli-darkly
Copy link
Contributor

Sorry about the delay in fixing this. We're about to release a fix, but I just wanted to check that this will work better for your purposes. The proposed fix is, for each reconnect attempt, instead of a single ERROR-level line that includes the delay time, there will be two lines: an ERROR-level message that says only "Streaming connection failed, will attempt to restart", followed by an INFO-level message that says "Will reconnect after delay of ____s". Does that sound good?

@henryl
Copy link
Author

henryl commented Aug 3, 2018

That sounds perfect. Thanks.

@eli-darkly
Copy link
Contributor

OK, this change is in the 6.2.0 release, available now.

eli-darkly added a commit that referenced this issue Jan 31, 2019
1. add test for whether our package is valid
@woodmicha
Copy link

Is there a way to filter these using before_send? I to have loads of these due to the way we back off due to no message being available in the queue. My messages aren't the errors, they read like this: "[Giving up get_msg_backoff(...) after 10 tries ([])]

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