Implement Sentry integration #16
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0xdiba
approved these changes
Aug 3, 2020
| @@ -19,6 +19,7 @@ Gem::Specification.new do |s| | |||
| end | |||
|
|
|||
| s.add_dependency "redis" | |||
| s.add_dependency "sentry-raven" | |||
There was a problem hiding this comment.
Just a note that although raven-ruby has only one dependency at the moment as far as I can tell, this could create unnecessary issues with gem incompatibilities when the integration is not used. So perhaps in the future the whole integration along with dependencies can become opt-in?
There was a problem hiding this comment.
Maybe we could do this by not adding sentry-raven as a dependency and requiring the user to explicitly add it to their Gemfile. That said, I believe we should hold on until this pops up as an issue.
|
@georgepsarakis I actually removed the Sentry event of the "New/untimed job" because it was too spammy and caused a flood of events. It makes more sense to report that via StatsD (#2). |
georgepsarakis
approved these changes
Aug 6, 2020
This patch adds support for Workers to emit various RSpecQ internal events to a Sentry[1] instance. Reporting to Sentry is opt-in and is enabled by setting the SENTRY_DSN environment variable (see https://github.com/getsentry/raven-ruby#raven-only-runs-when-sentry_dsn-is-set). If SENTRY_DSN is not set this is a noop. Events emitted: - [error] the spec file split command failed; slow spec files files will be scheduled as a whole so build times will be negatively impacted - [warning] no previous timings were found in Redis; scheduling will be random so build times will be negatively impacted - [info] slow files were detected and will be split, including the actual filenames The above are still printed to the standard output of the worker. Closes #8 [1] https://sentry.io
Usually RSpec prints errors to stderr, so we have to grab it too in order to display a helpful error message in case this command fails. Ideally we'd do this with popen3, but this is good enough for now, considering #6.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds support for Workers to emit various RSpecQ internal events to
a Sentry[1] instance.
Reporting to Sentry is opt-in and is enabled by setting the SENTRY_DSN
environment variable (see
https://github.com/getsentry/raven-ruby#raven-only-runs-when-sentry_dsn-is-set).
If SENTRY_DSN is not set this is a noop.
Events emitted:
be scheduled as a whole so build times will be negatively impacted
so build times will be negatively impacted
actual filenames
The above are still printed to the standard output of the worker.
Closes #8
[1] https://sentry.io