Skip to content

Commit

Permalink
Merge pull request #688 from bugsnag/configuration-context-precedence
Browse files Browse the repository at this point in the history
Give the configuration context precedence over context in integrations
  • Loading branch information
imjoehaines authored Aug 25, 2021
2 parents 1bac40a + ba1e877 commit 50759dd
Show file tree
Hide file tree
Showing 16 changed files with 109 additions and 21 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ Changelog
| [#681](https://github.com/bugsnag/bugsnag-ruby/pull/681)
* Add `on_breadcrumb` callbacks to replace `before_breadcrumb_callbacks`
| [#686](https://github.com/bugsnag/bugsnag-ruby/pull/686)
* Add `context` attribute to configuration, which will be used as the default context for events
* Add `context` attribute to configuration, which will be used as the default context for events. Using this option will disable automatic context setting
| [#687](https://github.com/bugsnag/bugsnag-ruby/pull/687)
| [#688](https://github.com/bugsnag/bugsnag-ruby/pull/688)

### Fixes

Expand Down
13 changes: 13 additions & 0 deletions lib/bugsnag/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ class Configuration
attr_accessor :vendor_path

# The default context for all future events
# Setting this will disable automatic context setting
# @return [String, nil]
attr_accessor :context

Expand Down Expand Up @@ -552,6 +553,18 @@ def remove_on_breadcrumb(callback)
@on_breadcrumb_callbacks.remove(callback)
end

##
# Has the context been explicitly set?
#
# This is necessary to differentiate between the context not being set and
# the context being set to 'nil' explicitly
#
# @api private
# @return [Boolean]
def context_set?
defined?(@context) != nil
end

# TODO: These methods can be a simple attr_accessor when they replace the
# methods they are aliasing
# NOTE: they are not aliases as YARD doesn't allow documenting the non-alias
Expand Down
2 changes: 1 addition & 1 deletion lib/bugsnag/integrations/resque.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def save

context = "#{class_name}@#{queue}"
report.meta_data.merge!({ context: context, payload: metadata })
report.context = context
report.automatic_context = context
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/bugsnag/middleware/active_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def call(report)

if data
report.add_tab(:active_job, data)
report.context = "#{data[:job_name]}@#{data[:queue]}"
report.automatic_context = "#{data[:job_name]}@#{data[:queue]}"
end

@bugsnag.call(report)
Expand Down
2 changes: 1 addition & 1 deletion lib/bugsnag/middleware/delayed_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def call(report)
payload_data = construct_job_payload(job.payload_object)

context = get_context(payload_data, job_data[:active_job])
report.context = context unless context.nil?
report.automatic_context = context unless context.nil?

job_data[:payload] = payload_data
end
Expand Down
2 changes: 2 additions & 0 deletions lib/bugsnag/middleware/exception_meta_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ def call(report)

if exception.respond_to?(:bugsnag_context)
context = exception.bugsnag_context
# note: this should set 'context' not 'automatic_context' as it's a
# user-supplied value
report.context = context if context.is_a?(String)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/bugsnag/middleware/rack_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ def call(report)
client_ip = request.ip.to_s rescue SPOOF
session = env["rack.session"]

# Set the context
report.context = "#{request.request_method} #{request.path}"
# Set the automatic context
report.automatic_context = "#{request.request_method} #{request.path}"

# Set a sensible default for user_id
report.user["id"] = request.ip
Expand Down
4 changes: 2 additions & 2 deletions lib/bugsnag/middleware/rails3_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ def call(report)
client_ip = env["action_dispatch.remote_ip"].to_s rescue SPOOF

if params
# Set the context
report.context = "#{params[:controller]}##{params[:action]}"
# Set the automatic context
report.automatic_context = "#{params[:controller]}##{params[:action]}"

# Augment the request tab
report.add_tab(:request, {
Expand Down
2 changes: 1 addition & 1 deletion lib/bugsnag/middleware/rake.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def call(report)
:arguments => task.arg_description
})

report.context ||= task.name
report.automatic_context ||= task.name
end

@bugsnag.call(report)
Expand Down
2 changes: 1 addition & 1 deletion lib/bugsnag/middleware/sidekiq.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def call(report)
sidekiq = report.request_data[:sidekiq]
if sidekiq
report.add_tab(:sidekiq, sidekiq)
report.context ||= "#{sidekiq[:msg]['wrapped'] || sidekiq[:msg]['class']}@#{sidekiq[:msg]['queue']}"
report.automatic_context ||= "#{sidekiq[:msg]['wrapped'] || sidekiq[:msg]['class']}@#{sidekiq[:msg]['queue']}"
end
@bugsnag.call(report)
end
Expand Down
25 changes: 20 additions & 5 deletions lib/bugsnag/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ class Report
# @return [Configuration]
attr_accessor :configuration

# Additional context for this report
# @return [String, nil]
attr_accessor :context

# The delivery method that will be used for this report
# @see Configuration#delivery_method
# @return [Symbol]
Expand Down Expand Up @@ -118,7 +114,7 @@ def initialize(exception, passed_configuration, auto_notify=false)
self.app_type = configuration.app_type
self.app_version = configuration.app_version
self.breadcrumbs = []
self.context = configuration.context
self.context = configuration.context if configuration.context_set?
self.delivery_method = configuration.delivery_method
self.hostname = configuration.hostname
self.runtime_versions = configuration.runtime_versions.dup
Expand All @@ -129,6 +125,25 @@ def initialize(exception, passed_configuration, auto_notify=false)
self.user = {}
end

##
# Additional context for this report
# @!attribute context
# @return [String, nil]
def context
return @context if defined?(@context)

@automatic_context
end

attr_writer :context

##
# Context set automatically by Bugsnag uses this attribute, which prevents
# it from overwriting the user-supplied context
# @api private
# @return [String, nil]
attr_accessor :automatic_context

##
# Add a new metadata tab to this notification.
#
Expand Down
2 changes: 1 addition & 1 deletion lib/bugsnag/tasks/bugsnag.rake
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace :bugsnag do
raise RuntimeError.new("Bugsnag test exception")
rescue => e
Bugsnag.notify(e) do |report|
report.context = "rake#test_exception"
report.automatic_context = "rake#test_exception"
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/integrations/rack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class Request
allow(report).to receive(:request_data).and_return({
:rack_env => rack_env
})
expect(report).to receive(:context=).with("TEST /TEST_PATH")
expect(report).to receive(:automatic_context=).with("TEST /TEST_PATH")
expect(report).to receive(:user).and_return({})

config = Bugsnag.configuration
Expand Down Expand Up @@ -185,7 +185,7 @@ class Request
allow(report).to receive(:request_data).and_return({
:rack_env => rack_env
})
expect(report).to receive(:context=).with("TEST /TEST_PATH")
expect(report).to receive(:automatic_context=).with("TEST /TEST_PATH")
expect(report).to receive(:user).and_return({})

config = Bugsnag.configuration
Expand Down
4 changes: 2 additions & 2 deletions spec/integrations/rake_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
:description => "TEST_COMMENT",
:arguments => "TEST_ARGS"
})
expect(report).to receive(:context).with(no_args)
expect(report).to receive(:context=).with("TEST_NAME")
expect(report).to receive(:automatic_context).with(no_args)
expect(report).to receive(:automatic_context=).with("TEST_NAME")

expect(callback).to receive(:call).with(report)

Expand Down
2 changes: 1 addition & 1 deletion spec/integrations/resque_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def require(path)
"class" => "class"
}
})
expect(report).to receive(:context=).with(expected_context)
expect(report).to receive(:automatic_context=).with(expected_context)
expect(Bugsnag).to receive(:notify).with(exception, true).and_yield(report)
resque.save
end
Expand Down
57 changes: 57 additions & 0 deletions spec/report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,63 @@ def gloops
}
end

