Skip to content

Commit

Permalink
Session tracking (#411)
Browse files Browse the repository at this point in the history
* Added session tracking basics to notifier

* Added session config to config

* Added session tracking mechanisms

* Send bugsnag properties in headers

* Added backoff to delivery

* Minor updates and fixes

* Added tests for session-tracker

* Ensured success code is correctly checked before backoff

* Added hook to rails

* Ensured deliver doesn't send empty sessions

* Replaced rails sessions with rack to service both

* Complete re-do of backoff throttling

* Changed success criteria on sessions

* Added checks on asynchronous delivery

* Updated tests

* Added maximum amount of sessions to be sent at one time

* Added final attempt to send sessions at exit

* Style update

* Changed to sessionCounts

* Registered final at_exit block to send sessions

* Updated tests

* Fixed config ref

* Ensured thread sessions accessed through accessor

* Refactored some backoff functionality into functions

* Fixed issue with accessing get/set session

* Fixed issue with get_current_session calls

* Added session-tracking to rails-51 example

* Removed unnecessary info

* Added fallback to send sessions every 5 minutes

* Ensured deliver_fallback only terminated if exists

* Ensured headers are backwards compatible

* v6.3.0.beta.0
  • Loading branch information
Cawllec authored Jan 4, 2018
1 parent 02c5a21 commit bc1912a
Show file tree
Hide file tree
Showing 20 changed files with 601 additions and 97 deletions.
3 changes: 2 additions & 1 deletion example/rails-51/config/initializers/bugsnag.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Bugsnag.configure do |config|
config.api_key = "YOUR_API_KEY_HERE"
config.api_key = "YOUR_API_KEY"
config.track_sessions = true
end
12 changes: 10 additions & 2 deletions lib/bugsnag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require "bugsnag/report"
require "bugsnag/cleaner"
require "bugsnag/helpers"
require "bugsnag/session_tracker"

require "bugsnag/delivery"
require "bugsnag/delivery/synchronous"
Expand Down Expand Up @@ -38,6 +39,8 @@ def configure
configuration.warn("No valid API key has been set, notifications will not be sent")
@key_warning = true
end

session_tracker.config = configuration
end

# Explicitly notify of an exception
Expand Down Expand Up @@ -113,8 +116,8 @@ def notify(exception, auto_notify=false, &block)

# Deliver
configuration.info("Notifying #{configuration.endpoint} of #{report.exceptions.last[:errorClass]}")
payload_string = ::JSON.dump(Bugsnag::Helpers.trim_if_needed(report.as_json))
Bugsnag::Delivery[configuration.delivery_method].deliver(configuration.endpoint, payload_string, configuration)
options = {:headers => report.headers, :trim_payload => true}
Bugsnag::Delivery[configuration.delivery_method].deliver(configuration.endpoint, report.as_json, configuration, options)
end
end

Expand All @@ -124,6 +127,11 @@ def configuration
@configuration || LOCK.synchronize { @configuration ||= Bugsnag::Configuration.new }
end

def session_tracker
@session_tracker = nil unless defined?(@session_tracker)
@session_tracker || LOCK.synchronize { @session_tracker ||= Bugsnag::SessionTracker.new(configuration)}
end

# Allow access to "before notify" callbacks
def before_notify_callbacks
Bugsnag.configuration.request_data[:before_callbacks] ||= []
Expand Down
9 changes: 8 additions & 1 deletion lib/bugsnag/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require "bugsnag/middleware/ignore_error_class"
require "bugsnag/middleware/suggestion_data"
require "bugsnag/middleware/classify_error"
require "bugsnag/middleware/session_data"

module Bugsnag
class Configuration
Expand All @@ -22,7 +23,7 @@ class Configuration
attr_accessor :app_type
attr_accessor :meta_data_filters
attr_accessor :endpoint
attr_accessor :logger
attr_accessor :logger
attr_accessor :middleware
attr_accessor :internal_middleware
attr_accessor :proxy_host
Expand All @@ -32,10 +33,13 @@ class Configuration
attr_accessor :timeout
attr_accessor :hostname
attr_accessor :ignore_classes
attr_accessor :track_sessions
attr_accessor :session_endpoint

API_KEY_REGEX = /[0-9a-f]{32}/i
THREAD_LOCAL_NAME = "bugsnag_req_data"
DEFAULT_ENDPOINT = "https://notify.bugsnag.com"
DEFAULT_SESSION_ENDPOINT = "https://sessions.bugsnag.com"

DEFAULT_META_DATA_FILTERS = [
/authorization/i,
Expand All @@ -57,6 +61,8 @@ def initialize
self.hostname = default_hostname
self.timeout = 15
self.notify_release_stages = nil
self.track_sessions = false
self.session_endpoint = DEFAULT_SESSION_ENDPOINT

# SystemExit and Interrupt are common Exception types seen with successful
# exits and are not automatically reported to Bugsnag
Expand All @@ -81,6 +87,7 @@ def initialize
self.internal_middleware.use Bugsnag::Middleware::IgnoreErrorClass
self.internal_middleware.use Bugsnag::Middleware::SuggestionData
self.internal_middleware.use Bugsnag::Middleware::ClassifyError
self.internal_middleware.use Bugsnag::Middleware::SessionData

self.middleware = Bugsnag::MiddlewareStack.new
self.middleware.use Bugsnag::Middleware::Callbacks
Expand Down
113 changes: 107 additions & 6 deletions lib/bugsnag/delivery/synchronous.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@
module Bugsnag
module Delivery
class Synchronous
HEADERS = {"Content-Type" => "application/json"}
BACKOFF_THREADS = {}
BACKOFF_REQUESTS = {}
BACKOFF_LOCK = Mutex.new

class << self
def deliver(url, body, configuration)
def deliver(url, body, configuration, options={})
begin
response = request(url, body, configuration)
response = request(url, body, configuration, options)
configuration.debug("Request to #{url} completed, status: #{response.code}")
success = options[:success] || '200'
if options[:backoff] && !(response.code == success)
backoff(url, body, configuration, options)
end
rescue StandardError => e
# KLUDGE: Since we don't re-raise http exceptions, this breaks rspec
raise if e.class.to_s == "RSpec::Expectations::ExpectationNotMetError"
Expand All @@ -22,9 +28,14 @@ def deliver(url, body, configuration)

private

def request(url, body, configuration)
def request(url, body, configuration, options)
uri = URI.parse(url)

if options[:trim_payload]
body = Bugsnag::Helpers.trim_if_needed(body)
end
payload = ::JSON.dump(body)

if configuration.proxy_host
http = Net::HTTP.new(uri.host, uri.port, configuration.proxy_host, configuration.proxy_port, configuration.proxy_user, configuration.proxy_password)
else
Expand All @@ -39,14 +50,104 @@ def request(url, body, configuration)
http.ca_file = configuration.ca_file if configuration.ca_file
end

request = Net::HTTP::Post.new(path(uri), HEADERS)
request.body = body
headers = options.key?(:headers) ? options[:headers] : {}
headers.merge!(default_headers)

request = Net::HTTP::Post.new(path(uri), headers)
request.body = payload

http.request(request)
end

def backoff(url, body, configuration, options)
# Ensure we have the latest configuration for making these requests
@latest_configuration = configuration

BACKOFF_LOCK.lock
begin
# Define an exit function once to handle outstanding requests
@registered_at_exit = false unless defined?(@registered_at_exit)
if !@registered_at_exit
@registered_at_exit = true
at_exit do
backoff_exit
end
end
if BACKOFF_REQUESTS[url] && !BACKOFF_REQUESTS[url].empty?
last_request = BACKOFF_REQUESTS[url].last
new_body_length = ::JSON.dump(body).length
old_body_length = ::JSON.dump(last_request[:body]).length
if new_body_length + old_body_length >= Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
BACKOFF_REQUESTS[url].push({:body => body, :options => options})
else
Bugsnag::Helpers::deep_merge!(last_request, {:body => body, :options => options})
end
else
BACKOFF_REQUESTS[url] = [{:body => body, :options => options}]
end
if !(BACKOFF_THREADS[url] && BACKOFF_THREADS[url].status)
spawn_backoff_thread(url)
end
ensure
BACKOFF_LOCK.unlock
end
end

def backoff_exit
# Kill existing threads
BACKOFF_THREADS.each do |url, thread|
thread.exit
end
# Retry outstanding requests once, then exit
BACKOFF_REQUESTS.each do |url, requests|
requests.map! do |req|
response = request(url, req[:body], @latest_configuration, req[:options])
success = req[:options][:success] || '200'
response.code == success
end
requests.reject! { |i| i }
@latest_configuration.warn("Requests to #{url} finished, #{requests.size} failed")
end
end

def spawn_backoff_thread(url)
new_thread = Thread.new(url) do |url|
interval = 2
while BACKOFF_REQUESTS[url].size > 0
sleep(interval)
interval = interval * 2
interval = 600 if interval > 600
BACKOFF_LOCK.lock
begin
BACKOFF_REQUESTS[url].map! do |req|
response = request(url, req[:body], @latest_configuration, req[:options])
success = req[:options][:success] || '200'
if response.code == success
@latest_configuration.debug("Request to #{url} completed, status: #{response.code}")
false
else
req
end
end
BACKOFF_REQUESTS[url].reject! { |i| !i }
ensure
BACKOFF_LOCK.unlock
end
end
end
BACKOFF_THREADS[url] = new_thread
end

def path(uri)
uri.path == "" ? "/" : uri.path
end

def default_headers
{
"Content-Type" => "application/json",
"Bugsnag-Sent-At" => Time.now().utc().strftime('%Y-%m-%dT%H:%M:%S')
}
end
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/bugsnag/delivery/thread_queue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ThreadQueue < Synchronous
MUTEX = Mutex.new

class << self
def deliver(url, body, configuration)
def deliver(url, body, configuration, options={})
@configuration = configuration

start_once!
Expand All @@ -19,7 +19,7 @@ def deliver(url, body, configuration)
end

# Add delivery to the worker thread
@queue.push proc { super(url, body, configuration) }
@queue.push proc { super(url, body, configuration, options) }
end

private
Expand Down
24 changes: 24 additions & 0 deletions lib/bugsnag/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,30 @@ def self.trim_if_needed(value)
remove_metadata_from_events(reduced_value)
end

def self.deep_merge(l_hash, r_hash)
l_hash.merge(r_hash) do |key, l_val, r_val|
if l_val.is_a?(Hash) && r_val.is_a?(Hash)
deep_merge(l_val, r_val)
elsif l_val.is_a?(Array) && r_val.is_a?(Array)
l_val.concat(r_val)
else
r_val
end
end
end

def self.deep_merge!(l_hash, r_hash)
l_hash.merge!(r_hash) do |key, l_val, r_val|
if l_val.is_a?(Hash) && r_val.is_a?(Hash)
deep_merge(l_val, r_val)
elsif l_val.is_a?(Array) && r_val.is_a?(Array)
l_val.concat(r_val)
else
r_val
end
end
end

private

TRUNCATION_INFO = '[TRUNCATED]'
Expand Down
1 change: 1 addition & 0 deletions lib/bugsnag/integrations/rack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def initialize(app)
def call(env)
# Set the request data for bugsnag middleware to use
Bugsnag.configuration.set_request_data(:rack_env, env)
Bugsnag.session_tracker.create_session

begin
response = @app.call(env)
Expand Down
1 change: 1 addition & 0 deletions lib/bugsnag/integrations/rails/controller_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ def self.included(base)
end

module ClassMethods

private
def before_bugsnag_notify(*methods, &block)
_add_bugsnag_notify_callback(:before_callbacks, *methods, &block)
Expand Down
21 changes: 21 additions & 0 deletions lib/bugsnag/middleware/session_data.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module Bugsnag::Middleware
class SessionData
def initialize(bugsnag)
@bugsnag = bugsnag
end

def call(report)
session = Bugsnag::SessionTracker.get_current_session
unless session.nil?
if report.unhandled
session[:events][:unhandled] += 1
else
session[:events][:handled] += 1
end
report.session = session
end

@bugsnag.call(report)
end
end
end
15 changes: 12 additions & 3 deletions lib/bugsnag/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ class Report

MAX_EXCEPTIONS_TO_UNWRAP = 5

CURRENT_PAYLOAD_VERSION = "2"
CURRENT_PAYLOAD_VERSION = "4.0"

attr_reader :unhandled
attr_accessor :api_key
attr_accessor :app_type
attr_accessor :app_version
Expand All @@ -31,6 +32,7 @@ class Report
attr_accessor :meta_data
attr_accessor :raw_exceptions
attr_accessor :release_stage
attr_accessor :session
attr_accessor :severity
attr_accessor :severity_reason
attr_accessor :user
Expand Down Expand Up @@ -92,7 +94,7 @@ def as_json
},
exceptions: exceptions,
groupingHash: grouping_hash,
payloadVersion: CURRENT_PAYLOAD_VERSION,
session: session,
severity: severity,
severityReason: severity_reason,
unhandled: @unhandled,
Expand All @@ -108,7 +110,6 @@ def as_json

# return the payload hash
{
:apiKey => api_key,
:notifier => {
:name => NOTIFIER_NAME,
:version => NOTIFIER_VERSION,
Expand All @@ -118,6 +119,14 @@ def as_json
}
end

def headers
{
"Bugsnag-Api-Key" => api_key,
"Bugsnag-Payload-Version" => CURRENT_PAYLOAD_VERSION,
"Bugsnag-Sent-At" => Time.now().utc().strftime('%Y-%m-%dT%H:%M:%S')
}
end

def ignore?
@should_ignore
end
Expand Down
Loading

0 comments on commit bc1912a

Please sign in to comment.