Culprits -> Transactions

[nateberkopec]

Close #667.

This also allows anyone to set a culprit by setting culprit in the context object.

[nateberkopec]

Intended to be the "ruby" version of getsentry/raven-python#768

[nateberkopec]

We're gonna need a stack as well for things like someone running Sidekiq inline (transaction top-level is the Rails controller action, then lowest-level will be the Sidekiq job.)

[nateberkopec]

This PR also fixes some bugs with Sidekiq:

• Fixes transaction name.
• Adds some context about worker class name and queue.

[SirRawlins]

@nateberkopec this PR looks to have broken something for me. I have my own around_action callbacks defined within my application.

# Wrap all actions in delayed touch.
prepend_around_action :delay_touching

# Delay touching to once per
# transaction to prevent the noise
def delay_touching
    # Use the gem to wrap every action within the app.
    ActiveRecord::Base.delay_touching { yield }
end

This worked up until this PR was merged in, all builds after that date result in an error being raised by my own around_action.

LocalJumpError:
    no block given (yield)

Is the new around_action behavior you've defined augmenting the request in a way it shouldn't? Meaning that the block has been stripped by the time it arrives in my own handler?

[nateberkopec]

Yeah I'm seeing the same on Codetriage. Working on it.

[bjeanes]

I think it's also worth noting that this change broke compatibility with Rails 3 (I think). The README states that it works with Rails 4.2+, so I guess this is fair game, but it was working well enough in Rails 3, even so.

Nonetheless, for posterity and future readers, I thought it noteworthy. Rails 3.x will get the following exception:

NoMethodError: undefined method `around_action' for ActionController::Base:Class

[nateberkopec]

🤷‍♂️

[ragesoss]

The change from :culprit to :transaction was a breaking change for my app. I only discovered it in production, as my test that also exercises the call to Raven.capture_message didn't raise an error in the test environment.

Is there a way to configure Raven so that I can be confident that I'm using the interface correctly in tests? It looks like it will take any hash at all without complaint in test, but will error with unexpected keys / keywords in production.

[nateberkopec]

@ragesoss Good point. In my head this wasn't a breaking change because the Sentry server accepts culprit and transaction, but if you're manually putting a culprit on the event and then we add a transaction attribute, Sentry will ignore the former.

Are you saying it raised an exception in production? I don't see how that would occur.

[ragesoss]

@nateberkopec Here's what the call site was (the old version): WikiEducationFoundation/WikiEduDashboard@ccff70d

And the error (which was itself caught by Sentry and reported):

NoMethodError: undefined method `culprit=' for #<Raven::Event:0x007f8008d9ee20>
  from raven/event.rb:39:in `public_send'
  from raven/event.rb:39:in `block in initialize'
  from raven/event.rb:39:in `each_pair'
  from raven/event.rb:39:in `initialize'
  from raven/event.rb:83:in `new'
  from raven/event.rb:83:in `from_message'
  from raven/instance.rb:114:in `capture_type'
  from lib/wiki_edits.rb:35:in `notify_untrained'
  from app/workers/notify_untrained_users_worker.rb:16:in `perform'
  from sidekiq/processor.rb:188:in `execute_job'
  from sidekiq/processor.rb:170:in `block (2 levels) in process'
  from sidekiq/middleware/chain.rb:128:in `block in invoke'
  from new_relic/agent/instrumentation/sidekiq.rb:33:in `block in call'
  from new_relic/agent/instrumentation/controller_instrumentation.rb:368:in `perform_action_with_newrelic_trace'
  from new_relic/agent/instrumentation/sidekiq.rb:29:in `call'
  from sidekiq/middleware/chain.rb:130:in `block in invoke'
  from raven/integrations/sidekiq.rb:9:in `call'
  from sidekiq/middleware/chain.rb:130:in `block in invoke'
  from sidekiq_unique_jobs/lock/until_executed.rb:63:in `after_yield_yield'
  from sidekiq_unique_jobs/lock/until_executed.rb:20:in `execute'
  from sidekiq_unique_jobs/server/middleware.rb:18:in `call'
  from sidekiq/middleware/chain.rb:130:in `block in invoke'
  from sidekiq/middleware/chain.rb:133:in `invoke'
  from sidekiq/processor.rb:169:in `block in process'
  from sidekiq/processor.rb:141:in `block (6 levels) in dispatch'
  from sidekiq/job_retry.rb:97:in `local'
  from sidekiq/processor.rb:140:in `block (5 levels) in dispatch'
  from sidekiq/rails.rb:42:in `block in call'
  from active_support/execution_wrapper.rb:85:in `wrap'
  from active_support/reloader.rb:68:in `block in wrap'
  from active_support/execution_wrapper.rb:85:in `wrap'
  from active_support/reloader.rb:67:in `wrap'
  from sidekiq/rails.rb:41:in `call'
  from sidekiq/processor.rb:136:in `block (4 levels) in dispatch'
  from sidekiq/processor.rb:204:in `stats'
  from sidekiq/processor.rb:131:in `block (3 levels) in dispatch'
  from sidekiq/job_logger.rb:7:in `call'
  from sidekiq/processor.rb:130:in `block (2 levels) in dispatch'
  from sidekiq/job_retry.rb:72:in `global'
  from sidekiq/processor.rb:129:in `block in dispatch'
  from sidekiq/logging.rb:44:in `with_context'
  from sidekiq/logging.rb:38:in `with_job_hash_context'
  from sidekiq/processor.rb:128:in `dispatch'
  from sidekiq/processor.rb:168:in `process'
  from sidekiq/processor.rb:85:in `process_one'
  from sidekiq/processor.rb:73:in `run'
  from sidekiq/util.rb:16:in `watchdog'
  from sidekiq/util.rb:25:in `block in safe_thread

[nateberkopec]

Ah, crap. You're right. I should have left an alias in. Sorry about that.

If you want errors like this to surface in your test suite, Raven needs to be configured to actually capture events in that environment. Of course you probably don't want messages and stuff from the test environment to show up on your Sentry dashboard, so this may be a good time to use the dummy transport, which does everything except actually send the event anywhere.

Here's an example, which is how we test Raven in the gem:

require "raven/transports/dummy"

Raven.configure do |config|
  config.dsn = "dummy:https://12345:[email protected]/sentry/42"
  config.encoding = "json"
  logger = Logger.new(nil)
  config.logger = logger
end

You can then make assertions against the state of the dummy transport:

evt = Raven.client.transport.events.first[1]
assert_equal "test", JSON.parse(evt)["environment"]

[ragesoss]

@nateberkopec awesome, thanks! I did it in the Raven initializer, like this: WikiEducationFoundation/WikiEduDashboard@a785d76

[ragesoss]

Whoops. Just in case anyone reading this issue tries to follow that example... dummy: transport doesn't work for raven-js. WikiEducationFoundation/WikiEduDashboard@794f225

[nateberkopec]

Ah yeah, dummy is just a thing I implemented in the Ruby client, it isn't a standard part of the client SDK (though maybe it should be?)