Add exception_notification gem #1740

monfresh · 2017-10-13T17:50:37Z

Why: To allow us to receive real-time notifications via email
any time an error occurs in production. The gem also has support for
other notification channels, such as Slack. It also captures Sidekiq
errors out of the box.

Using this gem is a lot easier and cheaper than maintaining
a self-hosted version of Sentry or Bugsnag, for example.

Visit the gem's repo for more configuration options:
https://github.com/smartinez87/exception_notification

monfresh · 2017-10-13T17:52:01Z

Since this adds a new entry to application.yml, I believe it also requires a change in the devops repo, but I'm not sure if that's still the case with the improvements @zachmargolis and @brodygov have made that make it easier to update application.yml.

monfresh · 2017-10-13T17:58:44Z

Here is what the email looks like:

monfresh · 2017-10-13T17:59:16Z

And here is what is included inside in the email:

An ActionController::InvalidAuthenticityToken occurred in registrations#create:

ActionController::InvalidAuthenticityToken
lib/headers_filter.rb:13:in `call'


-------------------------------
Request:
-------------------------------

* URL : http://localhost:3000/sign_up/enter_email
* HTTP Method: POST
* IP address : ::1
* Parameters : {"utf8"=>"✓", "authenticity_token"=>"/VZGRNtAgisiBAx6dwNRYYdc17ssiMnVfmKuZ51VcYUvjD5By91/20m2W4jBtBTRsYO70cbeN06hytP7bPA==", "user"=>"[FILTERED]", "commit"=>"Submit", "controller"=>"sign_up/registrations", "action"=>"create"}
* Timestamp : 2017-10-13 17:56:37 UTC
* Server : FCOH2J-04QG3QK
* Rails root : /Users/moncefbelyamani/projects/18f/identity
* Process: 18194

-------------------------------
Session:
-------------------------------

* session id: "0b993bd5882af41b3bb825b0c2ffac47"
* data: {}

-------------------------------
Environment:
-------------------------------

* CONTENT_LENGTH : 198
* CONTENT_TYPE : application/x-www-form-urlencoded
* GATEWAY_INTERFACE : CGI/1.2
* HTTP_ACCEPT : text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
* HTTP_ACCEPT_ENCODING : gzip, deflate, br
* HTTP_ACCEPT_LANGUAGE : en-US,en;q=0.8
* HTTP_CACHE_CONTROL : no-cache
* HTTP_CONNECTION : keep-alive
* HTTP_COOKIE : _identity-sp_session=WTFjdnI5VjRkVEVXWTJIS0VnVkR3bUZmalB2SkZQSUppQjFUK3FOeTlKem9tdVlNNW1XRCtGb21mNDk4eWJEUkNldEFueEo4akFIdGZDMmRSdER1b000M0FPcXV5b1hsVUtVSEgxelRKcnpDQlJtcEdNQ1JLTmFoS2JLT3VjR01RaElvTWIyU2EwZkY4eGtBbFVpM29kdGpJK1hVVVFZSFFFa3pSd080MGRXbVFyL2cyakdtMlI1N0ZDbEJ6TndJaUFPRVl0K1JXUGVCQ05RQVJoZEd3eTM4WFdlbHBITlZiQUQrRXUva2xjQVkzMmFPazIxQkxGWThHZ3VnNVA2dW96ZUprZTVlaktJNkhveWRzV3hFRVE9PS0tUHc4RFlqcXV5cnlOWGhJVDlKckZyQT09--0e1881e98529ae13a724beaf506125c1cbc8e294; sinatra_sp=BAh7BkkiD3Nlc3Npb25faWQGOgZFVEkiRWI0MDczYTBkNjBiNjJhYTBhZDM1%0AM2MzOTRjMjgxMDE0NTQ1YTQ1ODZiODBiMDBiMjYyZDYzZjA4MTUzNjYwNjUG%0AOwBG%0A--8b88b274812175dc17de18b4f93bef2a241c9317; ahoy_visitor=a6b15cad-6408-43c5-acc3-0a9362b14f13; ahoy_track=true; _upaya_session=90451fa75fa9039afb47411215032d98; ahoy_visit=62c4c163-4391-476c-a8d7-6fe5fc634b2e
* HTTP_DNT : 1
* HTTP_HOST : localhost:3000
* HTTP_ORIGIN : http://localhost:3000
* HTTP_PRAGMA : no-cache
* HTTP_REFERER : http://localhost:3000/sign_up/enter_email
* HTTP_UPGRADE_INSECURE_REQUESTS : 1
* HTTP_USER_AGENT : [FILTERED]
* HTTP_VERSION : HTTP/1.1
* ORIGINAL_FULLPATH : /sign_up/enter_email
* ORIGINAL_SCRIPT_NAME : 
* PATH_INFO : /sign_up/enter_email
* QUERY_STRING : 
* REMOTE_ADDR : ::1
* REQUEST_METHOD : POST
* REQUEST_PATH : /sign_up/enter_email
* REQUEST_URI : /sign_up/enter_email
* ROUTES_70249067681280_SCRIPT_NAME : 
* SCRIPT_NAME : 
* SERVER_NAME : localhost
* SERVER_PORT : 3000
* SERVER_PROTOCOL : HTTP/1.1
* SERVER_SOFTWARE : thin 1.7.2 codename Bachmanity
* action_controller.instance : #
* action_dispatch.backtrace_cleaner : #
* action_dispatch.cookies : #
* action_dispatch.cookies_digest : 
* action_dispatch.cookies_serializer : json
* action_dispatch.encrypted_cookie_salt : encrypted cookie
* action_dispatch.encrypted_signed_cookie_salt: signed encrypted cookie
* action_dispatch.http_auth_salt : http authentication
* action_dispatch.key_generator : #
* action_dispatch.logger : #
* action_dispatch.parameter_filter : [:code, :email, :idv_finance_form, :idv_phone_form, :password, :phone, :profile, :user]
* action_dispatch.redirect_filter : ["token"]
* action_dispatch.remote_ip : ::1
* action_dispatch.request.content_type : application/x-www-form-urlencoded
* action_dispatch.request.flash_hash : #
* action_dispatch.request.formats : [#]
* action_dispatch.request.parameters : {"utf8"=>"✓", "authenticity_token"=>"/VZGRNtAgisiBAx6dwNRYYdc17ssiMnVfmKuZ51VcYUvjD5By91/20m2W4jBtBTRsYO70cbeN06hytP7bPA==", "user"=>"[FILTERED]", "commit"=>"Submit", "controller"=>"sign_up/registrations", "action"=>"create"}
* action_dispatch.request.path_parameters : {:controller=>"sign_up/registrations", :action=>"create"}
* action_dispatch.request.query_parameters : {}
* action_dispatch.request.request_parameters : {"utf8"=>"✓", "authenticity_token"=>"/VZGRNtAgisiBAx6dwNRYYdc17ssiMnVfmKuZ51VcYUvjD5By91/20m2W4jBtBTRsYO70cbeN06hytP7bPA==", "user"=>"[FILTERED]", "commit"=>"Submit"}
* action_dispatch.request_id : f521d242-ec34-4625-9828-8e867b5efda6
* action_dispatch.routes : #
* action_dispatch.secret_key_base : development_secret_key_base
* action_dispatch.secret_token : 
* action_dispatch.show_detailed_exceptions : true
* action_dispatch.show_exceptions : true
* action_dispatch.signed_cookie_salt : signed cookie
* async.callback : #
* async.close : #
* http_accept_language.parser : #
* newrelic.transaction_started : true
* rack-timeout.info : #
* rack.cors : #
* rack.errors : #
* rack.input : #
* rack.multiprocess : false
* rack.multithread : false
* rack.request.cookie_hash : {"_identity-sp_session"=>"WTFjdnI5VjRkVEVXWTJIS0VnVkR3bUZmalB2SkZQSUppQjFUK3FOeTlKem9tdVlNNW1XRCtGb21mNDk4eWJEUkNldEFueEo4akFIdGZDMmRSdER1b000M0FPcXV5b1hsVUtVSEgxelRKcnpDQlJtcEdNQ1JLTmFoS2JLT3VjR01RaElvTWIyU2EwZkY4eGtBbFVpM29kdGpJK1hVVVFZSFFFa3pSd080MGRXbVFyL2cyakdtMlI1N0ZDbEJ6TndJaUFPRVl0K1JXUGVCQ0...
* rack.request.cookie_string : _identity-sp_session=WTFjdnI5VjRkVEVXWTJIS0VnVkR3bUZmalB2SkZQSUppQjFUK3FOeTlKem9tdVlNNW1XRCtGb21mNDk4eWJEUkNldEFueEo4akFIdGZDMmRSdER1b000M0FPcXV5b1hsVUtVSEgxelRKcnpDQlJtcEdNQ1JLTmFoS2JLT3VjR01RaElvTWIyU2EwZkY4eGtBbFVpM29kdGpJK1hVVVFZSFFFa3pSd080MGRXbVFyL2cyakdtMlI1N0ZDbEJ6TndJaUFPRVl0K1JXUGVCQ05RQVJoZEd3eTM4WFdlbHBITlZiQUQrRXUva2xjQVkzMmFPazIxQkxGWThHZ3VnNVA2dW96ZUprZTVlaktJNkhveWRzV3hFRVE9PS0tUHc4RFlqcXV5cnlOWGhJVDlKckZyQT09--0e1881e98529ae13a724beaf506125c1cbc8e294; sinatra_sp=BAh7BkkiD3Nlc3Npb25faWQGOgZFVEkiRWI0MDczYTBkNjBiNjJhYTBhZDM1%0AM2MzOTRjMjgxMDE0NTQ1YTQ1ODZiODBiMDBiMjYyZDYzZjA4MTUzNjYwNjUG%0AOwBG%0A--8b88b274812175dc17de18b4f93bef2a241c9317; ahoy_visitor=a6b15cad-6408-43c5-acc3-0a9362b14f13; ahoy_track=true; _upaya_session=90451fa75fa9039afb47411215032d98; ahoy_visit=62c4c163-4391-476c-a8d7-6fe5fc634b2e
* rack.request.form_hash : {"utf8"=>"✓", "authenticity_token"=>"/VZGRNtAgisiBAx6dwNRYYdc17ssiMnVfmKuZ51VcYUvjD5By91/20m2W4jBtBTRsYO70cbeN06hytP7bPA==", "user"=>"[FILTERED]", "commit"=>"Submit"}
* rack.request.form_input : #
* rack.request.form_vars : [FILTERED]
* rack.request.query_hash : {}
* rack.request.query_string : 
* rack.run_once : false
* rack.session : #
* rack.session.options : #
* rack.tempfiles : []
* rack.url_scheme : http
* rack.version : [1, 0]
* warden : Warden::Proxy:70248922638320 @config={:default_scope=>:user, :scope_defaults=>{}, :default_strategies=>{:user=>[:database_authenticatable]}, :intercept_401=>false, :failure_app=>#}

-------------------------------
Backtrace:
-------------------------------

lib/headers_filter.rb:13:in `call'

