Skip to content

Commit

Permalink
Replace active_sentry_environments with enabled_environments
Browse files Browse the repository at this point in the history
The custom option `active_sentry_environments` was added in
#168.

However, it turns out there is already a native option for this:
`enabled_environments`. See
https://docs.sentry.io/platforms/ruby/configuration/options/

This allows us to simplify our code a fair bit.

I've also removed the stubbed `sending_allowed?` method as it was
interfering with some tests and giving a false sense of security.
We're now providing all of the necessary configuration for
the `Sentry::Configuration` instance to return `true` on its
`valid?` method, so no need for the hack.
Each config option is documented in an inline comment as otherwise
it can be quite obtuse.
  • Loading branch information
ChrisBAshton committed Jun 8, 2021
1 parent bf5f3b9 commit 945ad98
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 24 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ You can add your environment to the list of active Sentry environments like so:

```ruby
GovukError.configure do |config|
config.active_sentry_environments << "my-test-environment"
config.enabled_environments << "my-test-environment"
end
```

Expand Down
11 changes: 2 additions & 9 deletions lib/govuk_app_config/govuk_error/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,14 @@

module GovukError
class Configuration < SimpleDelegator
attr_reader :data_sync, :sentry_environment
attr_accessor :active_sentry_environments, :data_sync_excluded_exceptions
attr_reader :data_sync
attr_accessor :data_sync_excluded_exceptions

def initialize(_sentry_configuration)
super
@sentry_environment = ENV["SENTRY_CURRENT_ENV"]
@data_sync = GovukDataSync.new(ENV["GOVUK_DATA_SYNC_PERIOD"])
self.active_sentry_environments = []
self.data_sync_excluded_exceptions = []
@before_send_callbacks = [
ignore_exceptions_if_not_in_active_sentry_env,
ignore_excluded_exceptions_in_data_sync,
increment_govuk_statsd_counters,
]
Expand All @@ -26,10 +23,6 @@ def before_send=(closure)

protected

def ignore_exceptions_if_not_in_active_sentry_env
->(event, _hint) { event if active_sentry_environments.include?(sentry_environment) }
end