it "uses automatic context if no other context has been set" do
Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report|
report.automatic_context = "automatic context"
end

expect(Bugsnag).to(have_sent_notification { |payload, _headers|
event = get_event_from_payload(payload)
expect(event["context"]).to eq("automatic context")
})
end

it "uses Configuration context even if the automatic context has been set" do
Bugsnag.configure do |config|
config.context = "configuration context"
end

Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report|
report.automatic_context = "automatic context"
end

expect(Bugsnag).to(have_sent_notification { |payload, _headers|
event = get_event_from_payload(payload)
expect(event["context"]).to eq("configuration context")
})
end

it "uses overridden context even if the automatic context has been set" do
Bugsnag.configure do |config|
config.context = "configuration context"
end

Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report|
report.context = "overridden context"
report.automatic_context = "automatic context"
end

expect(Bugsnag).to(have_sent_notification { |payload, _headers|
event = get_event_from_payload(payload)
expect(event["context"]).to eq("overridden context")
})
end

it "uses overridden context even it is set to 'nil'" do
Bugsnag.configure do |config|
config.context = nil
end

Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report|
report.automatic_context = "automatic context"
end

expect(Bugsnag).to(have_sent_notification { |payload, _headers|
event = get_event_from_payload(payload)
expect(event["context"]).to be_nil
})
end

it "uses the context from Configuration, if set" do
Bugsnag.configure do |config|
config.context = "example context"
Expand Down

0 comments on commit 50759dd

Please sign in to comment.