monfresh · 2017-10-13T18:01:18Z

config/initializers/exception_notification.rb

+  config.add_notifier(
+    :email,
+    email_prefix: "[#{APP_NAME} EXCEPTION] ",
+    sender_address: %("Exception Notifier" <[email protected]>),


Should we i18n any of these, or make them configurable in application.yml?

nah, doesn't seem worth it to to i18n? but it would probably be worth it to build the email address via the mail gem?

Say more. I haven't used the mail gem.

We can build this format of string programatically (which is useful if there are spaces or unicode)

require 'mail' => true Mail::Address.new('[email protected]').tap { |a| a.display_name = 'fooooo barrrr' }.to_s => "fooooo barrrr <[email protected]>"

Maybe I'm misunderstanding, but is there a problem with the way the current sender address is implemented? I tested it with SES and it works fine. In what situations would the current implementation be problematic?

In case we wanted to make the email configurable without interpolation or escaping? It's not a big deal.

zachmargolis · 2017-10-14T18:13:00Z

config/initializers/exception_notification.rb

+ExceptionNotification.configure do |config|
+  config.add_notifier(
+    :email,
+    email_prefix: "[#{APP_NAME} EXCEPTION] ",


can we add the environment in the prefix?

Somewhat out of scope, but this would also be good to do in other places where we use APP_NAME, such as OTPs and user email. It would be good to be able to distinguish environments based on the received messages.

It doesn't look like you can add the current host in the subject. We'll need to submit a pull request to the gem. The gem currently only supports adding the controller name and action, and configuring whether or not the exception appears in the subject.

Oh wait. We don't need the request to get the host name. We can read from Figaro.

Added here: 7225dc5

monfresh · 2017-10-17T04:36:37Z

config/application.yml.example

@@ -9,6 +9,7 @@
 # Figaro requires all values to be explicit strings.

 email_from: '[email protected]'
+exception_recipients: '["[email protected]", "[email protected]"]'


Should we make this a comma separated string and then use split(',') instead of JSON.parse in the initializer?

I'm inclined to say "yes".

zachmargolis · 2017-10-18T15:21:10Z

config/initializers/exception_notification.rb

+ExceptionNotification.configure do |config|
+  config.add_notifier(
+    :email,
+    email_prefix: "[#{APP_NAME} EXCEPTION - #{Figaro.env.domain_name}] ",


Still doesn't have the environment? We won't be able to tell staging apart from production?

We can use LoginGov::Hostdata.env for that

domain_name is unique per environment. In production, it is secure.login.gov. In staging, it is idp.staging.login.gov.

I agree with Zach that it may be easier to read at a glance if we have the name of the env instead of the URL.

Oh, by env, you mean the short name, like staging, dev, int?

Fixed here: 7a67adb

jmhooper · 2017-10-24T13:01:47Z

config/initializers/exception_notification.rb

+require 'exception_notification/rails'
+require 'exception_notification/sidekiq'
+
+EXCEPTION_RECIPIENTS = JSON.parse(Figaro.env.exception_recipients).freeze


This breaks if Figaro.env.exception_recipients is nil. Any thoughts on changing this up so that it works if exception_recipients is nil.

This is currently a required Figaro key, so it can't be nil.

jmhooper

Approved, but we need to make sure we've added the config before we merge.

jmhooper · 2017-10-25T16:41:37Z

@monfresh: I took the liberty of opening a devops PR to add the config https://github.com/18F/identity-devops/pull/663

monfresh · 2017-10-26T14:11:53Z

Thank you, sir!

**Why**: To allow us to receive real-time notifications via email any time an error occurs in production. The gem also has support for other notification channels, such as Slack. It also captures Sidekiq errors out of the box. Using this gem is a lot easier and cheaper than maintaining a self-hosted version of Sentry or Bugsnag, for example. Visit the gem's repo for more configuration options: https://github.com/smartinez87/exception_notification

monfresh · 2017-10-30T20:48:56Z

The devops PR has been merged. Can we merge this now, or should we wait until we've populated the config with some emails?

jmhooper · 2017-10-30T20:50:47Z

Let's add the config now lest we forget later.

monfresh requested review from jmhooper, brodygov and tbaxter-18f October 13, 2017 17:50

monfresh added the status - ready for review label Oct 13, 2017

monfresh force-pushed the mb-exception-notification branch from 5d70d7a to bfe2cc9 Compare October 13, 2017 17:58

monfresh commented Oct 13, 2017

View reviewed changes

zachmargolis reviewed Oct 14, 2017

View reviewed changes

monfresh commented Oct 17, 2017

View reviewed changes

zachmargolis reviewed Oct 18, 2017

View reviewed changes

jmhooper reviewed Oct 24, 2017

View reviewed changes

jmhooper approved these changes Oct 25, 2017

View reviewed changes

monfresh force-pushed the mb-exception-notification branch from 7a67adb to 1ce93e3 Compare October 26, 2017 14:13

monfresh merged commit c7c3662 into master Oct 31, 2017

monfresh deleted the mb-exception-notification branch October 31, 2017 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add exception_notification gem #1740

Add exception_notification gem #1740

monfresh commented Oct 13, 2017

monfresh commented Oct 13, 2017

monfresh commented Oct 13, 2017

monfresh commented Oct 13, 2017

monfresh Oct 13, 2017

zachmargolis Oct 14, 2017

monfresh Oct 17, 2017

zachmargolis Oct 17, 2017

monfresh Oct 18, 2017

zachmargolis Oct 18, 2017

zachmargolis Oct 14, 2017

brodygov Oct 15, 2017 •

edited

Loading

monfresh Oct 17, 2017 •

edited

Loading

monfresh Oct 17, 2017

monfresh Oct 17, 2017

monfresh Oct 17, 2017

monfresh Oct 17, 2017

zachmargolis Oct 18, 2017

monfresh Oct 19, 2017

jmhooper Oct 24, 2017

monfresh Oct 25, 2017

monfresh Oct 25, 2017

jmhooper Oct 24, 2017

monfresh Oct 25, 2017

jmhooper left a comment

jmhooper commented Oct 25, 2017

monfresh commented Oct 26, 2017

monfresh commented Oct 30, 2017

jmhooper commented Oct 30, 2017

Add exception_notification gem #1740

Add exception_notification gem #1740

Conversation

monfresh commented Oct 13, 2017

monfresh commented Oct 13, 2017

monfresh commented Oct 13, 2017

monfresh commented Oct 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brodygov Oct 15, 2017 • edited Loading

Choose a reason for hiding this comment

monfresh Oct 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmhooper left a comment

Choose a reason for hiding this comment

jmhooper commented Oct 25, 2017

monfresh commented Oct 26, 2017

monfresh commented Oct 30, 2017

jmhooper commented Oct 30, 2017

brodygov Oct 15, 2017 •

edited

Loading

monfresh Oct 17, 2017 •

edited

Loading