def ignore_excluded_exceptions_in_data_sync
lambda { |event, hint|
data_sync_ignored_error = data_sync_excluded_exceptions.any? do |exception_to_ignore|
Expand Down
2 changes: 1 addition & 1 deletion lib/govuk_app_config/govuk_error/configure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# ENV variable) where we want to capture Sentry errors. If
# `SENTRY_CURRENT_ENV` isn't in this list, or isn't defined, then
# don't capture the error.
config.active_sentry_environments = %w[
config.enabled_environments = %w[
integration-blue-aws
staging
production
Expand Down
33 changes: 20 additions & 13 deletions spec/govuk_error/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
require "govuk_app_config/govuk_error/configuration"

RSpec.describe GovukError::Configuration do
let(:dummy_dsn) { "http://12345:67890@sentry.localdomain/sentry/42" }

before :each do
stub_const("GovukStatsd", double(Module, increment: nil))
stub_request(:post, "http://sentry.localdomain/sentry/api/42/envelope/")
.to_return(status: 200, body: "", headers: {})
end

describe ".initialize" do
Expand All @@ -26,23 +30,23 @@

it "ignores errors if they happen in an environment we don't care about" do
ClimateControl.modify SENTRY_CURRENT_ENV: "some-temporary-environment" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
sentry_client = send_exception_to_sentry(StandardError.new, configuration)
expect(sentry_client.transport.events).to be_empty
end
end

it "captures errors if they happen in an environment we care about" do
ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
sentry_client = send_exception_to_sentry(StandardError.new, configuration)
expect(sentry_client.transport.events.count).to eq(1)
end
end

it "allows string messages to be sent (rather than exceptions)" do
ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
configuration.data_sync_excluded_exceptions << "SomeError"
sentry_client = Sentry::Client.new(optimise_configuration_for_testing(configuration))
sentry_hub = Sentry::Hub.new(sentry_client, Sentry::Scope.new)
Expand All @@ -55,7 +59,7 @@
context "during the data sync" do
around do |example|
ClimateControl.modify SENTRY_CURRENT_ENV: "production", GOVUK_DATA_SYNC_PERIOD: "22:00-08:00" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
travel_to(Time.current.change(hour: 23)) do
example.run
end
Expand Down Expand Up @@ -115,7 +119,7 @@
context "outside of the data sync" do
around do |example|
ClimateControl.modify SENTRY_CURRENT_ENV: "production", GOVUK_DATA_SYNC_PERIOD: "22:00-08:00" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
travel_to(Time.current.change(hour: 21)) do
example.run
end
Expand All @@ -133,7 +137,7 @@
context "when the before_send lambda has not been overridden" do
it "increments the appropriate counters" do
ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
expect(GovukStatsd).to receive(:increment).exactly(1).times.with("errors_occurred")
expect(GovukStatsd).to receive(:increment).exactly(1).times.with("error_types.standard_error")
send_exception_to_sentry(StandardError.new, configuration)
Expand All @@ -144,7 +148,7 @@
context "when the before_send lambda has been overridden" do
it "increments the appropriate counters" do
ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
expect(GovukStatsd).to receive(:increment).exactly(1).times.with("errors_occurred")
expect(GovukStatsd).to receive(:increment).exactly(1).times.with("error_types.standard_error")
expect(GovukStatsd).to receive(:increment).exactly(1).times.with("hello_world")
Expand All @@ -162,7 +166,7 @@
context "when the before_send lambda has been overridden several times, all take effect" do
it "increments the appropriate counters" do
ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
expect(GovukStatsd).to receive(:increment).exactly(1).times.with("errors_occurred")
expect(GovukStatsd).to receive(:increment).exactly(1).times.with("error_types.standard_error")
expect(GovukStatsd).to receive(:increment).exactly(1).times.with("hello_world")
Expand All @@ -185,7 +189,7 @@
context "when a message rather than an exception is sent to Sentry" do
it "does not increment the error counters" do
ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
configuration.active_sentry_environments << "production"
configuration.enabled_environments << "production"
sentry_client = Sentry::Client.new(optimise_configuration_for_testing(configuration))
sentry_hub = Sentry::Hub.new(sentry_client, Sentry::Scope.new)

Expand All @@ -202,7 +206,7 @@
it "Allows apps to add their own `before_send` callback, that is evaluated alongside the default. If all return their parameter, then the chain continues, but if any returns `nil`, then it ends and the error is dropped" do
ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
sentry_configurator = GovukError::Configuration.new(Sentry::Configuration.new)
sentry_configurator.active_sentry_environments << "production"
sentry_configurator.enabled_environments << "production"
stub_const("CustomError", Class.new(StandardError))
sentry_configurator.before_send = lambda do |event, hint|
event if hint[:exception].is_a?(CustomError)
Expand All @@ -218,7 +222,7 @@
it "does not increment the counters if the callback returns nil" do
ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
sentry_configurator = GovukError::Configuration.new(Sentry::Configuration.new)
sentry_configurator.active_sentry_environments << "production"
sentry_configurator.enabled_environments << "production"
sentry_configurator.before_send = lambda do |_error_or_event, _hint|
nil
end
Expand All @@ -241,8 +245,11 @@ def send_exception_to_sentry(event, configuration)
def optimise_configuration_for_testing(configuration)
# prevent the sending happening in a separate worker, which would cause async results
configuration.background_worker_threads = 0
# force configuration to allow sending
allow(configuration).to receive(:sending_allowed?).and_return(true)
# allows us to debug which events have been sent to Sentry
# https://github.com/getsentry/sentry-ruby/blob/b9aa6ca8ad2bb1965ca58c7f8fc0dd16b5df310b/sentry-ruby/lib/sentry/transport/dummy_transport.rb#L7-L11
configuration.transport.transport_class = Sentry::DummyTransport
# required for `Sentry::Configuration`'s `valid?` method to return `true`
configuration.dsn = dummy_dsn
configuration
end
end

0 comments on commit 945ad98

Please sign in to comment.