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

Report initialization errors to Sentry before exiting #172

Merged
merged 1 commit into from
Dec 19, 2017

Conversation

ChimeraCoder
Copy link
Contributor

@ChimeraCoder ChimeraCoder commented Jun 12, 2017

Summary

If we encounter errors when initializing Veneur, we should send those to Sentry (if possible), rather than (just) logging them.

Motivation

Monitoring Veneur failures is a fun problem - we can't use statsd, because Veneur won't be around for another 10 seconds to report its error (if it is even capable of reporting it in the first place). We don't have Sentry configured in cmd/veneur, and doing so would be pretty inconvenient for a rather dubious reward.

The tests here are currently failing, and will continue to do so until #133 is fixed and merged.

An alternative approach would be to export the Raven client from the Veneur package so that cmd/veneur is able to report the error. I'm doing that separately, but I think that's suboptimal, because it means we're essentially duplicating the logic in two places.

Test plan

Rollout/monitoring/revert plan

r? @cory-stripe
cc @stripe/observability

@cory-stripe
Copy link
Contributor

👍

Nice! Unsure why this is failing as the builds all expired. :)

@aditya-stripe aditya-stripe force-pushed the aditya-initialization-monitor branch from 7a034d9 to 1d5be79 Compare June 21, 2017 21:46
@stripe-ci
Copy link

Gerald Rule: Copy Observability on Veneur and Unilog pull requests

cc @stripe/observability
cc @stripe/observability-stripe

@aditya-stripe aditya-stripe force-pushed the aditya-initialization-monitor branch from b2aaf53 to c13923c Compare December 19, 2017 16:37
@ChimeraCoder
Copy link
Contributor Author

re-r? @cory-stripe

@cory-stripe
Copy link
Contributor

cory-stripe commented Dec 19, 2017

👍

(assuming builds work)

@ChimeraCoder ChimeraCoder merged commit 9c4a618 into master Dec 19, 2017
@ChimeraCoder ChimeraCoder deleted the aditya-initialization-monitor branch December 19, 2017 16:48
arielshaqed pushed a commit to binaris/veneur that referenced this pull request Mar 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants