diff --git a/lib/semian/adapter.rb b/lib/semian/adapter.rb index fb292226..c1ab1535 100644 --- a/lib/semian/adapter.rb +++ b/lib/semian/adapter.rb @@ -9,19 +9,25 @@ def semian_identifier end def semian_resource - @semian_resource ||= case semian_options + return @semian_resource if @semian_resource + + case semian_options when false - UnprotectedResource.new(semian_identifier) + @semian_resource = UnprotectedResource.new(semian_identifier) when nil Semian.logger.info("Semian is not configured for #{self.class.name}: #{semian_identifier}") - UnprotectedResource.new(semian_identifier) + @semian_resource = UnprotectedResource.new(semian_identifier) else options = semian_options.dup options.delete(:name) options[:consumer] = self options[:exceptions] ||= [] options[:exceptions] += resource_exceptions - ::Semian.retrieve_or_register(semian_identifier, **options) + resource = ::Semian.retrieve_or_register(semian_identifier, **options) + + @semian_resource = resource if options.fetch(:deterministic, true) + + resource end end @@ -53,7 +59,11 @@ def semian_options return @semian_options if defined? @semian_options options = raw_semian_options - @semian_options = options && options.map { |k, v| [k.to_sym, v] }.to_h + + symbolized_options = options && options.map { |k, v| [k.to_sym, v] }.to_h + symbolized_options.tap do + @semian_options = symbolized_options if (symbolized_options || {}).fetch(:deterministic, true) + end end def raw_semian_options diff --git a/lib/semian/net_http.rb b/lib/semian/net_http.rb index 01c621b3..3dac03c7 100644 --- a/lib/semian/net_http.rb +++ b/lib/semian/net_http.rb @@ -91,16 +91,37 @@ def disabled? end def connect - return super if disabled? + with_cleared_non_deterministic_options do + return super if disabled? - acquire_semian_resource(adapter: :http, scope: :connection) { super } + acquire_semian_resource(adapter: :http, scope: :connection) { super } + end end def transport_request(*) - return super if disabled? + with_cleared_non_deterministic_options do + return super if disabled? + + acquire_semian_resource(adapter: :http, scope: :query) do + handle_error_responses(super) + end + end + end + + def with_cleared_non_deterministic_options + unless @resource_acquisition_in_progress + @resource_acquisition_in_progress = true + resource_acquisition_started = true + end - acquire_semian_resource(adapter: :http, scope: :query) do - handle_error_responses(super) + yield + ensure + if @resource_acquisition_in_progress && resource_acquisition_started + unless @raw_semian_options&.fetch(:deterministic, true) + # Clear @raw_semian_options if the resource was flagged as non-deterministic. + @raw_semian_options = nil + end + @resource_acquisition_in_progress = false end end diff --git a/test/adapter_test.rb b/test/adapter_test.rb index f7744aac..018ae08f 100644 --- a/test/adapter_test.rb +++ b/test/adapter_test.rb @@ -91,6 +91,14 @@ def test_consumer_registration_does_not_prevent_gc end end + def test_does_not_memoize_non_deterministic_options + non_deterministic_client = Semian::NonDeterministicAdapterTestClient.new(quota: 0.5) + + refute_nil(non_deterministic_client.semian_resource) + assert_equal(4, non_deterministic_client.raw_semian_options[:success_threshold]) + assert_nil(non_deterministic_client.instance_variable_get("@semian_options")) + end + class MyAdapterError < StandardError include Semian::AdapterError end diff --git a/test/adapters/net_http_test.rb b/test/adapters/net_http_test.rb index 32aba441..21f89512 100644 --- a/test/adapters/net_http_test.rb +++ b/test/adapters/net_http_test.rb @@ -23,6 +23,10 @@ def setup destroy_all_semian_resources end + def teardown + Thread.current[:sub_resource_name] = nil + end + def test_semian_identifier with_server do with_semian_configuration do @@ -484,6 +488,31 @@ def test_semian_disabled_env ENV.delete("SEMIAN_DISABLED") end + def test_non_deterministic_config + options = proc do |host, port| + DEFAULT_SEMIAN_OPTIONS.merge( + deterministic: false, + name: "#{host}_#{port}_#{Thread.current[:sub_resource_name]}", + ) + end + host = SemianConfig["toxiproxy_upstream_host"] + port = SemianConfig["http_toxiproxy_port"] + http = Net::HTTP.new(host, port) + Thread.current[:sub_resource_name] = "service_a" + + with_semian_configuration(options) do + with_server do + http.get("/200") + + Thread.current[:sub_resource_name] = "service_b" + http.get("/200") + + assert_includes(Semian.resources.keys, "nethttp_#{host}_#{port}_service_a") + assert_includes(Semian.resources.keys, "nethttp_#{host}_#{port}_service_b") + end + end + end + private def half_open_cicuit!(backwards_time_travel = 10) diff --git a/test/helpers/adapter_helper.rb b/test/helpers/adapter_helper.rb index 30c89dff..74ae3e08 100644 --- a/test/helpers/adapter_helper.rb +++ b/test/helpers/adapter_helper.rb @@ -19,6 +19,28 @@ def resource_exceptions end end + module NonDeterministicAdapterTest + include Semian::Adapter + + def semian_identifier + :non_deterministic_semian_adapter_test + end + + def raw_semian_options + { + bulkhead: false, + error_threshold: 1, + error_timeout: 1, + deterministic: false, + success_threshold: @current_success_threshold += 1, + } + end + + def resource_exceptions + [] + end + end + class AdapterTestClient include AdapterTest @@ -34,4 +56,16 @@ def ==(other) inspect == other.inspect end end + + class NonDeterministicAdapterTestClient + include NonDeterministicAdapterTest + + def initialize(**args) + @current_success_threshold = 1 + end + + def ==(other) + inspect == other.inspect + end + end end