From ec917ed6d0b93522fed93175333f5c826883ebc7 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Fri, 11 Jul 2025 12:55:02 -0700 Subject: [PATCH 01/10] Thread local approach --- .../lib/smithy-client/pageable_response.rb | 10 +++++++++- .../lib/smithy-client/plugins/protocol.rb | 13 ++++++++++++- .../smithy-client/plugins/request_compression.rb | 13 ++++++++++++- .../lib/smithy-client/plugins/retry_errors.rb | 14 ++++++++++++++ .../lib/smithy-client/waiters/poller.rb | 10 +++++++++- 5 files changed, 56 insertions(+), 4 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/pageable_response.rb b/gems/smithy-client/lib/smithy-client/pageable_response.rb index 99b5e8844..f3f16b4dc 100644 --- a/gems/smithy-client/lib/smithy-client/pageable_response.rb +++ b/gems/smithy-client/lib/smithy-client/pageable_response.rb @@ -92,7 +92,7 @@ def next_page(params = {}) raise LastPageError, self if last_page? params = next_page_params(params) - context.client.send(context.operation_name, params) + with_metric { context.client.send(context.operation_name, params) } end # Yields the current and each following response to the given block. @@ -133,6 +133,14 @@ def next_page_params(params) new_params = context.params.except(*prev_tokens) new_params.merge!(@paginator.next_tokens(data).merge(params)) end + + def with_metric(&block) + Thread.current[:aws_sdk_core_user_agent_metric] ||= [] + Thread.current[:aws_sdk_core_user_agent_metric] << 'C' + block.call + ensure + Thread.current[:aws_sdk_core_user_agent_metric].pop + end end end end diff --git a/gems/smithy-client/lib/smithy-client/plugins/protocol.rb b/gems/smithy-client/lib/smithy-client/plugins/protocol.rb index 4451434a7..7ca694277 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/protocol.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/protocol.rb @@ -16,7 +16,18 @@ class Protocol < Plugin class BuildHandler < Handler def call(context) context.config.protocol.build_request(context) - @handler.call(context) + with_metric(context.config.protocol) { @handler.call(context) } + end + + def with_metric(protocol, &block) + metric = protocol.is_a?(Smithy::Client::RPCv2CBOR::Protocol) ? 'M' : nil + return block.call unless metric + + Thread.current[:smithy_ruby_user_agent_metric] ||= [] + Thread.current[:smithy_ruby_user_agent_metric] << metric + block.call + ensure + Thread.current[:smithy_ruby_user_agent_metric].pop end end diff --git a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb index a9c5653d0..64bf8e3ac 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb @@ -77,7 +77,7 @@ def call(context) end end end - @handler.call(context) + with_metric(selected_encoding) { @handler.call(context) } end private @@ -149,6 +149,17 @@ def update_content_encoding(encoding, context) end end + def with_metric(encoding, &block) + metric = encoding == 'gzip' ? 'L' : nil + return block.call unless metric + + Thread.current[:smithy_ruby_user_agent_metric] ||= [] + Thread.current[:smithy_ruby_user_agent_metric] << metric + block.call + ensure + Thread.current[:smithy_ruby_user_agent_metric].pop + end + # @api private class GzipIO def initialize(body) diff --git a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb index 915af5865..27042c967 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb @@ -117,6 +117,20 @@ def reset_response(context, response) context.http_response.reset response.error = nil end + + def with_metric(retry_strategy, &block) + metric = case retry_strategy + when Retry::Standard then 'E' + when Retry::Adaptive then 'F' + else return block.call + end + + Thread.current[:smithy_ruby_user_agent_metric] ||= [] + Thread.current[:smithy_ruby_user_agent_metric] << metric + block.call + ensure + Thread.current[:smithy_ruby_user_agent_metric].pop if metric + end end handler(Handler, step: :retry) diff --git a/gems/smithy-client/lib/smithy-client/waiters/poller.rb b/gems/smithy-client/lib/smithy-client/waiters/poller.rb index 3fdd25649..89293da0c 100644 --- a/gems/smithy-client/lib/smithy-client/waiters/poller.rb +++ b/gems/smithy-client/lib/smithy-client/waiters/poller.rb @@ -15,7 +15,7 @@ def call(client, params) @input = params request = client.build_request(@operation_name, params) request.handlers.remove(Plugins::RaiseResponseErrors::Handler) - response = request.send_request + response = with_metric { request.send_request } status = evaluate_acceptors(response) [response, status.to_sym] end @@ -87,6 +87,14 @@ def anyStringEquals?(actual, expected) actual.any? { |value| value == expected } end # rubocop:enable Naming/MethodName + + def with_metric (&block) + Thread.current[:smithy_ruby_user_agent_metric] ||= [] + Thread.current[:smithy_ruby_user_agent_metric] << 'B' + block.call + ensure + Thread.current[:smithy_ruby_user_agent_metric].pop + end end end end From ba083b431a18d5488361bf6b8b30518633998418 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 14 Jul 2025 11:13:27 -0700 Subject: [PATCH 02/10] Context approach --- .../lib/smithy-client/pageable_response.rb | 6 +++++- .../lib/smithy-client/plugins/request_compression.rb | 10 +++++++++- .../lib/smithy-client/plugins/retry_errors.rb | 10 ++++++++++ .../lib/smithy-client/rpc_v2_cbor/handler.rb | 3 +++ gems/smithy-client/lib/smithy-client/waiters/poller.rb | 9 ++++++++- 5 files changed, 35 insertions(+), 3 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/pageable_response.rb b/gems/smithy-client/lib/smithy-client/pageable_response.rb index f3f16b4dc..7b797f339 100644 --- a/gems/smithy-client/lib/smithy-client/pageable_response.rb +++ b/gems/smithy-client/lib/smithy-client/pageable_response.rb @@ -92,7 +92,11 @@ def next_page(params = {}) raise LastPageError, self if last_page? params = next_page_params(params) - with_metric { context.client.send(context.operation_name, params) } + context[:user_agent_feature_ids] ||= [] + context[:user_agent_feature_ids] << 'PAGINATOR' + resp = context.client.send(context.operation_name, params) + context[:user_agent_feature_ids].delete('PAGINATOR') + resp end # Yields the current and each following response to the given block. diff --git a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb index 64bf8e3ac..1d12b81e6 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb @@ -77,7 +77,15 @@ def call(context) end end end - with_metric(selected_encoding) { @handler.call(context) } + if selected_encoding == 'gzip' + context[:user_agent_feature_ids] ||= [] + context[:user_agent_feature_ids] << 'GZIP_REQUEST_COMPRESSION' + response = @handler.call(context) + context[:user_agent_feature_ids].delete('GZIP_REQUEST_COMPRESSION') + response + else + @handler.call(context) + end end private diff --git a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb index 27042c967..96525bc98 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb @@ -84,7 +84,17 @@ def call(context) private def handle(context, retry_strategy, token) + feature_id = case retry_strategy + when Retry::Standard then 'RETRY_MODE_STANDARD' + when Retry::Adaptive then 'RETRY_MODE_ADAPTIVE' + end + if feature_id + context[:user_agent_feature_ids] ||= [] + context[:user_agent_feature_ids] << feature_id + end response = @handler.call(context) + context[:user_agent_feature_ids]&.delete(feature_id) if feature_id + if (error = response.error) return response unless retryable?(context.http_request) diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb index 45abb038a..3036c23d9 100644 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb +++ b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb @@ -6,8 +6,11 @@ module RpcV2Cbor # @api private class Handler < Client::Handler def call(context) + context[:user_agent_feature_ids] ||= [] + context[:user_agent_feature_ids] << 'PROTOCOL_RPC_V2_CBOR' build_request(context) response = @handler.call(context) + context[:user_agent_feature_ids].delete('PROTOCOL_RPC_V2_CBOR') response.on_done(200..299) { |resp| resp.data = parse_body(context) } response end diff --git a/gems/smithy-client/lib/smithy-client/waiters/poller.rb b/gems/smithy-client/lib/smithy-client/waiters/poller.rb index 89293da0c..bc8fc3b87 100644 --- a/gems/smithy-client/lib/smithy-client/waiters/poller.rb +++ b/gems/smithy-client/lib/smithy-client/waiters/poller.rb @@ -15,7 +15,14 @@ def call(client, params) @input = params request = client.build_request(@operation_name, params) request.handlers.remove(Plugins::RaiseResponseErrors::Handler) - response = with_metric { request.send_request } + request.handle do |context| + context[:user_agent_feature_ids] ||= [] + context[:user_agent_feature_ids] << 'WAITER' + @handler.call(context) + ensure + context.config[:user_agent_feature_ids].delete('WAITER') + end + response = request.send_request status = evaluate_acceptors(response) [response, status.to_sym] end From 674da7cfd982cc75802bf5e21feb1f4ef607b9f7 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 15 Jul 2025 10:52:49 -0700 Subject: [PATCH 03/10] Features module --- gems/smithy-client/lib/smithy-client.rb | 1 + .../lib/smithy-client/features.rb | 20 +++++++++++++ .../lib/smithy-client/pageable_response.rb | 14 +--------- .../plugins/request_compression.rb | 23 ++++----------- .../lib/smithy-client/plugins/retry_errors.rb | 28 ++++--------------- .../lib/smithy-client/rpc_v2_cbor/handler.rb | 5 +--- .../lib/smithy-client/waiters/poller.rb | 17 +---------- 7 files changed, 36 insertions(+), 72 deletions(-) create mode 100644 gems/smithy-client/lib/smithy-client/features.rb diff --git a/gems/smithy-client/lib/smithy-client.rb b/gems/smithy-client/lib/smithy-client.rb index c85cea37b..c3b8be960 100644 --- a/gems/smithy-client/lib/smithy-client.rb +++ b/gems/smithy-client/lib/smithy-client.rb @@ -14,6 +14,7 @@ require_relative 'smithy-client/default_params' require_relative 'smithy-client/dynamic_errors' require_relative 'smithy-client/endpoint_rules' +require_relative 'smithy-client/features' require_relative 'smithy-client/handler' require_relative 'smithy-client/handler_builder' require_relative 'smithy-client/handler_context' diff --git a/gems/smithy-client/lib/smithy-client/features.rb b/gems/smithy-client/lib/smithy-client/features.rb new file mode 100644 index 000000000..6741baff3 --- /dev/null +++ b/gems/smithy-client/lib/smithy-client/features.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Smithy + module Client + # @api private + module Features + def self.clear + Thread.current[:smithy_ruby_user_agent_metric] = [] + end + + def self.with_metric(*metrics, &block) + Thread.current[:smithy_ruby_user_agent_metric] ||= [] + Thread.current[:smithy_ruby_user_agent_metric].concat(metrics) + block.call + ensure + Thread.current[:smithy_ruby_user_agent_metric].pop(metrics.size) + end + end + end +end diff --git a/gems/smithy-client/lib/smithy-client/pageable_response.rb b/gems/smithy-client/lib/smithy-client/pageable_response.rb index 7b797f339..6e97d87b7 100644 --- a/gems/smithy-client/lib/smithy-client/pageable_response.rb +++ b/gems/smithy-client/lib/smithy-client/pageable_response.rb @@ -92,11 +92,7 @@ def next_page(params = {}) raise LastPageError, self if last_page? params = next_page_params(params) - context[:user_agent_feature_ids] ||= [] - context[:user_agent_feature_ids] << 'PAGINATOR' - resp = context.client.send(context.operation_name, params) - context[:user_agent_feature_ids].delete('PAGINATOR') - resp + Features.with_metric('PAGINATOR') { context.client.send(context.operation_name, params) } end # Yields the current and each following response to the given block. @@ -137,14 +133,6 @@ def next_page_params(params) new_params = context.params.except(*prev_tokens) new_params.merge!(@paginator.next_tokens(data).merge(params)) end - - def with_metric(&block) - Thread.current[:aws_sdk_core_user_agent_metric] ||= [] - Thread.current[:aws_sdk_core_user_agent_metric] << 'C' - block.call - ensure - Thread.current[:aws_sdk_core_user_agent_metric].pop - end end end end diff --git a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb index 1d12b81e6..72bd8c231 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb @@ -77,15 +77,7 @@ def call(context) end end end - if selected_encoding == 'gzip' - context[:user_agent_feature_ids] ||= [] - context[:user_agent_feature_ids] << 'GZIP_REQUEST_COMPRESSION' - response = @handler.call(context) - context[:user_agent_feature_ids].delete('GZIP_REQUEST_COMPRESSION') - response - else - @handler.call(context) - end + with_metric(selected_encoding) { @handler.call(context) } end private @@ -158,14 +150,11 @@ def update_content_encoding(encoding, context) end def with_metric(encoding, &block) - metric = encoding == 'gzip' ? 'L' : nil - return block.call unless metric - - Thread.current[:smithy_ruby_user_agent_metric] ||= [] - Thread.current[:smithy_ruby_user_agent_metric] << metric - block.call - ensure - Thread.current[:smithy_ruby_user_agent_metric].pop + if encoding == 'gzip' + Features.with_metric('GZIP_REQUEST_COMPRESSION', &block) + else + block.call + end end # @api private diff --git a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb index 96525bc98..4e47f6168 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb @@ -84,17 +84,7 @@ def call(context) private def handle(context, retry_strategy, token) - feature_id = case retry_strategy - when Retry::Standard then 'RETRY_MODE_STANDARD' - when Retry::Adaptive then 'RETRY_MODE_ADAPTIVE' - end - if feature_id - context[:user_agent_feature_ids] ||= [] - context[:user_agent_feature_ids] << feature_id - end - response = @handler.call(context) - context[:user_agent_feature_ids]&.delete(feature_id) if feature_id - + response = with_metric(retry_strategy) { @handler.call(context) } if (error = response.error) return response unless retryable?(context.http_request) @@ -129,17 +119,11 @@ def reset_response(context, response) end def with_metric(retry_strategy, &block) - metric = case retry_strategy - when Retry::Standard then 'E' - when Retry::Adaptive then 'F' - else return block.call - end - - Thread.current[:smithy_ruby_user_agent_metric] ||= [] - Thread.current[:smithy_ruby_user_agent_metric] << metric - block.call - ensure - Thread.current[:smithy_ruby_user_agent_metric].pop if metric + case retry_strategy + when Retry::Standard then Features.with_metric('RETRY_MODE_STANDARD') { block.call } + when Retry::Adaptive then Features.with_metric('RETRY_MODE_ADAPTIVE') { block.call } + else block.call + end end end diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb index 3036c23d9..04eafca3e 100644 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb +++ b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb @@ -6,11 +6,8 @@ module RpcV2Cbor # @api private class Handler < Client::Handler def call(context) - context[:user_agent_feature_ids] ||= [] - context[:user_agent_feature_ids] << 'PROTOCOL_RPC_V2_CBOR' build_request(context) - response = @handler.call(context) - context[:user_agent_feature_ids].delete('PROTOCOL_RPC_V2_CBOR') + response = Features.with_metric('PROTOCOL_RPC_V2_CBOR') { @handler.call(context) } response.on_done(200..299) { |resp| resp.data = parse_body(context) } response end diff --git a/gems/smithy-client/lib/smithy-client/waiters/poller.rb b/gems/smithy-client/lib/smithy-client/waiters/poller.rb index bc8fc3b87..bc0a6a549 100644 --- a/gems/smithy-client/lib/smithy-client/waiters/poller.rb +++ b/gems/smithy-client/lib/smithy-client/waiters/poller.rb @@ -15,14 +15,7 @@ def call(client, params) @input = params request = client.build_request(@operation_name, params) request.handlers.remove(Plugins::RaiseResponseErrors::Handler) - request.handle do |context| - context[:user_agent_feature_ids] ||= [] - context[:user_agent_feature_ids] << 'WAITER' - @handler.call(context) - ensure - context.config[:user_agent_feature_ids].delete('WAITER') - end - response = request.send_request + response = Features.with_metric('WAITER') { request.send_request } status = evaluate_acceptors(response) [response, status.to_sym] end @@ -94,14 +87,6 @@ def anyStringEquals?(actual, expected) actual.any? { |value| value == expected } end # rubocop:enable Naming/MethodName - - def with_metric (&block) - Thread.current[:smithy_ruby_user_agent_metric] ||= [] - Thread.current[:smithy_ruby_user_agent_metric] << 'B' - block.call - ensure - Thread.current[:smithy_ruby_user_agent_metric].pop - end end end end From 413935616dd07a68a51b6538c0c569b318136f28 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 15 Jul 2025 11:32:00 -0700 Subject: [PATCH 04/10] Clear thread in build request --- gems/smithy-client/lib/smithy-client/base.rb | 1 + gems/smithy-client/lib/smithy-client/features.rb | 1 - gems/smithy/lib/smithy/templates/client/client.erb | 1 + projections/shapes/lib/shapes/client.rb | 1 + projections/weather/lib/weather/client.rb | 1 + 5 files changed, 4 insertions(+), 1 deletion(-) diff --git a/gems/smithy-client/lib/smithy-client/base.rb b/gems/smithy-client/lib/smithy-client/base.rb index 460011c48..0c2ae30ef 100644 --- a/gems/smithy-client/lib/smithy-client/base.rb +++ b/gems/smithy-client/lib/smithy-client/base.rb @@ -22,6 +22,7 @@ def initialize(plugins, options) # @param [Symbol] operation_name # @return [Request] def build_request(operation_name, params = {}) + Features.clear Request.new( handlers: @handlers.for(operation_name), context: context_for(operation_name, params) diff --git a/gems/smithy-client/lib/smithy-client/features.rb b/gems/smithy-client/lib/smithy-client/features.rb index 6741baff3..f5b6635e0 100644 --- a/gems/smithy-client/lib/smithy-client/features.rb +++ b/gems/smithy-client/lib/smithy-client/features.rb @@ -9,7 +9,6 @@ def self.clear end def self.with_metric(*metrics, &block) - Thread.current[:smithy_ruby_user_agent_metric] ||= [] Thread.current[:smithy_ruby_user_agent_metric].concat(metrics) block.call ensure diff --git a/gems/smithy/lib/smithy/templates/client/client.erb b/gems/smithy/lib/smithy/templates/client/client.erb index edfde738e..b338af5c6 100644 --- a/gems/smithy/lib/smithy/templates/client/client.erb +++ b/gems/smithy/lib/smithy/templates/client/client.erb @@ -103,6 +103,7 @@ module <%= module_name %> # @api private def build_request(operation_name, params) + Smithy::Client::Features.clear handlers = @handlers.for(operation_name) context = Smithy::Client::HandlerContext.new( operation_name: operation_name, diff --git a/projections/shapes/lib/shapes/client.rb b/projections/shapes/lib/shapes/client.rb index 6d4a334c0..3f790e0cd 100644 --- a/projections/shapes/lib/shapes/client.rb +++ b/projections/shapes/lib/shapes/client.rb @@ -330,6 +330,7 @@ def wait_until(waiter_name, params = {}, options = {}) # @api private def build_request(operation_name, params) + Smithy::Client::Features.clear handlers = @handlers.for(operation_name) context = Smithy::Client::HandlerContext.new( operation_name: operation_name, diff --git a/projections/weather/lib/weather/client.rb b/projections/weather/lib/weather/client.rb index 5f6531af5..886aad51e 100644 --- a/projections/weather/lib/weather/client.rb +++ b/projections/weather/lib/weather/client.rb @@ -318,6 +318,7 @@ def wait_until(waiter_name, params = {}, options = {}) # @api private def build_request(operation_name, params) + Smithy::Client::Features.clear handlers = @handlers.for(operation_name) context = Smithy::Client::HandlerContext.new( operation_name: operation_name, From e79a5599b4e59960092eccfb5d6be6d52ed781c6 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 15 Jul 2025 11:50:33 -0700 Subject: [PATCH 05/10] Move clear --- gems/smithy-client/lib/smithy-client/base.rb | 1 - gems/smithy-client/lib/smithy-client/request.rb | 1 + gems/smithy/lib/smithy/templates/client/client.erb | 1 - projections/shapes/lib/shapes/client.rb | 1 - projections/weather/lib/weather/client.rb | 1 - 5 files changed, 1 insertion(+), 4 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/base.rb b/gems/smithy-client/lib/smithy-client/base.rb index 0c2ae30ef..460011c48 100644 --- a/gems/smithy-client/lib/smithy-client/base.rb +++ b/gems/smithy-client/lib/smithy-client/base.rb @@ -22,7 +22,6 @@ def initialize(plugins, options) # @param [Symbol] operation_name # @return [Request] def build_request(operation_name, params = {}) - Features.clear Request.new( handlers: @handlers.for(operation_name), context: context_for(operation_name, params) diff --git a/gems/smithy-client/lib/smithy-client/request.rb b/gems/smithy-client/lib/smithy-client/request.rb index 3c7d12045..5d96c0bff 100644 --- a/gems/smithy-client/lib/smithy-client/request.rb +++ b/gems/smithy-client/lib/smithy-client/request.rb @@ -9,6 +9,7 @@ class Request # @option options [HandlerList] :handlers (nil) # @option options [HandlerContext] :context (nil) def initialize(options = {}) + Features.clear @handlers = options[:handlers] || HandlerList.new @context = options[:context] || HandlerContext.new end diff --git a/gems/smithy/lib/smithy/templates/client/client.erb b/gems/smithy/lib/smithy/templates/client/client.erb index b338af5c6..edfde738e 100644 --- a/gems/smithy/lib/smithy/templates/client/client.erb +++ b/gems/smithy/lib/smithy/templates/client/client.erb @@ -103,7 +103,6 @@ module <%= module_name %> # @api private def build_request(operation_name, params) - Smithy::Client::Features.clear handlers = @handlers.for(operation_name) context = Smithy::Client::HandlerContext.new( operation_name: operation_name, diff --git a/projections/shapes/lib/shapes/client.rb b/projections/shapes/lib/shapes/client.rb index 3f790e0cd..6d4a334c0 100644 --- a/projections/shapes/lib/shapes/client.rb +++ b/projections/shapes/lib/shapes/client.rb @@ -330,7 +330,6 @@ def wait_until(waiter_name, params = {}, options = {}) # @api private def build_request(operation_name, params) - Smithy::Client::Features.clear handlers = @handlers.for(operation_name) context = Smithy::Client::HandlerContext.new( operation_name: operation_name, diff --git a/projections/weather/lib/weather/client.rb b/projections/weather/lib/weather/client.rb index 886aad51e..5f6531af5 100644 --- a/projections/weather/lib/weather/client.rb +++ b/projections/weather/lib/weather/client.rb @@ -318,7 +318,6 @@ def wait_until(waiter_name, params = {}, options = {}) # @api private def build_request(operation_name, params) - Smithy::Client::Features.clear handlers = @handlers.for(operation_name) context = Smithy::Client::HandlerContext.new( operation_name: operation_name, From d39dea8ce92b7b95cedb21af16065cde4c328a84 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 15 Jul 2025 15:08:47 -0700 Subject: [PATCH 06/10] Clean up and add tests --- .../lib/smithy-client/features.rb | 12 ++- .../lib/smithy-client/pageable_response.rb | 2 +- .../plugins/request_compression.rb | 2 +- .../lib/smithy-client/plugins/retry_errors.rb | 4 +- .../lib/smithy-client/rpc_v2_cbor/handler.rb | 2 +- .../lib/smithy-client/waiters/poller.rb | 2 +- .../spec/smithy-client/features_spec.rb | 81 +++++++++++++++++++ 7 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 gems/smithy-client/spec/smithy-client/features_spec.rb diff --git a/gems/smithy-client/lib/smithy-client/features.rb b/gems/smithy-client/lib/smithy-client/features.rb index f5b6635e0..7cccd4fd5 100644 --- a/gems/smithy-client/lib/smithy-client/features.rb +++ b/gems/smithy-client/lib/smithy-client/features.rb @@ -5,14 +5,18 @@ module Client # @api private module Features def self.clear - Thread.current[:smithy_ruby_user_agent_metric] = [] + Thread.current[:smithy_ruby_features] = [] end - def self.with_metric(*metrics, &block) - Thread.current[:smithy_ruby_user_agent_metric].concat(metrics) + def self.track(*metrics, &block) + Thread.current[:smithy_ruby_features].concat(metrics) block.call ensure - Thread.current[:smithy_ruby_user_agent_metric].pop(metrics.size) + Thread.current[:smithy_ruby_features].pop(metrics.size) + end + + def self.list + Thread.current[:smithy_ruby_features] || [] end end end diff --git a/gems/smithy-client/lib/smithy-client/pageable_response.rb b/gems/smithy-client/lib/smithy-client/pageable_response.rb index 6e97d87b7..2f9fa1046 100644 --- a/gems/smithy-client/lib/smithy-client/pageable_response.rb +++ b/gems/smithy-client/lib/smithy-client/pageable_response.rb @@ -92,7 +92,7 @@ def next_page(params = {}) raise LastPageError, self if last_page? params = next_page_params(params) - Features.with_metric('PAGINATOR') { context.client.send(context.operation_name, params) } + Features.track('PAGINATOR') { context.client.send(context.operation_name, params) } end # Yields the current and each following response to the given block. diff --git a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb index 72bd8c231..eb936b22b 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb @@ -151,7 +151,7 @@ def update_content_encoding(encoding, context) def with_metric(encoding, &block) if encoding == 'gzip' - Features.with_metric('GZIP_REQUEST_COMPRESSION', &block) + Features.track('GZIP_REQUEST_COMPRESSION', &block) else block.call end diff --git a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb index 4e47f6168..481fa2dba 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb @@ -120,8 +120,8 @@ def reset_response(context, response) def with_metric(retry_strategy, &block) case retry_strategy - when Retry::Standard then Features.with_metric('RETRY_MODE_STANDARD') { block.call } - when Retry::Adaptive then Features.with_metric('RETRY_MODE_ADAPTIVE') { block.call } + when Retry::Standard then Features.track('RETRY_MODE_STANDARD') { block.call } + when Retry::Adaptive then Features.track('RETRY_MODE_ADAPTIVE') { block.call } else block.call end end diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb index 4822b26d5..98fd362ff 100644 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb +++ b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb @@ -7,7 +7,7 @@ module RpcV2Cbor class Handler < Client::Handler def call(context) build_request(context) - response = Features.with_metric('PROTOCOL_RPC_V2_CBOR') { @handler.call(context) } + response = Features.track('PROTOCOL_RPC_V2_CBOR') { @handler.call(context) } response.on_done(200..299) { |resp| resp.data = parse_body(context) } response end diff --git a/gems/smithy-client/lib/smithy-client/waiters/poller.rb b/gems/smithy-client/lib/smithy-client/waiters/poller.rb index bc0a6a549..74e2b7583 100644 --- a/gems/smithy-client/lib/smithy-client/waiters/poller.rb +++ b/gems/smithy-client/lib/smithy-client/waiters/poller.rb @@ -15,7 +15,7 @@ def call(client, params) @input = params request = client.build_request(@operation_name, params) request.handlers.remove(Plugins::RaiseResponseErrors::Handler) - response = Features.with_metric('WAITER') { request.send_request } + response = Features.track('WAITER') { request.send_request } status = evaluate_acceptors(response) [response, status.to_sym] end diff --git a/gems/smithy-client/spec/smithy-client/features_spec.rb b/gems/smithy-client/spec/smithy-client/features_spec.rb new file mode 100644 index 000000000..8ed43a53e --- /dev/null +++ b/gems/smithy-client/spec/smithy-client/features_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require_relative '../spec_helper' + +module Smithy + module Client + describe Features do + describe '#clear' do + it 'clears features' do + Thread.current[:smithy_ruby_features] = ['test'] + Features.clear + expect(Features.list).to be_empty + end + end + + describe '#track' do + before(:each) { Features.clear } + + it 'tracks and removes a feature' do + proc = proc { expect(Features.list).to eq(%w[A]) } + Features.track('A') { proc.call } + expect(Features.list).to be_empty + end + + it 'tracks and removes multiple features' do + features = %w[A B C] + proc = proc { expect(Features.list).to eq(features) } + Features.track(*features) { proc.call } + expect(Features.list).to be_empty + end + + it 'tracks and removes features in stack order' do + proc1 = proc do + Features.track('C') do + expect(Features.list).to eq(%w[A B C]) + end + expect(Features.list).to eq(%w[A B]) + end + proc2 = proc do + Features.track('B') do + expect(Features.list).to eq(%w[A B]) + proc1.call + end + expect(Features.list).to eq(%w[A]) + end + Features.track('A') { proc2.call } + expect(Features.list).to be_empty + end + + it 'ensures that features are removed' do + proc = proc do + expect(Features.list).to eq(%w[A]) + raise StandardError + end + begin + Features.track('A') { proc.call } + rescue StandardError + # ignore + end + expect(Features.list).to be_empty + end + + it 'tracks features in multiple threads' do + Features.track('A') do + expect(Features.list).to eq(%w[A]) + Thread.new do + expect(Features.list).to be_empty + Features.clear + Features.track('B') do + expect(Features.list).to eq(%w[B]) + end + expect(Features.list).to be_empty + end.join + expect(Features.list).to eq(%w[A]) + end + expect(Features.list).to be_empty + end + end + end + end +end From 40e44a4373b97dcb0ab194b93284f172e8dcf547 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 15 Jul 2025 15:12:56 -0700 Subject: [PATCH 07/10] Add list test --- gems/smithy-client/spec/smithy-client/features_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/gems/smithy-client/spec/smithy-client/features_spec.rb b/gems/smithy-client/spec/smithy-client/features_spec.rb index 8ed43a53e..faff9dac8 100644 --- a/gems/smithy-client/spec/smithy-client/features_spec.rb +++ b/gems/smithy-client/spec/smithy-client/features_spec.rb @@ -7,7 +7,7 @@ module Client describe Features do describe '#clear' do it 'clears features' do - Thread.current[:smithy_ruby_features] = ['test'] + Thread.current[:smithy_ruby_features] = %w[A] Features.clear expect(Features.list).to be_empty end @@ -76,6 +76,13 @@ module Client expect(Features.list).to be_empty end end + + describe '#list' do + it 'lists all features' do + Thread.current[:smithy_ruby_features] = %w[A] + expect(Features.list).to eq(%w[A]) + end + end end end end From fa9011ce233149dbcc6d1724276a661f52355775 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 16 Jul 2025 09:23:18 -0700 Subject: [PATCH 08/10] Remove clear and refactor tests --- .../lib/smithy-client/features.rb | 7 +- .../lib/smithy-client/plugins/retry_errors.rb | 4 +- .../lib/smithy-client/request.rb | 1 - .../spec/smithy-client/features_spec.rb | 100 +++++++----------- 4 files changed, 41 insertions(+), 71 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/features.rb b/gems/smithy-client/lib/smithy-client/features.rb index 7cccd4fd5..e5003cae8 100644 --- a/gems/smithy-client/lib/smithy-client/features.rb +++ b/gems/smithy-client/lib/smithy-client/features.rb @@ -4,18 +4,15 @@ module Smithy module Client # @api private module Features - def self.clear - Thread.current[:smithy_ruby_features] = [] - end - def self.track(*metrics, &block) + Thread.current[:smithy_ruby_features] ||= [] Thread.current[:smithy_ruby_features].concat(metrics) block.call ensure Thread.current[:smithy_ruby_features].pop(metrics.size) end - def self.list + def self.tracked Thread.current[:smithy_ruby_features] || [] end end diff --git a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb index 481fa2dba..5cad4d8bc 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb @@ -120,8 +120,8 @@ def reset_response(context, response) def with_metric(retry_strategy, &block) case retry_strategy - when Retry::Standard then Features.track('RETRY_MODE_STANDARD') { block.call } - when Retry::Adaptive then Features.track('RETRY_MODE_ADAPTIVE') { block.call } + when Retry::Standard then Features.track('RETRY_MODE_STANDARD', &block) + when Retry::Adaptive then Features.track('RETRY_MODE_ADAPTIVE', &block) else block.call end end diff --git a/gems/smithy-client/lib/smithy-client/request.rb b/gems/smithy-client/lib/smithy-client/request.rb index 5d96c0bff..3c7d12045 100644 --- a/gems/smithy-client/lib/smithy-client/request.rb +++ b/gems/smithy-client/lib/smithy-client/request.rb @@ -9,7 +9,6 @@ class Request # @option options [HandlerList] :handlers (nil) # @option options [HandlerContext] :context (nil) def initialize(options = {}) - Features.clear @handlers = options[:handlers] || HandlerList.new @context = options[:context] || HandlerContext.new end diff --git a/gems/smithy-client/spec/smithy-client/features_spec.rb b/gems/smithy-client/spec/smithy-client/features_spec.rb index faff9dac8..6f89bbaed 100644 --- a/gems/smithy-client/spec/smithy-client/features_spec.rb +++ b/gems/smithy-client/spec/smithy-client/features_spec.rb @@ -5,83 +5,57 @@ module Smithy module Client describe Features do - describe '#clear' do - it 'clears features' do - Thread.current[:smithy_ruby_features] = %w[A] - Features.clear - expect(Features.list).to be_empty - end + it 'tracks and removes a feature' do + Features.track('A') { expect(Features.tracked).to eq(%w[A]) } + expect(Features.tracked).to be_empty end - describe '#track' do - before(:each) { Features.clear } - - it 'tracks and removes a feature' do - proc = proc { expect(Features.list).to eq(%w[A]) } - Features.track('A') { proc.call } - expect(Features.list).to be_empty - end - - it 'tracks and removes multiple features' do - features = %w[A B C] - proc = proc { expect(Features.list).to eq(features) } - Features.track(*features) { proc.call } - expect(Features.list).to be_empty - end + it 'tracks and removes multiple features' do + features = %w[A B C] + Features.track(*features) { expect(Features.tracked).to eq(features) } + expect(Features.tracked).to be_empty + end - it 'tracks and removes features in stack order' do - proc1 = proc do + it 'tracks and removes features in stack order' do + Features.track('A') do + expect(Features.tracked).to eq(%w[A]) + Features.track('B') do + expect(Features.tracked).to eq(%w[A B]) Features.track('C') do - expect(Features.list).to eq(%w[A B C]) - end - expect(Features.list).to eq(%w[A B]) - end - proc2 = proc do - Features.track('B') do - expect(Features.list).to eq(%w[A B]) - proc1.call + expect(Features.tracked).to eq(%w[A B C]) end - expect(Features.list).to eq(%w[A]) + expect(Features.tracked).to eq(%w[A B]) end - Features.track('A') { proc2.call } - expect(Features.list).to be_empty - end - - it 'ensures that features are removed' do - proc = proc do - expect(Features.list).to eq(%w[A]) - raise StandardError - end - begin - Features.track('A') { proc.call } - rescue StandardError - # ignore - end - expect(Features.list).to be_empty + expect(Features.tracked).to eq(%w[A]) end + expect(Features.tracked).to be_empty + end - it 'tracks features in multiple threads' do + it 'ensures that features are removed' do + begin Features.track('A') do - expect(Features.list).to eq(%w[A]) - Thread.new do - expect(Features.list).to be_empty - Features.clear - Features.track('B') do - expect(Features.list).to eq(%w[B]) - end - expect(Features.list).to be_empty - end.join - expect(Features.list).to eq(%w[A]) + expect(Features.tracked).to eq(%w[A]) + raise StandardError end - expect(Features.list).to be_empty + rescue StandardError + # ignore end + expect(Features.tracked).to be_empty end - describe '#list' do - it 'lists all features' do - Thread.current[:smithy_ruby_features] = %w[A] - expect(Features.list).to eq(%w[A]) + it 'tracks features in multiple threads' do + Features.track('A') do + expect(Features.tracked).to eq(%w[A]) + Thread.new do + expect(Features.tracked).to be_empty + Features.track('B') do + expect(Features.tracked).to eq(%w[B]) + end + expect(Features.tracked).to be_empty + end.join + expect(Features.tracked).to eq(%w[A]) end + expect(Features.tracked).to be_empty end end end From a6d2a45d559f233e78c7030dbc4af536780b92e6 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 16 Jul 2025 09:30:29 -0700 Subject: [PATCH 09/10] Rename metric to feature --- .../lib/smithy-client/features.rb | 20 ++++++++++--------- .../plugins/request_compression.rb | 4 ++-- .../lib/smithy-client/plugins/retry_errors.rb | 4 ++-- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/features.rb b/gems/smithy-client/lib/smithy-client/features.rb index e5003cae8..f60321fd6 100644 --- a/gems/smithy-client/lib/smithy-client/features.rb +++ b/gems/smithy-client/lib/smithy-client/features.rb @@ -4,16 +4,18 @@ module Smithy module Client # @api private module Features - def self.track(*metrics, &block) - Thread.current[:smithy_ruby_features] ||= [] - Thread.current[:smithy_ruby_features].concat(metrics) - block.call - ensure - Thread.current[:smithy_ruby_features].pop(metrics.size) - end + class << self + def track(*features, &block) + Thread.current[:smithy_ruby_features] ||= [] + Thread.current[:smithy_ruby_features].concat(features) + block.call + ensure + Thread.current[:smithy_ruby_features].pop(features.size) + end - def self.tracked - Thread.current[:smithy_ruby_features] || [] + def tracked + Thread.current[:smithy_ruby_features] || [] + end end end end diff --git a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb index eb936b22b..e765369e3 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb @@ -77,7 +77,7 @@ def call(context) end end end - with_metric(selected_encoding) { @handler.call(context) } + track_feature(selected_encoding) { @handler.call(context) } end private @@ -149,7 +149,7 @@ def update_content_encoding(encoding, context) end end - def with_metric(encoding, &block) + def track_feature(encoding, &block) if encoding == 'gzip' Features.track('GZIP_REQUEST_COMPRESSION', &block) else diff --git a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb index 5cad4d8bc..3c6561452 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/retry_errors.rb @@ -84,7 +84,7 @@ def call(context) private def handle(context, retry_strategy, token) - response = with_metric(retry_strategy) { @handler.call(context) } + response = track_feature(retry_strategy) { @handler.call(context) } if (error = response.error) return response unless retryable?(context.http_request) @@ -118,7 +118,7 @@ def reset_response(context, response) response.error = nil end - def with_metric(retry_strategy, &block) + def track_feature(retry_strategy, &block) case retry_strategy when Retry::Standard then Features.track('RETRY_MODE_STANDARD', &block) when Retry::Adaptive then Features.track('RETRY_MODE_ADAPTIVE', &block) From 5141d2d60b26ce052be62d3bcf58cde9df2110dc Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 16 Jul 2025 11:18:30 -0700 Subject: [PATCH 10/10] PR comments --- .../lib/smithy-client/plugins/request_compression.rb | 3 ++- gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb index e765369e3..953f3d57f 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/request_compression.rb @@ -150,7 +150,8 @@ def update_content_encoding(encoding, context) end def track_feature(encoding, &block) - if encoding == 'gzip' + case encoding + when 'gzip' Features.track('GZIP_REQUEST_COMPRESSION', &block) else block.call diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb index 98fd362ff..b343a08a6 100644 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb +++ b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb @@ -7,7 +7,7 @@ module RpcV2Cbor class Handler < Client::Handler def call(context) build_request(context) - response = Features.track('PROTOCOL_RPC_V2_CBOR') { @handler.call(context) } + response = @handler.call(context) response.on_done(200..299) { |resp| resp.data = parse_body(context) } response end