From fd37682ff9500926d209ae2e37b95b24efd97a7d Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 14 Nov 2022 14:24:16 +0000 Subject: [PATCH 01/24] Add FeatureFlag class --- lib/bugsnag.rb | 2 + lib/bugsnag/feature_flag.rb | 64 +++++++++++++++++++ spec/feature_flag_spec.rb | 121 ++++++++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+) create mode 100644 lib/bugsnag/feature_flag.rb create mode 100644 spec/feature_flag_spec.rb diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 4d94ce8c..f1c76722 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -14,6 +14,8 @@ require "bugsnag/delivery/synchronous" require "bugsnag/delivery/thread_queue" +require "bugsnag/feature_flag" + # Rack is not bundled with the other integrations # as it doesn't auto-configure when loaded require "bugsnag/integrations/rack" diff --git a/lib/bugsnag/feature_flag.rb b/lib/bugsnag/feature_flag.rb new file mode 100644 index 00000000..0cfb5ba3 --- /dev/null +++ b/lib/bugsnag/feature_flag.rb @@ -0,0 +1,64 @@ +module Bugsnag + class FeatureFlag + # Get the name of this feature flag + # + # @return [String] + attr_reader :name + + # Get the variant of this feature flag + # + # @return [String, nil] + attr_reader :variant + + # @param name [String] The name of this feature flags + # @param variant [String, nil] An optional variant for this flag + def initialize(name, variant = nil) + @name = name + @variant = coerce_variant(variant) + end + + def ==(other) + self.class == other.class && @name == other.name && @variant == other.variant + end + + def hash + [@name, @variant].hash + end + + # Convert this flag to a hash + # + # @example With no variant + # { "featureFlag" => "name" } + # + # @example With a variant + # { "featureFlag" => "name", "variant" => "variant" } + # + # @return [Hash{String => String}] + def to_h + if @variant.nil? + { "featureFlag" => @name } + else + { "featureFlag" => @name, "variant" => @variant } + end + end + + private + + # Coerce this variant into a valid value (String or nil) + # + # If the variant is not already a string or nil, we use #to_s to coerce it. + # If #to_s raises, the variant will be set to nil + # + # @param variant [Object] + # @return [String, nil] + def coerce_variant(variant) + if variant.nil? || variant.is_a?(String) + variant + else + variant.to_s + end + rescue StandardError + nil + end + end +end diff --git a/spec/feature_flag_spec.rb b/spec/feature_flag_spec.rb new file mode 100644 index 00000000..370ffbbc --- /dev/null +++ b/spec/feature_flag_spec.rb @@ -0,0 +1,121 @@ +require 'spec_helper' + +describe Bugsnag::FeatureFlag do + it "has a name" do + flag = Bugsnag::FeatureFlag.new("abc") + + expect(flag.name).to eq("abc") + expect(flag.variant).to be_nil + end + + it "has an optional variant" do + flag = Bugsnag::FeatureFlag.new("abc", "xyz") + + expect(flag.name).to eq("abc") + expect(flag.variant).to eq("xyz") + end + + [ + [123, "123"], + [true, "true"], + [false, "false"], + [[1, 2, 3], "[1, 2, 3]"], + [{ a: 1, b: 2 }, "{:a=>1, :b=>2}"], + ].each do |variant, expected| + it "converts the variant to a string if given '#{variant.class}'" do + flag = Bugsnag::FeatureFlag.new("abc", variant) + + expect(flag.name).to eq("abc") + expect(flag.variant).to eq(expected) + end + end + + it "sets variant to 'nil' if variant cannot be converted to a string" do + class StringRaiser + def to_s + raise 'Oh no you do not!' + end + end + + flag = Bugsnag::FeatureFlag.new("abc", StringRaiser.new) + + expect(flag.name).to eq("abc") + expect(flag.variant).to be_nil + end + + + it "is immutable" do + flag = Bugsnag::FeatureFlag.new("abc", "xyz") + + expect(flag).not_to respond_to(:name=) + expect(flag).not_to respond_to(:variant=) + end + + describe "#to_h" do + it "converts the flag to a hash when no variant is given" do + flag = Bugsnag::FeatureFlag.new("xyz") + + expect(flag.to_h).to eq({ "featureFlag" => "xyz" }) + end + + it "converts the flag to a hash when variant is given" do + flag = Bugsnag::FeatureFlag.new("xyz", "1234") + + expect(flag.to_h).to eq({ "featureFlag" => "xyz", "variant" => "1234" }) + end + end + + describe "#==" do + it "is equal to other instances with the same name when neither have a variant" do + flag1 = Bugsnag::FeatureFlag.new("xyz") + flag2 = Bugsnag::FeatureFlag.new("xyz") + + expect(flag1).to eq(flag2) + expect(flag2).to eq(flag1) + + expect(flag1).not_to be(flag2) + end + + it "is equal to other instances with the same name and variant" do + flag1 = Bugsnag::FeatureFlag.new("xyz", "1234") + flag2 = Bugsnag::FeatureFlag.new("xyz", "1234") + + expect(flag1).to eq(flag2) + expect(flag2).to eq(flag1) + + expect(flag1).not_to be(flag2) + end + + it "is not equal to other instances with the same name but a different variant" do + flag1 = Bugsnag::FeatureFlag.new("xyz", "1234") + flag2 = Bugsnag::FeatureFlag.new("xyz", "9876") + + expect(flag1).not_to eq(flag2) + expect(flag2).not_to eq(flag1) + end + + it "is not equal to other instances with the same name when only one has a variant" do + flag1 = Bugsnag::FeatureFlag.new("xyz") + flag2 = Bugsnag::FeatureFlag.new("xyz", "9876") + + expect(flag1).not_to eq(flag2) + expect(flag2).not_to eq(flag1) + end + + it "is not equal to other instances with a different name but the same variant" do + flag1 = Bugsnag::FeatureFlag.new("xyz", "1234") + flag2 = Bugsnag::FeatureFlag.new("abc", "1234") + + expect(flag1).not_to eq(flag2) + expect(flag2).not_to eq(flag1) + end + + it "is not equal to other instances with a different name and variant" do + flag1 = Bugsnag::FeatureFlag.new("xyz", "1234") + flag2 = Bugsnag::FeatureFlag.new("abc", "9876") + + expect(flag1).not_to eq(flag2) + expect(flag2).not_to eq(flag1) + end + end +end From e4ca5e0e4f963776987eae18b49c6c09a77687da Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 14 Nov 2022 14:24:32 +0000 Subject: [PATCH 02/24] Add FeatureFlagDelegate class --- lib/bugsnag.rb | 1 + lib/bugsnag/utility/feature_flag_delegate.rb | 77 ++++++ spec/utility/feature_flag_delegate_spec.rb | 254 +++++++++++++++++++ 3 files changed, 332 insertions(+) create mode 100644 lib/bugsnag/utility/feature_flag_delegate.rb create mode 100644 spec/utility/feature_flag_delegate_spec.rb diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index f1c76722..3738b3f3 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -38,6 +38,7 @@ require "bugsnag/utility/duplicator" require "bugsnag/utility/metadata_delegate" +require "bugsnag/utility/feature_flag_delegate" # rubocop:todo Metrics/ModuleLength module Bugsnag diff --git a/lib/bugsnag/utility/feature_flag_delegate.rb b/lib/bugsnag/utility/feature_flag_delegate.rb new file mode 100644 index 00000000..b59ca13e --- /dev/null +++ b/lib/bugsnag/utility/feature_flag_delegate.rb @@ -0,0 +1,77 @@ +module Bugsnag::Utility + # @api private + class FeatureFlagDelegate + def initialize + # feature flags are stored internally in a hash of "name" => + # we don't use a Set because new feature flags should overwrite old ones + # that share a name, but FeatureFlag equality also uses the variant + @storage = {} + end + + # Add a feature flag with the given name & variant + # + # @param name [String] + # @param variant [String, nil] + # @return [void] + def add(name, variant) + @storage[name] = Bugsnag::FeatureFlag.new(name, variant) + end + + # Merge the given array of FeatureFlag instances into the stored feature + # flags + # + # New flags will be appended to the array. Flags with the same name will be + # overwritten, but their position in the array will not change + # + # @param feature_flags [Array] + # @return [void] + def merge(feature_flags) + feature_flags.each do |flag| + next unless flag.is_a?(Bugsnag::FeatureFlag) + + @storage[flag.name] = flag + end + end + + # Remove the stored flag with the given name + # + # @param name [String] + # @return [void] + def remove(name) + @storage.delete(name) + end + + # Remove all the stored flags + # + # @return [void] + def clear + @storage.clear + end + + # Get an array of FeatureFlag instances + # + # @example + # [ + # <#Bugsnag::FeatureFlag>, + # <#Bugsnag::FeatureFlag>, + # ] + # + # @return [Array] + def to_a + @storage.values + end + + # Get the feature flags in their JSON representation + # + # @example + # [ + # { "featureFlag" => "name", "variant" => "variant" }, + # { "featureFlag" => "another name" }, + # ] + # + # @return [Array String}>] + def as_json + to_a.map(&:to_h) + end + end +end diff --git a/spec/utility/feature_flag_delegate_spec.rb b/spec/utility/feature_flag_delegate_spec.rb new file mode 100644 index 00000000..71a0bc55 --- /dev/null +++ b/spec/utility/feature_flag_delegate_spec.rb @@ -0,0 +1,254 @@ +require 'spec_helper' + +describe Bugsnag::Utility::FeatureFlagDelegate do + it "contains no flags by default" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + expect(delegate.as_json).to eq([]) + end + + describe "#add" do + it "can add flags individually" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "xyz") + delegate.add("another", nil) + delegate.add("a third one", "1234") + + expect(delegate.as_json).to eq([ + { "featureFlag" => "abc", "variant" => "xyz" }, + { "featureFlag" => "another" }, + { "featureFlag" => "a third one", "variant" => "1234" }, + ]) + end + + it "replaces flags by name when the original has no variant" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", nil) + delegate.add("another", nil) + delegate.add("abc", "123") + + expect(delegate.as_json).to eq([ + { "featureFlag" => "abc", "variant" => "123" }, + { "featureFlag" => "another" }, + ]) + end + + it "replaces flags by name when the replacement has no variant" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "123") + delegate.add("another", nil) + delegate.add("abc", nil) + + expect(delegate.as_json).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "another" }, + ]) + end + + it "replaces flags by name when both have variants" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "123") + delegate.add("another", nil) + delegate.add("abc", "987") + + expect(delegate.as_json).to eq([ + { "featureFlag" => "abc", "variant" => "987" }, + { "featureFlag" => "another" }, + ]) + end + end + + describe "#merge" do + it "can add multiple flags at once" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.merge([ + Bugsnag::FeatureFlag.new("a", "xyz"), + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("c", "111"), + Bugsnag::FeatureFlag.new("d"), + ]) + + expect(delegate.as_json).to eq([ + { "featureFlag" => "a", "variant" => "xyz" }, + { "featureFlag" => "b" }, + { "featureFlag" => "c", "variant" => "111" }, + { "featureFlag" => "d" }, + ]) + end + + it "replaces flags by name when the original has no variant" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("a", nil) + + delegate.merge([ + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("a", "123"), + ]) + + expect(delegate.as_json).to eq([ + { "featureFlag" => "a", "variant" => "123" }, + { "featureFlag" => "b" }, + ]) + end + + it "replaces flags by name when the replacement has no variant" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("a", "123") + + delegate.merge([ + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("a"), + ]) + + expect(delegate.as_json).to eq([ + { "featureFlag" => "a" }, + { "featureFlag" => "b" }, + ]) + end + + it "replaces flags by name when both have variants" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("a", "987") + + delegate.merge([ + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("a", "123"), + ]) + + expect(delegate.as_json).to eq([ + { "featureFlag" => "a", "variant" => "123" }, + { "featureFlag" => "b" }, + ]) + end + + it "ignores anything that isn't a FeatureFlag instance" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.merge([ + Bugsnag::FeatureFlag.new("a", "xyz"), + 1234, + Bugsnag::FeatureFlag.new("b"), + "hello", + Bugsnag::FeatureFlag.new("c", "111"), + RuntimeError.new("xyz"), + Bugsnag::FeatureFlag.new("d"), + nil, + ]) + + expect(delegate.as_json).to eq([ + { "featureFlag" => "a", "variant" => "xyz" }, + { "featureFlag" => "b" }, + { "featureFlag" => "c", "variant" => "111" }, + { "featureFlag" => "d" }, + ]) + end + end + + describe "#remove" do + it "can remove flags by name" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "xyz") + delegate.add("another", nil) + delegate.add("a third one", "1234") + + delegate.remove("abc") + delegate.remove("a third one") + + expect(delegate.as_json).to eq([ + { "featureFlag" => "another" }, + ]) + end + + it "does nothing when no flag exists with the given name" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "xyz") + delegate.remove("xyz") + + expect(delegate.as_json).to eq([ + { "featureFlag" => "abc", "variant" => "xyz" }, + ]) + end + end + + describe "#clear" do + it "can remove all flags at once" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "xyz") + delegate.add("another", nil) + delegate.add("a third one", "1234") + + delegate.clear + + expect(delegate.as_json).to eq([]) + end + + it "does nothing when there are no flags" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.clear + + expect(delegate.as_json).to eq([]) + end + end + + describe "#to_a" do + it "returns an empty array when there are no feature flags" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + expect(delegate.to_a).to eq([]) + end + + it "returns an array of feature flags" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "xyz") + delegate.add("another", nil) + delegate.add("a third one", "1234") + + expect(delegate.to_a).to eq([ + Bugsnag::FeatureFlag.new("abc", "xyz"), + Bugsnag::FeatureFlag.new("another"), + Bugsnag::FeatureFlag.new("a third one", "1234"), + ]) + end + + it "can be mutated without affecting the internal storage" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "xyz") + delegate.add("another", nil) + delegate.add("a third one", "1234") + + flags = delegate.to_a + + expected = [ + Bugsnag::FeatureFlag.new("abc", "xyz"), + Bugsnag::FeatureFlag.new("another"), + Bugsnag::FeatureFlag.new("a third one", "1234"), + ] + + expect(flags).to eq(expected) + + flags.pop + flags.pop + flags.push(1234) + + expect(delegate.to_a).to eq(expected) + expect(flags).to eq([ + Bugsnag::FeatureFlag.new("abc", "xyz"), + 1234, + ]) + end + end +end From 9fcc5c8e346ece74b79434dec598eaf54faa4405 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 15 Nov 2022 12:28:22 +0000 Subject: [PATCH 03/24] Add feature flags to events --- lib/bugsnag/report.rb | 45 +++++++++++++++ spec/report_spec.rb | 125 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 2b92f363..8d88bddb 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -143,6 +143,7 @@ def initialize(exception, passed_configuration, auto_notify=false) self.user = {} @metadata_delegate = Utility::MetadataDelegate.new + @feature_flags = Utility::FeatureFlagDelegate.new end ## @@ -220,6 +221,7 @@ def as_json time: @created_at }, exceptions: exceptions, + featureFlags: @feature_flags.as_json, groupingHash: grouping_hash, metaData: meta_data, session: session, @@ -358,6 +360,49 @@ def clear_metadata(section, *args) @metadata_delegate.clear_metadata(@meta_data, section, *args) end + # Get the array of stored feature flags + # + # @return [Array] + def feature_flags + @feature_flags.to_a + end + + # Add a feature flag with the given name & variant + # + # @param name [String] + # @param variant [String, nil] + # @return [void] + def add_feature_flag(name, variant = nil) + @feature_flags.add(name, variant) + end + + # Merge the given array of FeatureFlag instances into the stored feature + # flags + # + # New flags will be appended to the array. Flags with the same name will be + # overwritten, but their position in the array will not change + # + # @param feature_flags [Array] + # @return [void] + def add_feature_flags(feature_flags) + @feature_flags.merge(feature_flags) + end + + # Remove the stored flag with the given name + # + # @param name [String] + # @return [void] + def clear_feature_flag(name) + @feature_flags.remove(name) + end + + # Remove all the stored flags + # + # @return [void] + def clear_feature_flags + @feature_flags.clear + end + ## # Set information about the current user # diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 27ff6d65..c1ed44c0 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -2098,5 +2098,130 @@ def to_s expect(payload["payloadVersion"]).to eq("4.0") }) end + + describe "feature flags" do + it "includes no feature flags by default" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([]) + } + end + + it "can add individual feature flags to the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + event.add_feature_flag("flag 1") + event.add_feature_flag("flag 2", "1234") + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([ + { "featureFlag" => "flag 1" }, + { "featureFlag" => "flag 2", "variant" => "1234" }, + ]) + } + end + + it "can add multiple feature flags to the payload in one go" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + flags = [ + Bugsnag::FeatureFlag.new("a"), + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("c", "1"), + Bugsnag::FeatureFlag.new("d", "2"), + ] + + event.add_feature_flags(flags) + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([ + { "featureFlag" => "a" }, + { "featureFlag" => "b" }, + { "featureFlag" => "c", "variant" => "1" }, + { "featureFlag" => "d", "variant" => "2" }, + ]) + } + end + + it "can remove a feature flag from the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + flags = [ + Bugsnag::FeatureFlag.new("a"), + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("c", "1"), + Bugsnag::FeatureFlag.new("d", "2"), + ] + + event.add_feature_flags(flags) + event.add_feature_flag("e") + + event.clear_feature_flag("b") + event.clear_feature_flag("d") + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([ + { "featureFlag" => "a" }, + { "featureFlag" => "c", "variant" => "1" }, + { "featureFlag" => "e" }, + ]) + } + end + + it "can remove all feature flags from the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + flags = [ + Bugsnag::FeatureFlag.new("a"), + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("c", "1"), + Bugsnag::FeatureFlag.new("d", "2"), + ] + + event.add_feature_flags(flags) + event.add_feature_flag("e") + + event.clear_feature_flags + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([]) + } + end + + it "can get feature flags from the event" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + flags = [ + Bugsnag::FeatureFlag.new("a"), + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("c", "1"), + Bugsnag::FeatureFlag.new("d", "2"), + ] + + event.add_feature_flags(flags) + event.add_feature_flag("e") + + expect(event.feature_flags).to eq([ + Bugsnag::FeatureFlag.new("a"), + Bugsnag::FeatureFlag.new("b"), + Bugsnag::FeatureFlag.new("c", "1"), + Bugsnag::FeatureFlag.new("d", "2"), + Bugsnag::FeatureFlag.new("e"), + ]) + + event.clear_feature_flags + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([]) + } + end + end end # rubocop:enable Metrics/BlockLength From 92c895048945d37da6f871379467e75a7abe266e Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Nov 2022 11:35:24 +0000 Subject: [PATCH 04/24] Add FeatureFlag#valid? This returns true if: - name is a string with at least 1 character - variant is nil OR variant is a string --- lib/bugsnag/feature_flag.rb | 10 +++++++++ spec/feature_flag_spec.rb | 43 +++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/lib/bugsnag/feature_flag.rb b/lib/bugsnag/feature_flag.rb index 0cfb5ba3..623a01b8 100644 --- a/lib/bugsnag/feature_flag.rb +++ b/lib/bugsnag/feature_flag.rb @@ -42,6 +42,16 @@ def to_h end end + # Check if this flag is valid, i.e. has a name that's a String and a variant + # that's either nil or a String + # + # @return [Boolean] + def valid? + @name.is_a?(String) && + !@name.empty? && + (@variant.nil? || @variant.is_a?(String)) + end + private # Coerce this variant into a valid value (String or nil) diff --git a/spec/feature_flag_spec.rb b/spec/feature_flag_spec.rb index 370ffbbc..4fdf5c22 100644 --- a/spec/feature_flag_spec.rb +++ b/spec/feature_flag_spec.rb @@ -118,4 +118,47 @@ def to_s expect(flag2).not_to eq(flag1) end end + + describe "#valid?" do + [ + nil, + true, + false, + 1234, + [1, 2, 3], + { a: 1, b: 2 }, + :abc, + "", + ].each do |name| + it "returns false when name is '#{name.inspect}' with no variant" do + flag = Bugsnag::FeatureFlag.new(name) + + expect(flag.valid?).to be(false) + end + + it "returns false when name is '#{name.inspect}' and variant is present" do + flag = Bugsnag::FeatureFlag.new(name, name) + + expect(flag.valid?).to be(false) + end + + it "returns true when name is a string and variant is '#{name.inspect}'" do + flag = Bugsnag::FeatureFlag.new("a name", name) + + expect(flag.valid?).to be(true) + end + end + + it "returns true when name is a string with no variant" do + flag = Bugsnag::FeatureFlag.new("a name") + + expect(flag.valid?).to be(true) + end + + it "returns true when name and variant are strings" do + flag = Bugsnag::FeatureFlag.new("a name", "a variant") + + expect(flag.valid?).to be(true) + end + end end From 5e256135b06d334fc31029103073121e65b94034 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Nov 2022 11:36:15 +0000 Subject: [PATCH 05/24] Drop feature flags that are invalid The FeatureFlagDelegate now ensures flags are valid before storing them, so we no longer deliver flags with invalid names --- lib/bugsnag/utility/feature_flag_delegate.rb | 7 +++- spec/utility/feature_flag_delegate_spec.rb | 42 ++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/lib/bugsnag/utility/feature_flag_delegate.rb b/lib/bugsnag/utility/feature_flag_delegate.rb index b59ca13e..41fc5ddb 100644 --- a/lib/bugsnag/utility/feature_flag_delegate.rb +++ b/lib/bugsnag/utility/feature_flag_delegate.rb @@ -14,7 +14,11 @@ def initialize # @param variant [String, nil] # @return [void] def add(name, variant) - @storage[name] = Bugsnag::FeatureFlag.new(name, variant) + flag = Bugsnag::FeatureFlag.new(name, variant) + + return unless flag.valid? + + @storage[flag.name] = flag end # Merge the given array of FeatureFlag instances into the stored feature @@ -28,6 +32,7 @@ def add(name, variant) def merge(feature_flags) feature_flags.each do |flag| next unless flag.is_a?(Bugsnag::FeatureFlag) + next unless flag.valid? @storage[flag.name] = flag end diff --git a/spec/utility/feature_flag_delegate_spec.rb b/spec/utility/feature_flag_delegate_spec.rb index 71a0bc55..7816cab1 100644 --- a/spec/utility/feature_flag_delegate_spec.rb +++ b/spec/utility/feature_flag_delegate_spec.rb @@ -1,6 +1,16 @@ require 'spec_helper' describe Bugsnag::Utility::FeatureFlagDelegate do + invalid_names = [ + nil, + true, + false, + 1234, + [1, 2, 3], + { a: 1, b: 2 }, + "", + ] + it "contains no flags by default" do delegate = Bugsnag::Utility::FeatureFlagDelegate.new @@ -60,6 +70,21 @@ { "featureFlag" => "another" }, ]) end + + invalid_names.each do |name| + it "drops flags when name is '#{name.inspect}'" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.add("abc", "123") + delegate.add(name, nil) + delegate.add("xyz", "987") + + expect(delegate.as_json).to eq([ + { "featureFlag" => "abc", "variant" => "123" }, + { "featureFlag" => "xyz", "variant" => "987" }, + ]) + end + end end describe "#merge" do @@ -150,6 +175,23 @@ { "featureFlag" => "d" }, ]) end + + invalid_names.each do |name| + it "drops flag when name is '#{name.inspect}'" do + delegate = Bugsnag::Utility::FeatureFlagDelegate.new + + delegate.merge([ + Bugsnag::FeatureFlag.new("abc", "123"), + Bugsnag::FeatureFlag.new(name, "456"), + Bugsnag::FeatureFlag.new("xyz", "789"), + ]) + + expect(delegate.as_json).to eq([ + { "featureFlag" => "abc", "variant" => "123" }, + { "featureFlag" => "xyz", "variant" => "789" }, + ]) + end + end end describe "#remove" do From 21b7edb785179e2504770bb0a21f179e89437eb9 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Nov 2022 09:49:10 +0000 Subject: [PATCH 06/24] Add tests for feature flags in Rack apps --- features/fixtures/rack/app/app.rb | 30 +++++++++++++++++++++++++ features/rack.feature | 37 +++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/features/fixtures/rack/app/app.rb b/features/fixtures/rack/app/app.rb index 41d0f2ec..1220c564 100644 --- a/features/fixtures/rack/app/app.rb +++ b/features/fixtures/rack/app/app.rb @@ -24,6 +24,36 @@ def call(env) rescue StandardError => e Bugsnag.notify(e) end + when '/feature-flags/unhandled' + Bugsnag.add_on_error(proc do |event| + event.add_feature_flag('a', '1') + + event.add_feature_flags([ + Bugsnag::FeatureFlag.new('b'), + Bugsnag::FeatureFlag.new('c', '3'), + ]) + + event.add_feature_flag('d') + + if req.params["clear_all_flags"] + event.clear_feature_flags + end + end) + + raise 'Unhandled error' + when '/feature-flags/handled' + Bugsnag.notify(RuntimeError.new('oh no')) do |event| + event.add_feature_flag('x') + + event.add_feature_flags([ + Bugsnag::FeatureFlag.new('y', '1234'), + Bugsnag::FeatureFlag.new('z'), + ]) + + if req.params["clear_all_flags"] + event.clear_feature_flags + end + end end res = Rack::Response.new diff --git a/features/rack.feature b/features/rack.feature index e38e1090..84c1ebe3 100644 --- a/features/rack.feature +++ b/features/rack.feature @@ -131,3 +131,40 @@ Scenario: A request with cookies and no matching filter will set cookies in meta And the event "metaData.request.params.b" equals "456" And the event "metaData.request.referer" is null And the event "metaData.request.url" ends with "/unhandled?a=123&b=456" + +Scenario: adding feature flags for an unhandled error + Given I start the rack service + When I navigate to the route "/feature-flags/unhandled" on the rack app + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier + And the event contains the following feature flags: + | featureFlag | variant | + | a | 1 | + | b | | + | c | 3 | + | d | | + + Scenario: adding feature flags for a handled error + Given I start the rack service + When I navigate to the route "/feature-flags/handled" on the rack app + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier + And the event contains the following feature flags: + | featureFlag | variant | + | x | | + | y | 1234 | + | z | | + +Scenario: clearing feature flags for an unhandled error + Given I start the rack service + When I navigate to the route "/feature-flags/unhandled?clear_all_flags=1" on the rack app + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier + And the event has no feature flags + + Scenario: clearing feature flags for a handled error + Given I start the rack service + When I navigate to the route "/feature-flags/handled?clear_all_flags=1" on the rack app + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier + And the event has no feature flags From 0e2171aad4c3a3d9a4fb13ad3c31d81cf2c33ee9 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 11:09:24 +0000 Subject: [PATCH 07/24] Test feature flags in Rails 7 --- .../controllers/feature_flags_controller.rb | 25 +++++++ features/fixtures/rails7/app/config/routes.rb | 3 + features/rails_features/feature_flags.feature | 71 +++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb create mode 100644 features/rails_features/feature_flags.feature diff --git a/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb new file mode 100644 index 00000000..ffb4007b --- /dev/null +++ b/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb @@ -0,0 +1,25 @@ +class FeatureFlagsController < ActionController::Base + protect_from_forgery + + before_bugsnag_notify :add_feature_flags + + def unhandled + raise 'oh no' + end + + def handled + Bugsnag.notify(RuntimeError.new('ahhh')) + end + + private + + def add_feature_flags(event) + params['flags'].each do |key, value| + event.add_feature_flag(key, value) + end + + if params.key?('clear_all_flags') + event.clear_feature_flags + end + end +end diff --git a/features/fixtures/rails7/app/config/routes.rb b/features/fixtures/rails7/app/config/routes.rb index f153a9ed..3c0f7543 100644 --- a/features/fixtures/rails7/app/config/routes.rb +++ b/features/fixtures/rails7/app/config/routes.rb @@ -65,4 +65,7 @@ get 'active_job/handled', to: 'active_job#handled' get 'active_job/unhandled', to: 'active_job#unhandled' + + get 'features/handled', to: 'feature_flags#handled' + get 'features/unhandled', to: 'feature_flags#unhandled' end diff --git a/features/rails_features/feature_flags.feature b/features/rails_features/feature_flags.feature new file mode 100644 index 00000000..bb00d27b --- /dev/null +++ b/features/rails_features/feature_flags.feature @@ -0,0 +1,71 @@ +Feature: feature flags + +@rails7 +Scenario: adding feature flags for an unhandled error + Given I start the rails service + When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4" on the rails app + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier + And the event "unhandled" is true + And the event "severity" equals "error" + And the exception "errorClass" equals "RuntimeError" + And the exception "message" equals "oh no" + And the event contains the following feature flags: + | featureFlag | variant | + | a | 1 | + | b | | + | c | 3 | + | d | 4 | + # ensure each request can have its own set of feature flags + When I discard the oldest error + And I navigate to the route "/features/unhandled?flags[x]=9&flags[y]&flags[z]=7" on the rails app + And I wait to receive an error + And the event contains the following feature flags: + | featureFlag | variant | + | x | 9 | + | y | | + | z | 7 | + +@rails7 +Scenario: adding feature flags for a handled error + Given I start the rails service + When I navigate to the route "/features/handled?flags[ab]=12&flags[cd]=34" on the rails app + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier + And the event "unhandled" is false + And the event "severity" equals "warning" + And the exception "errorClass" equals "RuntimeError" + And the exception "message" equals "ahhh" + And the event contains the following feature flags: + | featureFlag | variant | + | ab | 12 | + | cd | 34 | + # ensure each request can have its own set of feature flags + When I discard the oldest error + And I navigate to the route "/features/unhandled?flags[e]=h&flags[f]=i&flags[g]" on the rails app + And I wait to receive an error + And the event contains the following feature flags: + | featureFlag | variant | + | e | h | + | f | i | + | g | | + +@rails7 +Scenario: clearing all feature flags doesn't affect subsequent requests + Given I start the rails service + When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4&clear_all_flags" on the rails app + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier + And the event "unhandled" is true + And the event "severity" equals "error" + And the exception "errorClass" equals "RuntimeError" + And the exception "message" equals "oh no" + And the event has no feature flags + When I discard the oldest error + And I navigate to the route "/features/unhandled?flags[x]=9&flags[y]&flags[z]=7" on the rails app + And I wait to receive an error + And the event contains the following feature flags: + | featureFlag | variant | + | x | 9 | + | y | | + | z | 7 | From eca29f48261f3d5aa4091df6e14338cee350b1c9 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 11:16:04 +0000 Subject: [PATCH 08/24] Test feature flags in Rails 6 --- .../controllers/feature_flags_controller.rb | 25 +++++++++++++++++++ features/fixtures/rails6/app/config/routes.rb | 3 +++ features/rails_features/feature_flags.feature | 6 ++--- 3 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb diff --git a/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb new file mode 100644 index 00000000..ffb4007b --- /dev/null +++ b/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb @@ -0,0 +1,25 @@ +class FeatureFlagsController < ActionController::Base + protect_from_forgery + + before_bugsnag_notify :add_feature_flags + + def unhandled + raise 'oh no' + end + + def handled + Bugsnag.notify(RuntimeError.new('ahhh')) + end + + private + + def add_feature_flags(event) + params['flags'].each do |key, value| + event.add_feature_flag(key, value) + end + + if params.key?('clear_all_flags') + event.clear_feature_flags + end + end +end diff --git a/features/fixtures/rails6/app/config/routes.rb b/features/fixtures/rails6/app/config/routes.rb index f153a9ed..3c0f7543 100644 --- a/features/fixtures/rails6/app/config/routes.rb +++ b/features/fixtures/rails6/app/config/routes.rb @@ -65,4 +65,7 @@ get 'active_job/handled', to: 'active_job#handled' get 'active_job/unhandled', to: 'active_job#unhandled' + + get 'features/handled', to: 'feature_flags#handled' + get 'features/unhandled', to: 'feature_flags#unhandled' end diff --git a/features/rails_features/feature_flags.feature b/features/rails_features/feature_flags.feature index bb00d27b..874f2d1f 100644 --- a/features/rails_features/feature_flags.feature +++ b/features/rails_features/feature_flags.feature @@ -1,6 +1,6 @@ Feature: feature flags -@rails7 +@rails6 @rails7 Scenario: adding feature flags for an unhandled error Given I start the rails service When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4" on the rails app @@ -26,7 +26,7 @@ Scenario: adding feature flags for an unhandled error | y | | | z | 7 | -@rails7 +@rails6 @rails7 Scenario: adding feature flags for a handled error Given I start the rails service When I navigate to the route "/features/handled?flags[ab]=12&flags[cd]=34" on the rails app @@ -50,7 +50,7 @@ Scenario: adding feature flags for a handled error | f | i | | g | | -@rails7 +@rails6 @rails7 Scenario: clearing all feature flags doesn't affect subsequent requests Given I start the rails service When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4&clear_all_flags" on the rails app From 9c4506f38adf5fee156e4bc19226bf23157e1a77 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 11:22:07 +0000 Subject: [PATCH 09/24] Test feature flags in Rails 5 --- .../controllers/feature_flags_controller.rb | 25 +++++++++++++++++++ features/fixtures/rails5/app/config/routes.rb | 3 +++ features/rails_features/feature_flags.feature | 6 ++--- 3 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb diff --git a/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb new file mode 100644 index 00000000..ffb4007b --- /dev/null +++ b/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb @@ -0,0 +1,25 @@ +class FeatureFlagsController < ActionController::Base + protect_from_forgery + + before_bugsnag_notify :add_feature_flags + + def unhandled + raise 'oh no' + end + + def handled + Bugsnag.notify(RuntimeError.new('ahhh')) + end + + private + + def add_feature_flags(event) + params['flags'].each do |key, value| + event.add_feature_flag(key, value) + end + + if params.key?('clear_all_flags') + event.clear_feature_flags + end + end +end diff --git a/features/fixtures/rails5/app/config/routes.rb b/features/fixtures/rails5/app/config/routes.rb index f153a9ed..3c0f7543 100644 --- a/features/fixtures/rails5/app/config/routes.rb +++ b/features/fixtures/rails5/app/config/routes.rb @@ -65,4 +65,7 @@ get 'active_job/handled', to: 'active_job#handled' get 'active_job/unhandled', to: 'active_job#unhandled' + + get 'features/handled', to: 'feature_flags#handled' + get 'features/unhandled', to: 'feature_flags#unhandled' end diff --git a/features/rails_features/feature_flags.feature b/features/rails_features/feature_flags.feature index 874f2d1f..3f3a632a 100644 --- a/features/rails_features/feature_flags.feature +++ b/features/rails_features/feature_flags.feature @@ -1,6 +1,6 @@ Feature: feature flags -@rails6 @rails7 +@rails5 @rails6 @rails7 Scenario: adding feature flags for an unhandled error Given I start the rails service When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4" on the rails app @@ -26,7 +26,7 @@ Scenario: adding feature flags for an unhandled error | y | | | z | 7 | -@rails6 @rails7 +@rails5 @rails6 @rails7 Scenario: adding feature flags for a handled error Given I start the rails service When I navigate to the route "/features/handled?flags[ab]=12&flags[cd]=34" on the rails app @@ -50,7 +50,7 @@ Scenario: adding feature flags for a handled error | f | i | | g | | -@rails6 @rails7 +@rails5 @rails6 @rails7 Scenario: clearing all feature flags doesn't affect subsequent requests Given I start the rails service When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4&clear_all_flags" on the rails app From ff88b6c921c4a4c7546b388fb1848fb8a4ad23d7 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 11:27:41 +0000 Subject: [PATCH 10/24] Test feature flags in Rails 4 --- .../controllers/feature_flags_controller.rb | 27 +++++++++++++++++++ features/fixtures/rails4/app/config/routes.rb | 1 + features/rails_features/feature_flags.feature | 6 ++--- 3 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb diff --git a/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb new file mode 100644 index 00000000..d5e37750 --- /dev/null +++ b/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb @@ -0,0 +1,27 @@ +class FeatureFlagsController < ActionController::Base + protect_from_forgery + + before_bugsnag_notify :add_feature_flags + + def unhandled + raise 'oh no' + end + + def handled + Bugsnag.notify(RuntimeError.new('ahhh')) + + render json: {} + end + + private + + def add_feature_flags(event) + params['flags'].each do |key, value| + event.add_feature_flag(key, value) + end + + if params.key?('clear_all_flags') + event.clear_feature_flags + end + end +end diff --git a/features/fixtures/rails4/app/config/routes.rb b/features/fixtures/rails4/app/config/routes.rb index 8fee25c3..970a02a6 100644 --- a/features/fixtures/rails4/app/config/routes.rb +++ b/features/fixtures/rails4/app/config/routes.rb @@ -19,4 +19,5 @@ get "/breadcrumbs/(:action)", controller: 'breadcrumbs' get "/mongo/(:action)", controller: 'mongo' get "/active_job/(:action)", controller: 'active_job' + get "/features/(:action)", controller: 'feature_flags' end diff --git a/features/rails_features/feature_flags.feature b/features/rails_features/feature_flags.feature index 3f3a632a..f33000fe 100644 --- a/features/rails_features/feature_flags.feature +++ b/features/rails_features/feature_flags.feature @@ -1,6 +1,6 @@ Feature: feature flags -@rails5 @rails6 @rails7 +@rails4 @rails5 @rails6 @rails7 Scenario: adding feature flags for an unhandled error Given I start the rails service When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4" on the rails app @@ -26,7 +26,7 @@ Scenario: adding feature flags for an unhandled error | y | | | z | 7 | -@rails5 @rails6 @rails7 +@rails4 @rails5 @rails6 @rails7 Scenario: adding feature flags for a handled error Given I start the rails service When I navigate to the route "/features/handled?flags[ab]=12&flags[cd]=34" on the rails app @@ -50,7 +50,7 @@ Scenario: adding feature flags for a handled error | f | i | | g | | -@rails5 @rails6 @rails7 +@rails4 @rails5 @rails6 @rails7 Scenario: clearing all feature flags doesn't affect subsequent requests Given I start the rails service When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4&clear_all_flags" on the rails app From 748f5567999ac984ffe3b8e5e5becf4da5b3a2ce Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 11:30:30 +0000 Subject: [PATCH 11/24] Test feature flags in Rails 3 --- .../controllers/feature_flags_controller.rb | 27 +++++++++++++++++++ features/fixtures/rails3/app/config/routes.rb | 1 + features/rails_features/feature_flags.feature | 6 ++--- 3 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb diff --git a/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb new file mode 100644 index 00000000..d5e37750 --- /dev/null +++ b/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb @@ -0,0 +1,27 @@ +class FeatureFlagsController < ActionController::Base + protect_from_forgery + + before_bugsnag_notify :add_feature_flags + + def unhandled + raise 'oh no' + end + + def handled + Bugsnag.notify(RuntimeError.new('ahhh')) + + render json: {} + end + + private + + def add_feature_flags(event) + params['flags'].each do |key, value| + event.add_feature_flag(key, value) + end + + if params.key?('clear_all_flags') + event.clear_feature_flags + end + end +end diff --git a/features/fixtures/rails3/app/config/routes.rb b/features/fixtures/rails3/app/config/routes.rb index 4c1e0dfe..8b2c8769 100644 --- a/features/fixtures/rails3/app/config/routes.rb +++ b/features/fixtures/rails3/app/config/routes.rb @@ -15,5 +15,6 @@ get "/send_environment/(:action)", controller: 'send_environment' get "/warden/(:action)", controller: 'warden' get "/breadcrumbs/(:action)", controller: 'breadcrumbs' + get "/features/(:action)", controller: 'feature_flags' get "/(:action)", controller: 'application' end diff --git a/features/rails_features/feature_flags.feature b/features/rails_features/feature_flags.feature index f33000fe..af7c0106 100644 --- a/features/rails_features/feature_flags.feature +++ b/features/rails_features/feature_flags.feature @@ -1,6 +1,6 @@ Feature: feature flags -@rails4 @rails5 @rails6 @rails7 +@rails3 @rails4 @rails5 @rails6 @rails7 Scenario: adding feature flags for an unhandled error Given I start the rails service When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4" on the rails app @@ -26,7 +26,7 @@ Scenario: adding feature flags for an unhandled error | y | | | z | 7 | -@rails4 @rails5 @rails6 @rails7 +@rails3 @rails4 @rails5 @rails6 @rails7 Scenario: adding feature flags for a handled error Given I start the rails service When I navigate to the route "/features/handled?flags[ab]=12&flags[cd]=34" on the rails app @@ -50,7 +50,7 @@ Scenario: adding feature flags for a handled error | f | i | | g | | -@rails4 @rails5 @rails6 @rails7 +@rails3 @rails4 @rails5 @rails6 @rails7 Scenario: clearing all feature flags doesn't affect subsequent requests Given I start the rails service When I navigate to the route "/features/unhandled?flags[a]=1&flags[b]&flags[c]=3&flags[d]=4&clear_all_flags" on the rails app From 4562158bc8e376b9cf2c44eef0d0cfe09179e486 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 12:19:28 +0000 Subject: [PATCH 12/24] Allow storing feature flags in Configuration --- lib/bugsnag/configuration.rb | 51 +++++++++++++++++ spec/configuration_spec.rb | 103 +++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 36dda53d..0dc50caf 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -186,6 +186,11 @@ class Configuration # @return [Breadcrumbs::OnBreadcrumbCallbackList] attr_reader :on_breadcrumb_callbacks + # Expose feature_flag_delegate internally for creating new events + # @api private + # @return [FeatureFlagDelegate] + attr_reader :feature_flag_delegate + API_KEY_REGEX = /[0-9a-f]{32}/i THREAD_LOCAL_NAME = "bugsnag_req_data" @@ -289,6 +294,8 @@ def initialize self.middleware = Bugsnag::MiddlewareStack.new self.middleware.use Bugsnag::Middleware::Callbacks + + @feature_flag_delegate = Bugsnag::Utility::FeatureFlagDelegate.new end ## @@ -664,6 +671,50 @@ def clear_metadata(section, *args) end end + # Add a feature flag with the given name & variant + # + # @param name [String] + # @param variant [String, nil] + # @return [void] + def add_feature_flag(name, variant = nil) + @mutex.synchronize do + @feature_flag_delegate.add(name, variant) + end + end + + # Merge the given array of FeatureFlag instances into the stored feature + # flags + # + # New flags will be appended to the array. Flags with the same name will be + # overwritten, but their position in the array will not change + # + # @param feature_flags [Array] + # @return [void] + def add_feature_flags(feature_flags) + @mutex.synchronize do + @feature_flag_delegate.merge(feature_flags) + end + end + + # Remove the stored flag with the given name + # + # @param name [String] + # @return [void] + def clear_feature_flag(name) + @mutex.synchronize do + @feature_flag_delegate.remove(name) + end + end + + # Remove all the stored flags + # + # @return [void] + def clear_feature_flags + @mutex.synchronize do + @feature_flag_delegate.clear + end + end + ## # Has the context been explicitly set? # diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index df0567ea..73e41210 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -712,4 +712,107 @@ def output_lines end end end + + describe "feature flags" do + describe "#feature_flag_delegate" do + it "is initially empty" do + expect(subject.feature_flag_delegate.to_a).to be_empty + end + + it "cannot be reassigned" do + expect(subject).not_to respond_to(:feature_flag_delegate=) + end + + it "reflects changes in add/clear feature flags" do + subject.add_feature_flag('abc') + subject.add_feature_flags([ + Bugsnag::FeatureFlag.new('1'), + Bugsnag::FeatureFlag.new('2', 'z'), + Bugsnag::FeatureFlag.new('3'), + ]) + subject.add_feature_flag('xyz', '1234') + + subject.clear_feature_flag('3') + + expect(subject.feature_flag_delegate.to_a).to eq([ + Bugsnag::FeatureFlag.new('abc'), + Bugsnag::FeatureFlag.new('1'), + Bugsnag::FeatureFlag.new('2', 'z'), + Bugsnag::FeatureFlag.new('xyz', '1234'), + ]) + + subject.clear_feature_flags + + expect(subject.feature_flag_delegate.to_a).to be_empty + end + end + + describe "concurrent access" do + it "can handle multiple threads adding feature flags" do + configuration = Bugsnag::Configuration.new + + threads = 5.times.map do |i| + Thread.new do + configuration.add_feature_flag("thread_#{i} flag 1", i) + end + end + + threads += 5.times.map do |i| + Thread.new do + configuration.add_feature_flags([ + Bugsnag::FeatureFlag.new("thread_#{i} flag 2", i * 100), + Bugsnag::FeatureFlag.new("thread_#{i} flag 3", i * 100 + 1), + ]) + end + end + + threads.shuffle.map(&:join) + + flags = configuration.feature_flag_delegate.to_a.sort do |a, b| + a.name <=> b.name + end + + expect(flags).to eq([ + Bugsnag::FeatureFlag.new('thread_0 flag 1', 0), + Bugsnag::FeatureFlag.new('thread_0 flag 2', 0), + Bugsnag::FeatureFlag.new('thread_0 flag 3', 1), + Bugsnag::FeatureFlag.new('thread_1 flag 1', 1), + Bugsnag::FeatureFlag.new('thread_1 flag 2', 100), + Bugsnag::FeatureFlag.new('thread_1 flag 3', 101), + Bugsnag::FeatureFlag.new('thread_2 flag 1', 2), + Bugsnag::FeatureFlag.new('thread_2 flag 2', 200), + Bugsnag::FeatureFlag.new('thread_2 flag 3', 201), + Bugsnag::FeatureFlag.new('thread_3 flag 1', 3), + Bugsnag::FeatureFlag.new('thread_3 flag 2', 300), + Bugsnag::FeatureFlag.new('thread_3 flag 3', 301), + Bugsnag::FeatureFlag.new('thread_4 flag 1', 4), + Bugsnag::FeatureFlag.new('thread_4 flag 2', 400), + Bugsnag::FeatureFlag.new('thread_4 flag 3', 401), + ]) + end + + it "can handle multiple threads clearing feature flags" do + configuration = Bugsnag::Configuration.new + + configuration.add_feature_flag('abc') + configuration.add_feature_flag('xyz') + + 5.times do |i| + configuration.add_feature_flag("thread_#{i}", i) + end + + threads = 5.times.map do |i| + Thread.new do + configuration.clear_feature_flag("thread_#{i}") + configuration.clear_feature_flag('abc') + configuration.clear_feature_flag('xyz') + end + end + + threads.shuffle.map(&:join) + + expect(configuration.feature_flag_delegate.to_a).to be_empty + end + end + end end From 3901a4858423379be54c9357749df8390bd9da5e Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 12:19:05 +0000 Subject: [PATCH 13/24] Allow FeatureFlagDelegate instances to be dup-ed --- lib/bugsnag/utility/feature_flag_delegate.rb | 7 +++++ spec/utility/feature_flag_delegate_spec.rb | 31 ++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/lib/bugsnag/utility/feature_flag_delegate.rb b/lib/bugsnag/utility/feature_flag_delegate.rb index 41fc5ddb..c90980f3 100644 --- a/lib/bugsnag/utility/feature_flag_delegate.rb +++ b/lib/bugsnag/utility/feature_flag_delegate.rb @@ -8,6 +8,13 @@ def initialize @storage = {} end + def initialize_dup(original) + super + + # copy the internal storage when 'dup' is called + @storage = @storage.dup + end + # Add a feature flag with the given name & variant # # @param name [String] diff --git a/spec/utility/feature_flag_delegate_spec.rb b/spec/utility/feature_flag_delegate_spec.rb index 7816cab1..5e93ddfa 100644 --- a/spec/utility/feature_flag_delegate_spec.rb +++ b/spec/utility/feature_flag_delegate_spec.rb @@ -17,6 +17,37 @@ expect(delegate.as_json).to eq([]) end + it "does not get mutated after being duplicated" do + delegate1 = Bugsnag::Utility::FeatureFlagDelegate.new + delegate1.add('abc', '123') + + delegate2 = delegate1.dup + delegate2.add('xyz', '987') + + expect(delegate1.to_a).to eq([ + Bugsnag::FeatureFlag.new('abc', '123'), + ]) + + expect(delegate2.to_a).to eq([ + Bugsnag::FeatureFlag.new('abc', '123'), + Bugsnag::FeatureFlag.new('xyz', '987'), + ]) + + delegate3 = delegate2.dup + delegate3.clear + + expect(delegate1.to_a).to eq([ + Bugsnag::FeatureFlag.new('abc', '123'), + ]) + + expect(delegate2.to_a).to eq([ + Bugsnag::FeatureFlag.new('abc', '123'), + Bugsnag::FeatureFlag.new('xyz', '987'), + ]) + + expect(delegate3.to_a).to be_empty + end + describe "#add" do it "can add flags individually" do delegate = Bugsnag::Utility::FeatureFlagDelegate.new From 33cc5e6012a3e94ceb28da8d79f8aad5d3dab8dd Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 12:24:59 +0000 Subject: [PATCH 14/24] Include configuration feature flags in events --- lib/bugsnag/report.rb | 2 +- spec/report_spec.rb | 57 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 8d88bddb..02f7a765 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -143,7 +143,7 @@ def initialize(exception, passed_configuration, auto_notify=false) self.user = {} @metadata_delegate = Utility::MetadataDelegate.new - @feature_flags = Utility::FeatureFlagDelegate.new + @feature_flags = configuration.feature_flag_delegate.dup end ## diff --git a/spec/report_spec.rb b/spec/report_spec.rb index c1ed44c0..4c1bc3d0 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -2109,6 +2109,63 @@ def to_s } end + it "includes the configuration's feature flags if present" do + Bugsnag.configuration.add_feature_flag('abc') + Bugsnag.configuration.add_feature_flag('xyz', '123') + + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + ]) + } + end + + it "does not mutate the configuration's feature flags if more flags are added" do + Bugsnag.configuration.add_feature_flag('abc') + Bugsnag.configuration.add_feature_flag('xyz', '123') + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + event.add_feature_flag('another one') + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + { "featureFlag" => "another one" }, + ]) + + expect(Bugsnag.configuration.feature_flag_delegate.as_json).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + ]) + } + end + + it "does not mutate the configuration's feature flags if flags are removed" do + Bugsnag.configuration.add_feature_flag('abc') + Bugsnag.configuration.add_feature_flag('xyz', '123') + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + event.clear_feature_flags + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to be_empty + + expect(Bugsnag.configuration.feature_flag_delegate.as_json).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + ]) + } + end + it "can add individual feature flags to the payload" do Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| event.add_feature_flag("flag 1") From 97352473fba1f1dd94b005a1ec060edfbea689bf Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 17 Nov 2022 12:45:33 +0000 Subject: [PATCH 15/24] Forward feature flags API calls to configuration --- lib/bugsnag.rb | 36 ++++++++++++++++++++ spec/bugsnag_spec.rb | 80 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 3738b3f3..2cbec407 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -429,6 +429,42 @@ def clear_metadata(section, *args) configuration.clear_metadata(section, *args) end + # Add a feature flag with the given name & variant + # + # @param name [String] + # @param variant [String, nil] + # @return [void] + def add_feature_flag(name, variant = nil) + configuration.add_feature_flag(name, variant) + end + + # Merge the given array of FeatureFlag instances into the stored feature + # flags + # + # New flags will be appended to the array. Flags with the same name will be + # overwritten, but their position in the array will not change + # + # @param feature_flags [Array] + # @return [void] + def add_feature_flags(feature_flags) + configuration.add_feature_flags(feature_flags) + end + + # Remove the stored flag with the given name + # + # @param name [String] + # @return [void] + def clear_feature_flag(name) + configuration.clear_feature_flag(name) + end + + # Remove all the stored flags + # + # @return [void] + def clear_feature_flags + configuration.clear_feature_flags + end + private def should_deliver_notification?(exception, auto_notify) diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 6f5af3b4..4d27eee4 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -1127,4 +1127,84 @@ module Kernel }) end end + + describe "feature flags" do + it "is added to the payload" do + Bugsnag.add_feature_flag('abc') + Bugsnag.add_feature_flag('xyz', '123') + + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + ]) + } + end + + it "does not mutate the global feature flags if more flags are added" do + Bugsnag.add_feature_flag('abc') + Bugsnag.add_feature_flag('xyz', '123') + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + event.add_feature_flag('another one') + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + { "featureFlag" => "another one" }, + ]) + + expect(Bugsnag.configuration.feature_flag_delegate.as_json).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + ]) + } + end + + it "does not mutate the global feature flags if flags are removed" do + Bugsnag.add_feature_flag('abc') + Bugsnag.add_feature_flag('xyz', '123') + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + event.clear_feature_flags + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to be_empty + + expect(Bugsnag.configuration.feature_flag_delegate.as_json).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + ]) + } + end + + it "does not mutate the event's feature flags if global flags are removed" do + Bugsnag.add_feature_flags([ + Bugsnag::FeatureFlag.new('abc'), + Bugsnag::FeatureFlag.new('xyz', 123), + ]) + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| + Bugsnag.clear_feature_flags + end + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["featureFlags"]).to eq([ + { "featureFlag" => "abc" }, + { "featureFlag" => "xyz", "variant" => "123" }, + ]) + + expect(Bugsnag.configuration.feature_flag_delegate.as_json).to be_empty + } + end + end end From 6abf4ba40f417ee09757b6f5f04c3fc13ab4ce0d Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 21 Nov 2022 10:07:42 +0000 Subject: [PATCH 16/24] Add flags to configuration in Rack tests --- features/fixtures/rack/app/app.rb | 10 ++++++++++ features/rack.feature | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/features/fixtures/rack/app/app.rb b/features/fixtures/rack/app/app.rb index 1220c564..9b9e63b1 100644 --- a/features/fixtures/rack/app/app.rb +++ b/features/fixtures/rack/app/app.rb @@ -9,6 +9,12 @@ if ENV.key?('BUGSNAG_METADATA_FILTERS') config.meta_data_filters = JSON.parse(ENV['BUGSNAG_METADATA_FILTERS']) end + + config.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + Bugsnag::FeatureFlag.new('should be removed!'), + ]) end class BugsnagTests @@ -35,6 +41,8 @@ def call(env) event.add_feature_flag('d') + event.clear_feature_flag('should be removed!') + if req.params["clear_all_flags"] event.clear_feature_flags end @@ -50,6 +58,8 @@ def call(env) Bugsnag::FeatureFlag.new('z'), ]) + event.clear_feature_flag('should be removed!') + if req.params["clear_all_flags"] event.clear_feature_flags end diff --git a/features/rack.feature b/features/rack.feature index 84c1ebe3..3827dae8 100644 --- a/features/rack.feature +++ b/features/rack.feature @@ -139,6 +139,8 @@ Scenario: adding feature flags for an unhandled error Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier And the event contains the following feature flags: | featureFlag | variant | + | from config 1 | | + | from config 2 | abc xyz | | a | 1 | | b | | | c | 3 | @@ -151,6 +153,8 @@ Scenario: adding feature flags for an unhandled error Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier And the event contains the following feature flags: | featureFlag | variant | + | from config 1 | | + | from config 2 | abc xyz | | x | | | y | 1234 | | z | | From f1f416e3b640e8a83e64ec2b248ce3e7dcb2800f Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 21 Nov 2022 10:24:00 +0000 Subject: [PATCH 17/24] Add global flag in Rack tests --- features/fixtures/rack/app/app.rb | 2 ++ features/rack.feature | 2 ++ 2 files changed, 4 insertions(+) diff --git a/features/fixtures/rack/app/app.rb b/features/fixtures/rack/app/app.rb index 9b9e63b1..e606b9bc 100644 --- a/features/fixtures/rack/app/app.rb +++ b/features/fixtures/rack/app/app.rb @@ -17,6 +17,8 @@ ]) end +Bugsnag.add_feature_flag('from global', '123') + class BugsnagTests def call(env) req = Rack::Request.new(env) diff --git a/features/rack.feature b/features/rack.feature index 3827dae8..79172bda 100644 --- a/features/rack.feature +++ b/features/rack.feature @@ -139,6 +139,7 @@ Scenario: adding feature flags for an unhandled error Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier And the event contains the following feature flags: | featureFlag | variant | + | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | a | 1 | @@ -153,6 +154,7 @@ Scenario: adding feature flags for an unhandled error Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier And the event contains the following feature flags: | featureFlag | variant | + | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | x | | From 7ff633c22e19e9fe5e49c55edb618825150e9ce1 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 21 Nov 2022 10:25:30 +0000 Subject: [PATCH 18/24] Add flags to configuration in Rails tests --- .../app/app/controllers/feature_flags_controller.rb | 2 ++ .../fixtures/rails3/app/config/initializers/bugsnag.rb | 6 ++++++ .../app/app/controllers/feature_flags_controller.rb | 2 ++ .../fixtures/rails4/app/config/initializers/bugsnag.rb | 6 ++++++ .../app/app/controllers/feature_flags_controller.rb | 2 ++ .../fixtures/rails5/app/config/initializers/bugsnag.rb | 6 ++++++ .../app/app/controllers/feature_flags_controller.rb | 2 ++ .../fixtures/rails6/app/config/initializers/bugsnag.rb | 6 ++++++ .../app/app/controllers/feature_flags_controller.rb | 2 ++ .../fixtures/rails7/app/config/initializers/bugsnag.rb | 6 ++++++ features/rails_features/feature_flags.feature | 10 ++++++++++ 11 files changed, 50 insertions(+) diff --git a/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb index d5e37750..c58c0279 100644 --- a/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb @@ -22,6 +22,8 @@ def add_feature_flags(event) if params.key?('clear_all_flags') event.clear_feature_flags + else + event.clear_feature_flag('should be removed!') end end end diff --git a/features/fixtures/rails3/app/config/initializers/bugsnag.rb b/features/fixtures/rails3/app/config/initializers/bugsnag.rb index c044fb40..03ada9ac 100644 --- a/features/fixtures/rails3/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails3/app/config/initializers/bugsnag.rb @@ -32,4 +32,10 @@ report.request[:params][:another_thing] = "hi" end) end + + config.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + Bugsnag::FeatureFlag.new('should be removed!'), + ]) end diff --git a/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb index d5e37750..c58c0279 100644 --- a/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb @@ -22,6 +22,8 @@ def add_feature_flags(event) if params.key?('clear_all_flags') event.clear_feature_flags + else + event.clear_feature_flag('should be removed!') end end end diff --git a/features/fixtures/rails4/app/config/initializers/bugsnag.rb b/features/fixtures/rails4/app/config/initializers/bugsnag.rb index c044fb40..03ada9ac 100644 --- a/features/fixtures/rails4/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails4/app/config/initializers/bugsnag.rb @@ -32,4 +32,10 @@ report.request[:params][:another_thing] = "hi" end) end + + config.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + Bugsnag::FeatureFlag.new('should be removed!'), + ]) end diff --git a/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb index ffb4007b..5d7947b4 100644 --- a/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb @@ -20,6 +20,8 @@ def add_feature_flags(event) if params.key?('clear_all_flags') event.clear_feature_flags + else + event.clear_feature_flag('should be removed!') end end end diff --git a/features/fixtures/rails5/app/config/initializers/bugsnag.rb b/features/fixtures/rails5/app/config/initializers/bugsnag.rb index c92eb03e..7fa941f7 100644 --- a/features/fixtures/rails5/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails5/app/config/initializers/bugsnag.rb @@ -32,4 +32,10 @@ report.request[:params][:another_thing] = "hi" end) end + + config.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + Bugsnag::FeatureFlag.new('should be removed!'), + ]) end diff --git a/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb index ffb4007b..5d7947b4 100644 --- a/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb @@ -20,6 +20,8 @@ def add_feature_flags(event) if params.key?('clear_all_flags') event.clear_feature_flags + else + event.clear_feature_flag('should be removed!') end end end diff --git a/features/fixtures/rails6/app/config/initializers/bugsnag.rb b/features/fixtures/rails6/app/config/initializers/bugsnag.rb index c044fb40..03ada9ac 100644 --- a/features/fixtures/rails6/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails6/app/config/initializers/bugsnag.rb @@ -32,4 +32,10 @@ report.request[:params][:another_thing] = "hi" end) end + + config.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + Bugsnag::FeatureFlag.new('should be removed!'), + ]) end diff --git a/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb index ffb4007b..5d7947b4 100644 --- a/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb @@ -20,6 +20,8 @@ def add_feature_flags(event) if params.key?('clear_all_flags') event.clear_feature_flags + else + event.clear_feature_flag('should be removed!') end end end diff --git a/features/fixtures/rails7/app/config/initializers/bugsnag.rb b/features/fixtures/rails7/app/config/initializers/bugsnag.rb index 4c2b7658..0a373c4b 100644 --- a/features/fixtures/rails7/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails7/app/config/initializers/bugsnag.rb @@ -32,4 +32,10 @@ report.request[:params][:another_thing] = "hi" end) end + + config.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + Bugsnag::FeatureFlag.new('should be removed!'), + ]) end diff --git a/features/rails_features/feature_flags.feature b/features/rails_features/feature_flags.feature index af7c0106..c17ffeeb 100644 --- a/features/rails_features/feature_flags.feature +++ b/features/rails_features/feature_flags.feature @@ -12,6 +12,8 @@ Scenario: adding feature flags for an unhandled error And the exception "message" equals "oh no" And the event contains the following feature flags: | featureFlag | variant | + | from config 1 | | + | from config 2 | abc xyz | | a | 1 | | b | | | c | 3 | @@ -22,6 +24,8 @@ Scenario: adding feature flags for an unhandled error And I wait to receive an error And the event contains the following feature flags: | featureFlag | variant | + | from config 1 | | + | from config 2 | abc xyz | | x | 9 | | y | | | z | 7 | @@ -38,6 +42,8 @@ Scenario: adding feature flags for a handled error And the exception "message" equals "ahhh" And the event contains the following feature flags: | featureFlag | variant | + | from config 1 | | + | from config 2 | abc xyz | | ab | 12 | | cd | 34 | # ensure each request can have its own set of feature flags @@ -46,6 +52,8 @@ Scenario: adding feature flags for a handled error And I wait to receive an error And the event contains the following feature flags: | featureFlag | variant | + | from config 1 | | + | from config 2 | abc xyz | | e | h | | f | i | | g | | @@ -66,6 +74,8 @@ Scenario: clearing all feature flags doesn't affect subsequent requests And I wait to receive an error And the event contains the following feature flags: | featureFlag | variant | + | from config 1 | | + | from config 2 | abc xyz | | x | 9 | | y | | | z | 7 | From d32fe0944f74e35e3a69a406aa462b358c5b7849 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 21 Nov 2022 10:26:36 +0000 Subject: [PATCH 19/24] Add global flag in Rails tests --- features/fixtures/rails3/app/config/initializers/bugsnag.rb | 2 ++ features/fixtures/rails4/app/config/initializers/bugsnag.rb | 2 ++ features/fixtures/rails5/app/config/initializers/bugsnag.rb | 2 ++ features/fixtures/rails6/app/config/initializers/bugsnag.rb | 2 ++ features/fixtures/rails7/app/config/initializers/bugsnag.rb | 2 ++ features/rails_features/feature_flags.feature | 5 +++++ 6 files changed, 15 insertions(+) diff --git a/features/fixtures/rails3/app/config/initializers/bugsnag.rb b/features/fixtures/rails3/app/config/initializers/bugsnag.rb index 03ada9ac..7ef606dc 100644 --- a/features/fixtures/rails3/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails3/app/config/initializers/bugsnag.rb @@ -39,3 +39,5 @@ Bugsnag::FeatureFlag.new('should be removed!'), ]) end + +Bugsnag.add_feature_flag('from global', '123') diff --git a/features/fixtures/rails4/app/config/initializers/bugsnag.rb b/features/fixtures/rails4/app/config/initializers/bugsnag.rb index 03ada9ac..7ef606dc 100644 --- a/features/fixtures/rails4/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails4/app/config/initializers/bugsnag.rb @@ -39,3 +39,5 @@ Bugsnag::FeatureFlag.new('should be removed!'), ]) end + +Bugsnag.add_feature_flag('from global', '123') diff --git a/features/fixtures/rails5/app/config/initializers/bugsnag.rb b/features/fixtures/rails5/app/config/initializers/bugsnag.rb index 7fa941f7..ebb7ea39 100644 --- a/features/fixtures/rails5/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails5/app/config/initializers/bugsnag.rb @@ -39,3 +39,5 @@ Bugsnag::FeatureFlag.new('should be removed!'), ]) end + +Bugsnag.add_feature_flag('from global', '123') diff --git a/features/fixtures/rails6/app/config/initializers/bugsnag.rb b/features/fixtures/rails6/app/config/initializers/bugsnag.rb index 03ada9ac..7ef606dc 100644 --- a/features/fixtures/rails6/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails6/app/config/initializers/bugsnag.rb @@ -39,3 +39,5 @@ Bugsnag::FeatureFlag.new('should be removed!'), ]) end + +Bugsnag.add_feature_flag('from global', '123') diff --git a/features/fixtures/rails7/app/config/initializers/bugsnag.rb b/features/fixtures/rails7/app/config/initializers/bugsnag.rb index 0a373c4b..2fc79edb 100644 --- a/features/fixtures/rails7/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails7/app/config/initializers/bugsnag.rb @@ -39,3 +39,5 @@ Bugsnag::FeatureFlag.new('should be removed!'), ]) end + +Bugsnag.add_feature_flag('from global', '123') diff --git a/features/rails_features/feature_flags.feature b/features/rails_features/feature_flags.feature index c17ffeeb..5c3a1962 100644 --- a/features/rails_features/feature_flags.feature +++ b/features/rails_features/feature_flags.feature @@ -12,6 +12,7 @@ Scenario: adding feature flags for an unhandled error And the exception "message" equals "oh no" And the event contains the following feature flags: | featureFlag | variant | + | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | a | 1 | @@ -24,6 +25,7 @@ Scenario: adding feature flags for an unhandled error And I wait to receive an error And the event contains the following feature flags: | featureFlag | variant | + | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | x | 9 | @@ -42,6 +44,7 @@ Scenario: adding feature flags for a handled error And the exception "message" equals "ahhh" And the event contains the following feature flags: | featureFlag | variant | + | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | ab | 12 | @@ -52,6 +55,7 @@ Scenario: adding feature flags for a handled error And I wait to receive an error And the event contains the following feature flags: | featureFlag | variant | + | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | e | h | @@ -74,6 +78,7 @@ Scenario: clearing all feature flags doesn't affect subsequent requests And I wait to receive an error And the event contains the following feature flags: | featureFlag | variant | + | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | x | 9 | From 7d943a8bd8049795a775e58fbf48e4f3884e9b52 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 22 Nov 2022 15:39:21 +0000 Subject: [PATCH 20/24] Store feature flags per-request This allows users to add feature flags to Bugsnag whenever they interact with their feature flag service, e.g. ```ruby flag = get_flag_from_service('new-login-page') if flag.enabled? Bugsnag.add_feature_flag('new-login-page') # render new login page else Bugsnag.clear_feature_flag('new-login-page') # render old login page end ``` The previous implementation relied on event's already being request specific, but meant feature flags would usually have to be added in an on_error callback --- features/fixtures/rack/app/app.rb | 67 ++++++++-------- .../controllers/feature_flags_controller.rb | 6 +- .../rails3/app/config/initializers/bugsnag.rb | 17 ++-- .../controllers/feature_flags_controller.rb | 6 +- .../rails4/app/config/initializers/bugsnag.rb | 17 ++-- .../controllers/feature_flags_controller.rb | 6 +- .../rails5/app/config/initializers/bugsnag.rb | 17 ++-- .../controllers/feature_flags_controller.rb | 6 +- .../rails6/app/config/initializers/bugsnag.rb | 17 ++-- .../controllers/feature_flags_controller.rb | 6 +- .../rails7/app/config/initializers/bugsnag.rb | 17 ++-- features/rack.feature | 2 - features/rails_features/feature_flags.feature | 18 ++--- lib/bugsnag/configuration.rb | 31 +++----- spec/configuration_spec.rb | 79 ++++++------------- 15 files changed, 155 insertions(+), 157 deletions(-) diff --git a/features/fixtures/rack/app/app.rb b/features/fixtures/rack/app/app.rb index e606b9bc..68963bdd 100644 --- a/features/fixtures/rack/app/app.rb +++ b/features/fixtures/rack/app/app.rb @@ -2,6 +2,8 @@ require 'rack' require 'json' +$clear_all_flags = false + Bugsnag.configure do |config| config.api_key = ENV['BUGSNAG_API_KEY'] config.endpoint = ENV['BUGSNAG_ENDPOINT'] @@ -10,19 +12,31 @@ config.meta_data_filters = JSON.parse(ENV['BUGSNAG_METADATA_FILTERS']) end - config.add_feature_flags([ - Bugsnag::FeatureFlag.new('from config 1'), - Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), - Bugsnag::FeatureFlag.new('should be removed!'), - ]) -end + config.add_feature_flag( + 'this flag is added outside of a request and so should never normally be visible', + 'if an event was reported in the global scope, this flag would be attached to it' + ) + + config.add_on_error(proc do |event| + event.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + ]) -Bugsnag.add_feature_flag('from global', '123') + event.clear_feature_flag('should be removed!') + + if $clear_all_flags + event.clear_feature_flags + end + end) +end class BugsnagTests def call(env) req = Rack::Request.new(env) + $clear_all_flags = !!req.params['clear_all_flags'] + case req.env['REQUEST_PATH'] when '/unhandled' raise 'Unhandled error' @@ -33,39 +47,28 @@ def call(env) Bugsnag.notify(e) end when '/feature-flags/unhandled' - Bugsnag.add_on_error(proc do |event| - event.add_feature_flag('a', '1') - - event.add_feature_flags([ - Bugsnag::FeatureFlag.new('b'), - Bugsnag::FeatureFlag.new('c', '3'), - ]) + Bugsnag.add_feature_flag('a', '1') - event.add_feature_flag('d') + Bugsnag.add_feature_flags([ + Bugsnag::FeatureFlag.new('b'), + Bugsnag::FeatureFlag.new('c', '3'), + ]) - event.clear_feature_flag('should be removed!') - - if req.params["clear_all_flags"] - event.clear_feature_flags - end - end) + Bugsnag.add_feature_flag('d') + Bugsnag.add_feature_flag('should be removed!') raise 'Unhandled error' when '/feature-flags/handled' - Bugsnag.notify(RuntimeError.new('oh no')) do |event| - event.add_feature_flag('x') + Bugsnag.add_feature_flag('x') - event.add_feature_flags([ - Bugsnag::FeatureFlag.new('y', '1234'), - Bugsnag::FeatureFlag.new('z'), - ]) + Bugsnag.add_feature_flags([ + Bugsnag::FeatureFlag.new('y', '1234'), + Bugsnag::FeatureFlag.new('z'), + ]) - event.clear_feature_flag('should be removed!') + Bugsnag.add_feature_flag('should be removed!') - if req.params["clear_all_flags"] - event.clear_feature_flags - end - end + Bugsnag.notify(RuntimeError.new('oh no')) end res = Rack::Response.new diff --git a/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb index c58c0279..eaab63b1 100644 --- a/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails3/app/app/controllers/feature_flags_controller.rb @@ -4,10 +4,14 @@ class FeatureFlagsController < ActionController::Base before_bugsnag_notify :add_feature_flags def unhandled + Bugsnag.add_feature_flag('unhandled') + raise 'oh no' end def handled + Bugsnag.add_feature_flag('handled') + Bugsnag.notify(RuntimeError.new('ahhh')) render json: {} @@ -21,7 +25,7 @@ def add_feature_flags(event) end if params.key?('clear_all_flags') - event.clear_feature_flags + event.add_metadata(:clear_all_flags, :a, 1) else event.clear_feature_flag('should be removed!') end diff --git a/features/fixtures/rails3/app/config/initializers/bugsnag.rb b/features/fixtures/rails3/app/config/initializers/bugsnag.rb index 7ef606dc..0aabfa4c 100644 --- a/features/fixtures/rails3/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails3/app/config/initializers/bugsnag.rb @@ -33,11 +33,14 @@ end) end - config.add_feature_flags([ - Bugsnag::FeatureFlag.new('from config 1'), - Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), - Bugsnag::FeatureFlag.new('should be removed!'), - ]) -end + config.add_on_error(proc do |event| + event.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + ]) -Bugsnag.add_feature_flag('from global', '123') + if event.metadata.key?(:clear_all_flags) + event.clear_feature_flags + end + end) +end diff --git a/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb index c58c0279..eaab63b1 100644 --- a/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails4/app/app/controllers/feature_flags_controller.rb @@ -4,10 +4,14 @@ class FeatureFlagsController < ActionController::Base before_bugsnag_notify :add_feature_flags def unhandled + Bugsnag.add_feature_flag('unhandled') + raise 'oh no' end def handled + Bugsnag.add_feature_flag('handled') + Bugsnag.notify(RuntimeError.new('ahhh')) render json: {} @@ -21,7 +25,7 @@ def add_feature_flags(event) end if params.key?('clear_all_flags') - event.clear_feature_flags + event.add_metadata(:clear_all_flags, :a, 1) else event.clear_feature_flag('should be removed!') end diff --git a/features/fixtures/rails4/app/config/initializers/bugsnag.rb b/features/fixtures/rails4/app/config/initializers/bugsnag.rb index 7ef606dc..0aabfa4c 100644 --- a/features/fixtures/rails4/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails4/app/config/initializers/bugsnag.rb @@ -33,11 +33,14 @@ end) end - config.add_feature_flags([ - Bugsnag::FeatureFlag.new('from config 1'), - Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), - Bugsnag::FeatureFlag.new('should be removed!'), - ]) -end + config.add_on_error(proc do |event| + event.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + ]) -Bugsnag.add_feature_flag('from global', '123') + if event.metadata.key?(:clear_all_flags) + event.clear_feature_flags + end + end) +end diff --git a/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb index 5d7947b4..1433d586 100644 --- a/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails5/app/app/controllers/feature_flags_controller.rb @@ -4,10 +4,14 @@ class FeatureFlagsController < ActionController::Base before_bugsnag_notify :add_feature_flags def unhandled + Bugsnag.add_feature_flag('unhandled') + raise 'oh no' end def handled + Bugsnag.add_feature_flag('handled') + Bugsnag.notify(RuntimeError.new('ahhh')) end @@ -19,7 +23,7 @@ def add_feature_flags(event) end if params.key?('clear_all_flags') - event.clear_feature_flags + event.add_metadata(:clear_all_flags, :a, 1) else event.clear_feature_flag('should be removed!') end diff --git a/features/fixtures/rails5/app/config/initializers/bugsnag.rb b/features/fixtures/rails5/app/config/initializers/bugsnag.rb index ebb7ea39..5d26472f 100644 --- a/features/fixtures/rails5/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails5/app/config/initializers/bugsnag.rb @@ -33,11 +33,14 @@ end) end - config.add_feature_flags([ - Bugsnag::FeatureFlag.new('from config 1'), - Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), - Bugsnag::FeatureFlag.new('should be removed!'), - ]) -end + config.add_on_error(proc do |event| + event.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + ]) -Bugsnag.add_feature_flag('from global', '123') + if event.metadata.key?(:clear_all_flags) + event.clear_feature_flags + end + end) +end diff --git a/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb index 5d7947b4..1433d586 100644 --- a/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails6/app/app/controllers/feature_flags_controller.rb @@ -4,10 +4,14 @@ class FeatureFlagsController < ActionController::Base before_bugsnag_notify :add_feature_flags def unhandled + Bugsnag.add_feature_flag('unhandled') + raise 'oh no' end def handled + Bugsnag.add_feature_flag('handled') + Bugsnag.notify(RuntimeError.new('ahhh')) end @@ -19,7 +23,7 @@ def add_feature_flags(event) end if params.key?('clear_all_flags') - event.clear_feature_flags + event.add_metadata(:clear_all_flags, :a, 1) else event.clear_feature_flag('should be removed!') end diff --git a/features/fixtures/rails6/app/config/initializers/bugsnag.rb b/features/fixtures/rails6/app/config/initializers/bugsnag.rb index 7ef606dc..0aabfa4c 100644 --- a/features/fixtures/rails6/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails6/app/config/initializers/bugsnag.rb @@ -33,11 +33,14 @@ end) end - config.add_feature_flags([ - Bugsnag::FeatureFlag.new('from config 1'), - Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), - Bugsnag::FeatureFlag.new('should be removed!'), - ]) -end + config.add_on_error(proc do |event| + event.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + ]) -Bugsnag.add_feature_flag('from global', '123') + if event.metadata.key?(:clear_all_flags) + event.clear_feature_flags + end + end) +end diff --git a/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb b/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb index 5d7947b4..1433d586 100644 --- a/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb +++ b/features/fixtures/rails7/app/app/controllers/feature_flags_controller.rb @@ -4,10 +4,14 @@ class FeatureFlagsController < ActionController::Base before_bugsnag_notify :add_feature_flags def unhandled + Bugsnag.add_feature_flag('unhandled') + raise 'oh no' end def handled + Bugsnag.add_feature_flag('handled') + Bugsnag.notify(RuntimeError.new('ahhh')) end @@ -19,7 +23,7 @@ def add_feature_flags(event) end if params.key?('clear_all_flags') - event.clear_feature_flags + event.add_metadata(:clear_all_flags, :a, 1) else event.clear_feature_flag('should be removed!') end diff --git a/features/fixtures/rails7/app/config/initializers/bugsnag.rb b/features/fixtures/rails7/app/config/initializers/bugsnag.rb index 2fc79edb..a7079f0a 100644 --- a/features/fixtures/rails7/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails7/app/config/initializers/bugsnag.rb @@ -33,11 +33,14 @@ end) end - config.add_feature_flags([ - Bugsnag::FeatureFlag.new('from config 1'), - Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), - Bugsnag::FeatureFlag.new('should be removed!'), - ]) -end + config.add_on_error(proc do |event| + event.add_feature_flags([ + Bugsnag::FeatureFlag.new('from config 1'), + Bugsnag::FeatureFlag.new('from config 2', 'abc xyz'), + ]) -Bugsnag.add_feature_flag('from global', '123') + if event.metadata.key?(:clear_all_flags) + event.clear_feature_flags + end + end) +end diff --git a/features/rack.feature b/features/rack.feature index 79172bda..3827dae8 100644 --- a/features/rack.feature +++ b/features/rack.feature @@ -139,7 +139,6 @@ Scenario: adding feature flags for an unhandled error Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier And the event contains the following feature flags: | featureFlag | variant | - | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | a | 1 | @@ -154,7 +153,6 @@ Scenario: adding feature flags for an unhandled error Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier And the event contains the following feature flags: | featureFlag | variant | - | from global | 123 | | from config 1 | | | from config 2 | abc xyz | | x | | diff --git a/features/rails_features/feature_flags.feature b/features/rails_features/feature_flags.feature index 5c3a1962..3e18c266 100644 --- a/features/rails_features/feature_flags.feature +++ b/features/rails_features/feature_flags.feature @@ -12,7 +12,7 @@ Scenario: adding feature flags for an unhandled error And the exception "message" equals "oh no" And the event contains the following feature flags: | featureFlag | variant | - | from global | 123 | + | unhandled | | | from config 1 | | | from config 2 | abc xyz | | a | 1 | @@ -23,9 +23,9 @@ Scenario: adding feature flags for an unhandled error When I discard the oldest error And I navigate to the route "/features/unhandled?flags[x]=9&flags[y]&flags[z]=7" on the rails app And I wait to receive an error - And the event contains the following feature flags: + Then the event contains the following feature flags: | featureFlag | variant | - | from global | 123 | + | unhandled | | | from config 1 | | | from config 2 | abc xyz | | x | 9 | @@ -44,18 +44,18 @@ Scenario: adding feature flags for a handled error And the exception "message" equals "ahhh" And the event contains the following feature flags: | featureFlag | variant | - | from global | 123 | + | handled | | | from config 1 | | | from config 2 | abc xyz | | ab | 12 | | cd | 34 | # ensure each request can have its own set of feature flags When I discard the oldest error - And I navigate to the route "/features/unhandled?flags[e]=h&flags[f]=i&flags[g]" on the rails app + And I navigate to the route "/features/handled?flags[e]=h&flags[f]=i&flags[g]" on the rails app And I wait to receive an error - And the event contains the following feature flags: + Then the event contains the following feature flags: | featureFlag | variant | - | from global | 123 | + | handled | | | from config 1 | | | from config 2 | abc xyz | | e | h | @@ -76,9 +76,9 @@ Scenario: clearing all feature flags doesn't affect subsequent requests When I discard the oldest error And I navigate to the route "/features/unhandled?flags[x]=9&flags[y]&flags[z]=7" on the rails app And I wait to receive an error - And the event contains the following feature flags: + Then the event contains the following feature flags: | featureFlag | variant | - | from global | 123 | + | unhandled | | | from config 1 | | | from config 2 | abc xyz | | x | 9 | diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 0dc50caf..2e71d136 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -186,11 +186,6 @@ class Configuration # @return [Breadcrumbs::OnBreadcrumbCallbackList] attr_reader :on_breadcrumb_callbacks - # Expose feature_flag_delegate internally for creating new events - # @api private - # @return [FeatureFlagDelegate] - attr_reader :feature_flag_delegate - API_KEY_REGEX = /[0-9a-f]{32}/i THREAD_LOCAL_NAME = "bugsnag_req_data" @@ -294,8 +289,6 @@ def initialize self.middleware = Bugsnag::MiddlewareStack.new self.middleware.use Bugsnag::Middleware::Callbacks - - @feature_flag_delegate = Bugsnag::Utility::FeatureFlagDelegate.new end ## @@ -671,15 +664,21 @@ def clear_metadata(section, *args) end end + # Expose the feature flag delegate internally for use when creating new Events + # + # @return [Bugsnag::Utility::FeatureFlagDelegate] + # @api private + def feature_flag_delegate + request_data[:feature_flag_delegate] ||= Bugsnag::Utility::FeatureFlagDelegate.new + end + # Add a feature flag with the given name & variant # # @param name [String] # @param variant [String, nil] # @return [void] def add_feature_flag(name, variant = nil) - @mutex.synchronize do - @feature_flag_delegate.add(name, variant) - end + feature_flag_delegate.add(name, variant) end # Merge the given array of FeatureFlag instances into the stored feature @@ -691,9 +690,7 @@ def add_feature_flag(name, variant = nil) # @param feature_flags [Array] # @return [void] def add_feature_flags(feature_flags) - @mutex.synchronize do - @feature_flag_delegate.merge(feature_flags) - end + feature_flag_delegate.merge(feature_flags) end # Remove the stored flag with the given name @@ -701,18 +698,14 @@ def add_feature_flags(feature_flags) # @param name [String] # @return [void] def clear_feature_flag(name) - @mutex.synchronize do - @feature_flag_delegate.remove(name) - end + feature_flag_delegate.remove(name) end # Remove all the stored flags # # @return [void] def clear_feature_flags - @mutex.synchronize do - @feature_flag_delegate.clear - end + feature_flag_delegate.clear end ## diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 73e41210..b78a6c12 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -747,72 +747,41 @@ def output_lines end end - describe "concurrent access" do - it "can handle multiple threads adding feature flags" do - configuration = Bugsnag::Configuration.new - - threads = 5.times.map do |i| - Thread.new do - configuration.add_feature_flag("thread_#{i} flag 1", i) - end - end + it "has a set of feature flags per-thread" do + configuration = Bugsnag::Configuration.new - threads += 5.times.map do |i| - Thread.new do - configuration.add_feature_flags([ - Bugsnag::FeatureFlag.new("thread_#{i} flag 2", i * 100), - Bugsnag::FeatureFlag.new("thread_#{i} flag 3", i * 100 + 1), - ]) - end - end + threads = 5.times.map do |i| + Thread.new do + expect(configuration.feature_flag_delegate.to_a).to be_empty - threads.shuffle.map(&:join) + configuration.add_feature_flag("thread_#{i} flag 1", i) - flags = configuration.feature_flag_delegate.to_a.sort do |a, b| - a.name <=> b.name + expect(configuration.feature_flag_delegate.to_a).to eq([ + Bugsnag::FeatureFlag.new("thread_#{i} flag 1", i), + ]) end - - expect(flags).to eq([ - Bugsnag::FeatureFlag.new('thread_0 flag 1', 0), - Bugsnag::FeatureFlag.new('thread_0 flag 2', 0), - Bugsnag::FeatureFlag.new('thread_0 flag 3', 1), - Bugsnag::FeatureFlag.new('thread_1 flag 1', 1), - Bugsnag::FeatureFlag.new('thread_1 flag 2', 100), - Bugsnag::FeatureFlag.new('thread_1 flag 3', 101), - Bugsnag::FeatureFlag.new('thread_2 flag 1', 2), - Bugsnag::FeatureFlag.new('thread_2 flag 2', 200), - Bugsnag::FeatureFlag.new('thread_2 flag 3', 201), - Bugsnag::FeatureFlag.new('thread_3 flag 1', 3), - Bugsnag::FeatureFlag.new('thread_3 flag 2', 300), - Bugsnag::FeatureFlag.new('thread_3 flag 3', 301), - Bugsnag::FeatureFlag.new('thread_4 flag 1', 4), - Bugsnag::FeatureFlag.new('thread_4 flag 2', 400), - Bugsnag::FeatureFlag.new('thread_4 flag 3', 401), - ]) end - it "can handle multiple threads clearing feature flags" do - configuration = Bugsnag::Configuration.new - - configuration.add_feature_flag('abc') - configuration.add_feature_flag('xyz') + threads += 5.times.map do |i| + Thread.new do + expect(configuration.feature_flag_delegate.to_a).to be_empty - 5.times do |i| - configuration.add_feature_flag("thread_#{i}", i) - end + configuration.add_feature_flags([ + Bugsnag::FeatureFlag.new("thread_#{i} flag 2", i * 100), + Bugsnag::FeatureFlag.new("thread_#{i} flag 3", i * 100 + 1), + ]) - threads = 5.times.map do |i| - Thread.new do - configuration.clear_feature_flag("thread_#{i}") - configuration.clear_feature_flag('abc') - configuration.clear_feature_flag('xyz') - end + expect(configuration.feature_flag_delegate.to_a).to eq([ + Bugsnag::FeatureFlag.new("thread_#{i} flag 2", i * 100), + Bugsnag::FeatureFlag.new("thread_#{i} flag 3", i * 100 + 1), + ]) end + end - threads.shuffle.map(&:join) + threads.shuffle.map(&:join) - expect(configuration.feature_flag_delegate.to_a).to be_empty - end + # we added no flags in this thread, so there should be none + expect(configuration.feature_flag_delegate.to_a).to be_empty end end end From b18801e0f6c29ecfe90a4c41cc44916c9101c7a7 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 23 Nov 2022 11:51:06 +0000 Subject: [PATCH 21/24] Refactor feature flag methods into a new module Now that the implementations of add_/clear_feature_flag(s) are the same, we can use a module to implement these methods and save us from having copy pasted methods/docs in 3 places --- lib/bugsnag.rb | 46 +++++------------------ lib/bugsnag/configuration.rb | 38 +------------------ lib/bugsnag/report.rb | 46 ++++------------------- lib/bugsnag/utility/feature_data_store.rb | 41 ++++++++++++++++++++ 4 files changed, 60 insertions(+), 111 deletions(-) create mode 100644 lib/bugsnag/utility/feature_data_store.rb diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 2cbec407..57471265 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -2,6 +2,7 @@ require "thread" require "bugsnag/version" +require "bugsnag/utility/feature_data_store" require "bugsnag/configuration" require "bugsnag/meta_data" require "bugsnag/report" @@ -48,6 +49,8 @@ module Bugsnag NIL_EXCEPTION_DESCRIPTION = "'nil' was notified as an exception" class << self + include Utility::FeatureDataStore + ## # Configure the Bugsnag notifier application-wide settings. # @@ -429,42 +432,6 @@ def clear_metadata(section, *args) configuration.clear_metadata(section, *args) end - # Add a feature flag with the given name & variant - # - # @param name [String] - # @param variant [String, nil] - # @return [void] - def add_feature_flag(name, variant = nil) - configuration.add_feature_flag(name, variant) - end - - # Merge the given array of FeatureFlag instances into the stored feature - # flags - # - # New flags will be appended to the array. Flags with the same name will be - # overwritten, but their position in the array will not change - # - # @param feature_flags [Array] - # @return [void] - def add_feature_flags(feature_flags) - configuration.add_feature_flags(feature_flags) - end - - # Remove the stored flag with the given name - # - # @param name [String] - # @return [void] - def clear_feature_flag(name) - configuration.clear_feature_flag(name) - end - - # Remove all the stored flags - # - # @return [void] - def clear_feature_flags - configuration.clear_feature_flags - end - private def should_deliver_notification?(exception, auto_notify) @@ -595,6 +562,13 @@ def unwrap_bundler_exception(exception) unwrapped.cause end + + # Expose the feature flag delegate for {Bugsnag::Utility::FeatureDataStore} + # + # @return [Bugsnag::Utility::FeatureFlagDelegate] + def feature_flag_delegate + configuration.feature_flag_delegate + end end end # rubocop:enable Metrics/ModuleLength diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 2e71d136..f54d520b 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -18,6 +18,8 @@ module Bugsnag class Configuration + include Utility::FeatureDataStore + # Your Integration API Key # @return [String, nil] attr_accessor :api_key @@ -672,42 +674,6 @@ def feature_flag_delegate request_data[:feature_flag_delegate] ||= Bugsnag::Utility::FeatureFlagDelegate.new end - # Add a feature flag with the given name & variant - # - # @param name [String] - # @param variant [String, nil] - # @return [void] - def add_feature_flag(name, variant = nil) - feature_flag_delegate.add(name, variant) - end - - # Merge the given array of FeatureFlag instances into the stored feature - # flags - # - # New flags will be appended to the array. Flags with the same name will be - # overwritten, but their position in the array will not change - # - # @param feature_flags [Array] - # @return [void] - def add_feature_flags(feature_flags) - feature_flag_delegate.merge(feature_flags) - end - - # Remove the stored flag with the given name - # - # @param name [String] - # @return [void] - def clear_feature_flag(name) - feature_flag_delegate.remove(name) - end - - # Remove all the stored flags - # - # @return [void] - def clear_feature_flags - feature_flag_delegate.clear - end - ## # Has the context been explicitly set? # diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 02f7a765..8af18dd1 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -6,6 +6,8 @@ module Bugsnag # rubocop:todo Metrics/ClassLength class Report + include Utility::FeatureDataStore + NOTIFIER_NAME = "Ruby Bugsnag Notifier" NOTIFIER_VERSION = Bugsnag::VERSION NOTIFIER_URL = "https://www.bugsnag.com" @@ -143,7 +145,7 @@ def initialize(exception, passed_configuration, auto_notify=false) self.user = {} @metadata_delegate = Utility::MetadataDelegate.new - @feature_flags = configuration.feature_flag_delegate.dup + @feature_flag_delegate = configuration.feature_flag_delegate.dup end ## @@ -221,7 +223,7 @@ def as_json time: @created_at }, exceptions: exceptions, - featureFlags: @feature_flags.as_json, + featureFlags: @feature_flag_delegate.as_json, groupingHash: grouping_hash, metaData: meta_data, session: session, @@ -364,43 +366,7 @@ def clear_metadata(section, *args) # # @return [Array] def feature_flags - @feature_flags.to_a - end - - # Add a feature flag with the given name & variant - # - # @param name [String] - # @param variant [String, nil] - # @return [void] - def add_feature_flag(name, variant = nil) - @feature_flags.add(name, variant) - end - - # Merge the given array of FeatureFlag instances into the stored feature - # flags - # - # New flags will be appended to the array. Flags with the same name will be - # overwritten, but their position in the array will not change - # - # @param feature_flags [Array] - # @return [void] - def add_feature_flags(feature_flags) - @feature_flags.merge(feature_flags) - end - - # Remove the stored flag with the given name - # - # @param name [String] - # @return [void] - def clear_feature_flag(name) - @feature_flags.remove(name) - end - - # Remove all the stored flags - # - # @return [void] - def clear_feature_flags - @feature_flags.clear + @feature_flag_delegate.to_a end ## @@ -439,6 +405,8 @@ def unhandled_overridden? private + attr_reader :feature_flag_delegate + def update_handled_counts(is_unhandled, was_unhandled) # do nothing if there is no session to update return if @session.nil? diff --git a/lib/bugsnag/utility/feature_data_store.rb b/lib/bugsnag/utility/feature_data_store.rb new file mode 100644 index 00000000..c72939cf --- /dev/null +++ b/lib/bugsnag/utility/feature_data_store.rb @@ -0,0 +1,41 @@ +module Bugsnag::Utility + # @abstract Requires a #feature_flag_delegate method returning a + # {Bugsnag::Utility::FeatureFlagDelegate} + module FeatureDataStore + # Add a feature flag with the given name & variant + # + # @param name [String] + # @param variant [String, nil] + # @return [void] + def add_feature_flag(name, variant = nil) + feature_flag_delegate.add(name, variant) + end + + # Merge the given array of FeatureFlag instances into the stored feature + # flags + # + # New flags will be appended to the array. Flags with the same name will be + # overwritten, but their position in the array will not change + # + # @param feature_flags [Array] + # @return [void] + def add_feature_flags(feature_flags) + feature_flag_delegate.merge(feature_flags) + end + + # Remove the stored flag with the given name + # + # @param name [String] + # @return [void] + def clear_feature_flag(name) + feature_flag_delegate.remove(name) + end + + # Remove all the stored flags + # + # @return [void] + def clear_feature_flags + feature_flag_delegate.clear + end + end +end From 08b3be4236563ed9447a5e7b45975f9afe3da53c Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 23 Nov 2022 11:52:38 +0000 Subject: [PATCH 22/24] Enable YARD's 'embed-mixins' option This embeds methods from included modules directly into the class' docs, rather than linking to them separately This makes the docs a lot clearer when a class includes some methods from a module as they are now listed next to every other method --- .yardopts | 1 + 1 file changed, 1 insertion(+) diff --git a/.yardopts b/.yardopts index b1c9cc2b..b8575d80 100644 --- a/.yardopts +++ b/.yardopts @@ -4,6 +4,7 @@ --no-private --protected --title "bugsnag-ruby API Documentation" +--embed-mixins lib/**/*.rb - README.md From 92d2c5c6b186659e883483a1f17382274e2f7d7e Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 24 Nov 2022 11:42:48 +0000 Subject: [PATCH 23/24] Remove feature flag API from Configuration --- features/fixtures/rack/app/app.rb | 5 --- lib/bugsnag.rb | 17 +++++--- lib/bugsnag/configuration.rb | 10 ----- lib/bugsnag/report.rb | 2 +- spec/bugsnag_spec.rb | 47 +++++++++++++++++--- spec/configuration_spec.rb | 72 ------------------------------- spec/report_spec.rb | 22 +++++----- 7 files changed, 63 insertions(+), 112 deletions(-) diff --git a/features/fixtures/rack/app/app.rb b/features/fixtures/rack/app/app.rb index 68963bdd..847c7435 100644 --- a/features/fixtures/rack/app/app.rb +++ b/features/fixtures/rack/app/app.rb @@ -12,11 +12,6 @@ config.meta_data_filters = JSON.parse(ENV['BUGSNAG_METADATA_FILTERS']) end - config.add_feature_flag( - 'this flag is added outside of a request and so should never normally be visible', - 'if an event was reported in the global scope, this flag would be attached to it' - ) - config.add_on_error(proc do |event| event.add_feature_flags([ Bugsnag::FeatureFlag.new('from config 1'), diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 57471265..a25ed6b4 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -432,6 +432,16 @@ def clear_metadata(section, *args) configuration.clear_metadata(section, *args) end + # Expose the feature flag delegate internally for use when creating new Events + # + # The Bugsnag module's feature_flag_delegate is request-specific + # + # @return [Bugsnag::Utility::FeatureFlagDelegate] + # @api private + def feature_flag_delegate + configuration.request_data[:feature_flag_delegate] ||= Utility::FeatureFlagDelegate.new + end + private def should_deliver_notification?(exception, auto_notify) @@ -562,13 +572,6 @@ def unwrap_bundler_exception(exception) unwrapped.cause end - - # Expose the feature flag delegate for {Bugsnag::Utility::FeatureDataStore} - # - # @return [Bugsnag::Utility::FeatureFlagDelegate] - def feature_flag_delegate - configuration.feature_flag_delegate - end end end # rubocop:enable Metrics/ModuleLength diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index f54d520b..36dda53d 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -18,8 +18,6 @@ module Bugsnag class Configuration - include Utility::FeatureDataStore - # Your Integration API Key # @return [String, nil] attr_accessor :api_key @@ -666,14 +664,6 @@ def clear_metadata(section, *args) end end - # Expose the feature flag delegate internally for use when creating new Events - # - # @return [Bugsnag::Utility::FeatureFlagDelegate] - # @api private - def feature_flag_delegate - request_data[:feature_flag_delegate] ||= Bugsnag::Utility::FeatureFlagDelegate.new - end - ## # Has the context been explicitly set? # diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 8af18dd1..ca635395 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -145,7 +145,7 @@ def initialize(exception, passed_configuration, auto_notify=false) self.user = {} @metadata_delegate = Utility::MetadataDelegate.new - @feature_flag_delegate = configuration.feature_flag_delegate.dup + @feature_flag_delegate = Bugsnag.feature_flag_delegate.dup end ## diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 4d27eee4..9487b9a8 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -1144,7 +1144,7 @@ module Kernel } end - it "does not mutate the global feature flags if more flags are added" do + it "does not mutate the Bugsnag module's feature flags if more flags are added" do Bugsnag.add_feature_flag('abc') Bugsnag.add_feature_flag('xyz', '123') @@ -1160,14 +1160,14 @@ module Kernel { "featureFlag" => "another one" }, ]) - expect(Bugsnag.configuration.feature_flag_delegate.as_json).to eq([ + expect(Bugsnag.feature_flag_delegate.as_json).to eq([ { "featureFlag" => "abc" }, { "featureFlag" => "xyz", "variant" => "123" }, ]) } end - it "does not mutate the global feature flags if flags are removed" do + it "does not mutate the Bugsnag module's feature flags if flags are removed" do Bugsnag.add_feature_flag('abc') Bugsnag.add_feature_flag('xyz', '123') @@ -1179,14 +1179,14 @@ module Kernel event = get_event_from_payload(payload) expect(event["featureFlags"]).to be_empty - expect(Bugsnag.configuration.feature_flag_delegate.as_json).to eq([ + expect(Bugsnag.feature_flag_delegate.as_json).to eq([ { "featureFlag" => "abc" }, { "featureFlag" => "xyz", "variant" => "123" }, ]) } end - it "does not mutate the event's feature flags if global flags are removed" do + it "does not mutate the event's feature flags if the Bugsnag module's flags are removed" do Bugsnag.add_feature_flags([ Bugsnag::FeatureFlag.new('abc'), Bugsnag::FeatureFlag.new('xyz', 123), @@ -1203,8 +1203,43 @@ module Kernel { "featureFlag" => "xyz", "variant" => "123" }, ]) - expect(Bugsnag.configuration.feature_flag_delegate.as_json).to be_empty + expect(Bugsnag.feature_flag_delegate.as_json).to be_empty } end + + it "stores feature flags per-thread" do + threads = 5.times.map do |i| + Thread.new do + expect(Bugsnag.feature_flag_delegate.to_a).to be_empty + + Bugsnag.add_feature_flag("thread_#{i} flag 1", i) + + expect(Bugsnag.feature_flag_delegate.to_a).to eq([ + Bugsnag::FeatureFlag.new("thread_#{i} flag 1", i), + ]) + end + end + + threads += 5.times.map do |i| + Thread.new do + expect(Bugsnag.feature_flag_delegate.to_a).to be_empty + + Bugsnag.add_feature_flags([ + Bugsnag::FeatureFlag.new("thread_#{i} flag 2", i * 100), + Bugsnag::FeatureFlag.new("thread_#{i} flag 3", i * 100 + 1), + ]) + + expect(Bugsnag.feature_flag_delegate.to_a).to eq([ + Bugsnag::FeatureFlag.new("thread_#{i} flag 2", i * 100), + Bugsnag::FeatureFlag.new("thread_#{i} flag 3", i * 100 + 1), + ]) + end + end + + threads.shuffle.each(&:join) + + # we added no flags in this thread, so there should be none + expect(Bugsnag.feature_flag_delegate.to_a).to be_empty + end end end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index b78a6c12..df0567ea 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -712,76 +712,4 @@ def output_lines end end end - - describe "feature flags" do - describe "#feature_flag_delegate" do - it "is initially empty" do - expect(subject.feature_flag_delegate.to_a).to be_empty - end - - it "cannot be reassigned" do - expect(subject).not_to respond_to(:feature_flag_delegate=) - end - - it "reflects changes in add/clear feature flags" do - subject.add_feature_flag('abc') - subject.add_feature_flags([ - Bugsnag::FeatureFlag.new('1'), - Bugsnag::FeatureFlag.new('2', 'z'), - Bugsnag::FeatureFlag.new('3'), - ]) - subject.add_feature_flag('xyz', '1234') - - subject.clear_feature_flag('3') - - expect(subject.feature_flag_delegate.to_a).to eq([ - Bugsnag::FeatureFlag.new('abc'), - Bugsnag::FeatureFlag.new('1'), - Bugsnag::FeatureFlag.new('2', 'z'), - Bugsnag::FeatureFlag.new('xyz', '1234'), - ]) - - subject.clear_feature_flags - - expect(subject.feature_flag_delegate.to_a).to be_empty - end - end - - it "has a set of feature flags per-thread" do - configuration = Bugsnag::Configuration.new - - threads = 5.times.map do |i| - Thread.new do - expect(configuration.feature_flag_delegate.to_a).to be_empty - - configuration.add_feature_flag("thread_#{i} flag 1", i) - - expect(configuration.feature_flag_delegate.to_a).to eq([ - Bugsnag::FeatureFlag.new("thread_#{i} flag 1", i), - ]) - end - end - - threads += 5.times.map do |i| - Thread.new do - expect(configuration.feature_flag_delegate.to_a).to be_empty - - configuration.add_feature_flags([ - Bugsnag::FeatureFlag.new("thread_#{i} flag 2", i * 100), - Bugsnag::FeatureFlag.new("thread_#{i} flag 3", i * 100 + 1), - ]) - - expect(configuration.feature_flag_delegate.to_a).to eq([ - Bugsnag::FeatureFlag.new("thread_#{i} flag 2", i * 100), - Bugsnag::FeatureFlag.new("thread_#{i} flag 3", i * 100 + 1), - ]) - end - end - - threads.shuffle.map(&:join) - - # we added no flags in this thread, so there should be none - expect(configuration.feature_flag_delegate.to_a).to be_empty - end - end end diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 4c1bc3d0..b226dcae 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -2109,9 +2109,9 @@ def to_s } end - it "includes the configuration's feature flags if present" do - Bugsnag.configuration.add_feature_flag('abc') - Bugsnag.configuration.add_feature_flag('xyz', '123') + it "includes the Bugsnag module's feature flags if present" do + Bugsnag.add_feature_flag('abc') + Bugsnag.add_feature_flag('xyz', '123') Bugsnag.notify(BugsnagTestException.new("It crashed")) @@ -2124,9 +2124,9 @@ def to_s } end - it "does not mutate the configuration's feature flags if more flags are added" do - Bugsnag.configuration.add_feature_flag('abc') - Bugsnag.configuration.add_feature_flag('xyz', '123') + it "does not mutate the Bugsnag module's feature flags if more flags are added" do + Bugsnag.add_feature_flag('abc') + Bugsnag.add_feature_flag('xyz', '123') Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| event.add_feature_flag('another one') @@ -2140,16 +2140,16 @@ def to_s { "featureFlag" => "another one" }, ]) - expect(Bugsnag.configuration.feature_flag_delegate.as_json).to eq([ + expect(Bugsnag.feature_flag_delegate.as_json).to eq([ { "featureFlag" => "abc" }, { "featureFlag" => "xyz", "variant" => "123" }, ]) } end - it "does not mutate the configuration's feature flags if flags are removed" do - Bugsnag.configuration.add_feature_flag('abc') - Bugsnag.configuration.add_feature_flag('xyz', '123') + it "does not mutate the Bugsnag module's feature flags if flags are removed" do + Bugsnag.add_feature_flag('abc') + Bugsnag.add_feature_flag('xyz', '123') Bugsnag.notify(BugsnagTestException.new("It crashed")) do |event| event.clear_feature_flags @@ -2159,7 +2159,7 @@ def to_s event = get_event_from_payload(payload) expect(event["featureFlags"]).to be_empty - expect(Bugsnag.configuration.feature_flag_delegate.as_json).to eq([ + expect(Bugsnag.feature_flag_delegate.as_json).to eq([ { "featureFlag" => "abc" }, { "featureFlag" => "xyz", "variant" => "123" }, ]) From 25784ea5a66036fe4fa640ab59d32c8e28d248cb Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 30 Nov 2022 14:58:50 +0000 Subject: [PATCH 24/24] Update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e88acb5f..d90981eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changelog ========= +## TBD + +### Enhancements + +* Add support for feature flags & experiments. For more information, please see https://docs.bugsnag.com/product/features-experiments + | [#758](https://github.com/bugsnag/bugsnag-ruby/pull/758) + ## v6.24.2 (21 January 2022) ### Fixes