From 1869c677f3419d079ebc7b4a2b493d96e84415ce Mon Sep 17 00:00:00 2001 From: Matt Muller <53055821+mullermp@users.noreply.github.com> Date: Wed, 11 Sep 2024 12:57:48 -0400 Subject: [PATCH] Additional metrics collection (#3099) --- .../views/endpoints_module.rb | 6 +----- .../spec/endpoint_provider_spec_class.rb | 13 ++++++------ .../templates/endpoints_module.mustache | 5 ----- .../templates/endpoints_plugin.mustache | 11 +++++++++- .../endpoint_provider_spec_class.mustache | 2 +- build_tools/services.rb | 4 ++-- gems/aws-sdk-core/CHANGELOG.md | 2 ++ .../aws-sdk-core/plugins/regional_endpoint.rb | 1 + .../lib/aws-sdk-core/plugins/user_agent.rb | 20 +++++++++++-------- .../lib/aws-sdk-core/rpc_v2/handler.rb | 6 +++++- .../aws/plugins/request_compression_spec.rb | 14 ------------- .../aws/plugins/retry_errors_legacy_spec.rb | 9 --------- .../spec/aws/plugins/retry_errors_spec.rb | 18 ----------------- .../spec/plugins/express_session_auth_spec.rb | 7 ------- 14 files changed, 41 insertions(+), 77 deletions(-) diff --git a/build_tools/aws-sdk-code-generator/lib/aws-sdk-code-generator/views/endpoints_module.rb b/build_tools/aws-sdk-code-generator/lib/aws-sdk-code-generator/views/endpoints_module.rb index 1dfab1d7550..cc075d0af08 100644 --- a/build_tools/aws-sdk-code-generator/lib/aws-sdk-code-generator/views/endpoints_module.rb +++ b/build_tools/aws-sdk-code-generator/lib/aws-sdk-code-generator/views/endpoints_module.rb @@ -42,10 +42,6 @@ def initialize(options) # @return [Array] attr_reader :parameters - - def has_endpoint_built_in? - parameters.any? { |p| p.param_data['builtIn'] == 'SDK::Endpoint' } - end end class EndpointParameter @@ -155,7 +151,7 @@ def built_in_client_context_param_value(param_data) when 'AWS::S3::DisableMultiRegionAccessPoints' 'context.config.s3_disable_multiregion_access_points' when 'SDK::Endpoint' - 'endpoint' + 'context.config.regional_endpoint ? nil : context.config.endpoint.to_s' end end diff --git a/build_tools/aws-sdk-code-generator/lib/aws-sdk-code-generator/views/spec/endpoint_provider_spec_class.rb b/build_tools/aws-sdk-code-generator/lib/aws-sdk-code-generator/views/spec/endpoint_provider_spec_class.rb index 4b5dee46a59..69bbb10a877 100644 --- a/build_tools/aws-sdk-code-generator/lib/aws-sdk-code-generator/views/spec/endpoint_provider_spec_class.rb +++ b/build_tools/aws-sdk-code-generator/lib/aws-sdk-code-generator/views/spec/endpoint_provider_spec_class.rb @@ -51,9 +51,9 @@ def initialize(options) operation_name: Underscore.underscore( operation_inputs_test['operationName'] ), - operation_params: operation_inputs_test['operationParams'] || [], - built_in_params: operation_inputs_test['builtInParams'] || [], - client_params: operation_inputs_test['clientParams'] || [] + operation_params: operation_inputs_test['operationParams'] || {}, + built_in_params: operation_inputs_test['builtInParams'] || {}, + client_params: operation_inputs_test['clientParams'] || {} ) end end @@ -117,12 +117,13 @@ def initialize(options) @client_params = options[:client_params].map do |k,v| Param.new(Underscore.underscore(k), v) end - @client_params += options[:built_in_params].map do |k,v| built_in_to_param(k, v) end - # the expected default of UseGlobalEndpoint does not match the SDK's default value - if @service.identifier == 's3' && !options[:built_in_params].include?('AWS::S3::UseGlobalEndpoint') + # the expected default of UseGlobalEndpoint in rules + # does not match the Ruby SDK's default value + if @service.identifier == 's3' && + !options[:built_in_params].include?('AWS::S3::UseGlobalEndpoint') @client_params << built_in_to_param('AWS::S3::UseGlobalEndpoint', false) end end diff --git a/build_tools/aws-sdk-code-generator/templates/endpoints_module.mustache b/build_tools/aws-sdk-code-generator/templates/endpoints_module.mustache index 7ae625341e5..a93a5860b8a 100644 --- a/build_tools/aws-sdk-code-generator/templates/endpoints_module.mustache +++ b/build_tools/aws-sdk-code-generator/templates/endpoints_module.mustache @@ -11,11 +11,6 @@ module {{module_name}} {{#endpoint_classes}} class {{name}} def self.build(context) - {{#has_endpoint_built_in?}} - unless context.config.regional_endpoint - endpoint = context.config.endpoint.to_s - end - {{/has_endpoint_built_in?}} {{module_name}}::EndpointParameters.new( {{#parameters}} {{#static_string?}} diff --git a/build_tools/aws-sdk-code-generator/templates/endpoints_plugin.mustache b/build_tools/aws-sdk-code-generator/templates/endpoints_plugin.mustache index 6bdef3195e6..834fb111a36 100644 --- a/build_tools/aws-sdk-code-generator/templates/endpoints_plugin.mustache +++ b/build_tools/aws-sdk-code-generator/templates/endpoints_plugin.mustache @@ -44,11 +44,20 @@ module {{module_name}} context[:auth_scheme] = Aws::Endpoints.resolve_auth_scheme(context, endpoint) - @handler.call(context) + with_metrics(context) { @handler.call(context) } end private + def with_metrics(context, &block) + metrics = [] + metrics << 'ENDPOINT_OVERRIDE' unless context.config.regional_endpoint + if context[:auth_scheme] && context[:auth_scheme]['name'] == 'sigv4a' + metrics << 'SIGV4A_SIGNING' + end + Aws::Plugins::UserAgent.metric(*metrics, &block) + end + def apply_endpoint_headers(context, headers) headers.each do |key, values| value = values diff --git a/build_tools/aws-sdk-code-generator/templates/spec/endpoint_provider_spec_class.mustache b/build_tools/aws-sdk-code-generator/templates/spec/endpoint_provider_spec_class.mustache index 1a3e9536d2c..b86ebcfc4e6 100644 --- a/build_tools/aws-sdk-code-generator/templates/spec/endpoint_provider_spec_class.mustache +++ b/build_tools/aws-sdk-code-generator/templates/spec/endpoint_provider_spec_class.mustache @@ -11,7 +11,7 @@ module {{module_name}} subject { {{module_name}}::EndpointProvider.new } {{#endpoint_tests}} - context '{{documentation}}' do + context "{{{documentation}}}" do let(:expected) do {{{expect}}} end diff --git a/build_tools/services.rb b/build_tools/services.rb index 0909815bc96..e68d393bea6 100644 --- a/build_tools/services.rb +++ b/build_tools/services.rb @@ -10,10 +10,10 @@ class ServiceEnumerator MANIFEST_PATH = File.expand_path('../../services.json', __FILE__) # Minimum `aws-sdk-core` version for new gem builds - MINIMUM_CORE_VERSION = "3.203.0" + MINIMUM_CORE_VERSION = "3.204.1" # Minimum `aws-sdk-core` version for new S3 gem builds - MINIMUM_CORE_VERSION_S3 = "3.203.0" + MINIMUM_CORE_VERSION_S3 = "3.204.1" EVENTSTREAM_PLUGIN = "Aws::Plugins::EventStreamConfiguration" diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index 7c4ced811ce..cc88a654bff 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Issue - Additional metrics collection in the User-Agent plugin. + 3.204.0 (2024-09-10) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/regional_endpoint.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/regional_endpoint.rb index dd82bf90421..ede2b2d8b6c 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/regional_endpoint.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/regional_endpoint.rb @@ -205,6 +205,7 @@ def handle_legacy_pseudo_regions(cfg) cfg.override_config(:region, new_region) end end + # set a default endpoint in config using legacy (endpoints.json) resolver def resolve_legacy_endpoint(cfg) endpoint_prefix = cfg.api.metadata['endpointPrefix'] diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/user_agent.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/user_agent.rb index df7d94e1bd4..ac39fcbc37e 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/user_agent.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/user_agent.rb @@ -17,7 +17,10 @@ class UserAgent < Seahorse::Client::Plugin "S3_CRYPTO_V2": "I", "S3_EXPRESS_BUCKET": "J", "S3_ACCESS_GRANTS": "K", - "GZIP_REQUEST_COMPRESSION": "L" + "GZIP_REQUEST_COMPRESSION": "L", + "PROTOCOL_RPC_V2_CBOR": "M", + "ENDPOINT_OVERRIDE": "N", + "SIGV4A_SIGNING": "S" } METRICS @@ -45,15 +48,13 @@ def self.feature(_feature, &block) block.call end - def self.metric(metric, &block) + def self.metric(*metrics, &block) Thread.current[:aws_sdk_core_user_agent_metric] ||= [] - Thread.current[:aws_sdk_core_user_agent_metric] << METRICS[metric] + metrics = metrics.map { |metric| METRICS[metric] }.compact + Thread.current[:aws_sdk_core_user_agent_metric].concat(metrics) block.call ensure - Thread.current[:aws_sdk_core_user_agent_metric].pop - if Thread.current[:aws_sdk_core_user_agent_metric].empty? - Thread.current[:aws_sdk_core_user_agent_metric] = nil - end + Thread.current[:aws_sdk_core_user_agent_metric].pop(metrics.size) end # @api private @@ -166,7 +167,10 @@ def framework_metadata end def metric_metadata - return unless Thread.current[:aws_sdk_core_user_agent_metric] + if Thread.current[:aws_sdk_core_user_agent_metric].nil? || + Thread.current[:aws_sdk_core_user_agent_metric].empty? + return + end metrics = Thread.current[:aws_sdk_core_user_agent_metric].join(',') # Metric metadata is limited to 1024 bytes diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/rpc_v2/handler.rb b/gems/aws-sdk-core/lib/aws-sdk-core/rpc_v2/handler.rb index 278daee7f57..0e0d4049184 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/rpc_v2/handler.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/rpc_v2/handler.rb @@ -7,7 +7,7 @@ class Handler < Seahorse::Client::Handler # @return [Seahorse::Client::Response] def call(context) build_request(context) - response = @handler.call(context) + response = with_metric { @handler.call(context) } response.on(200..299) { |resp| resp.data = parse_body(context) } response.on(200..599) { |_resp| apply_request_id(context) } response @@ -15,6 +15,10 @@ def call(context) private + def with_metric(&block) + Aws::Plugins::UserAgent.metric('PROTOCOL_RPC_V2_CBOR', &block) + end + def build_request(context) context.http_request.headers['smithy-protocol'] = 'rpc-v2-cbor' context.http_request.http_method = 'POST' diff --git a/gems/aws-sdk-core/spec/aws/plugins/request_compression_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/request_compression_spec.rb index 897b1a7ef8a..48f61ae884b 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/request_compression_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/request_compression_spec.rb @@ -121,13 +121,6 @@ def expect_uncompressed_body(resp, body) end context 'request compression operation' do - it 'sets user-agent metric for the operation' do - resp = client.some_operation(body: uncompressed_body) - metric = Aws::Plugins::UserAgent::METRICS['GZIP_REQUEST_COMPRESSION'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end - it 'compresses the body and sets the content-encoding header' do resp = client.some_operation(body: uncompressed_body) expect(resp.context.http_request.headers['Content-Encoding']).to eq('gzip') @@ -165,13 +158,6 @@ def expect_uncompressed_body(resp, body) expect_uncompressed_body(resp, small_body) end - it 'sets user-agent metric for a streaming operation' do - resp = client.operation_streaming(body: 'body') - metric = Aws::Plugins::UserAgent::METRICS['GZIP_REQUEST_COMPRESSION'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end - it 'compresses a large streaming body' do large_body = StringIO.new('.' * 16_385) client.stub_responses(:operation_streaming, -> (context) do diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb index 13f66c5ab3f..067231e8004 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb @@ -32,15 +32,6 @@ module Plugins client = RetryErrorsSvc::Client.new(retry_mode: 'legacy', region: 'us-west-2') expect(client.handlers.entries.map(&:handler_class)).to include(RetryErrors::LegacyHandler) end - - it 'sets user-agent metric' do - client = RetryErrorsSvc::Client.new(retry_mode: 'legacy', region: 'us-west-2') - stub_request(:post, client.config.endpoint) - resp = client.example_operation - metric = Aws::Plugins::UserAgent::METRICS['RETRY_MODE_LEGACY'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end end describe RetryErrors::LegacyHandler do diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb index 82838e29bdd..9b56972f352 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb @@ -106,24 +106,6 @@ module Plugins .to receive(:correct_clock_skew).and_return('true') expect(client.config.correct_clock_skew).to eq(true) end - - it 'sets user-agent metric for standard' do - client = RetryErrorsSvc::Client.new(retry_mode: 'standard', region: 'us-west-2') - stub_request(:post, client.config.endpoint) - resp = client.example_operation - metric = Aws::Plugins::UserAgent::METRICS['RETRY_MODE_STANDARD'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end - - it 'sets user-agent metric for adaptive' do - client = RetryErrorsSvc::Client.new(retry_mode: 'adaptive', region: 'us-west-2') - stub_request(:post, client.config.endpoint) - resp = client.example_operation - metric = Aws::Plugins::UserAgent::METRICS['RETRY_MODE_ADAPTIVE'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end end describe RetryErrors::Handler do diff --git a/gems/aws-sdk-s3/spec/plugins/express_session_auth_spec.rb b/gems/aws-sdk-s3/spec/plugins/express_session_auth_spec.rb index f2ffd887b86..60d165a3237 100644 --- a/gems/aws-sdk-s3/spec/plugins/express_session_auth_spec.rb +++ b/gems/aws-sdk-s3/spec/plugins/express_session_auth_spec.rb @@ -128,13 +128,6 @@ module S3 end client.get_object(bucket: bucket, key: 'key') end - - it 'sets user-agent metric' do - resp = client.get_object(bucket: 'bucket--use1-az2--x-s3', key: 'key') - metric = Aws::Plugins::UserAgent::METRICS['S3_EXPRESS_BUCKET'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end end # does not have http checksum trait, but requires a checksum (md5)