From e47dcb8b9853b5d39ba0a3530c69081bed25613d Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Mon, 27 Nov 2023 11:30:58 -0800 Subject: [PATCH 1/9] Measure usage of .save --- lib/stripe/api_operations/request.rb | 26 ++++++++++++++------------ lib/stripe/api_operations/save.rb | 2 +- lib/stripe/stripe_client.rb | 24 ++++++++++++++---------- test/stripe/stripe_client_test.rb | 5 ++++- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/lib/stripe/api_operations/request.rb b/lib/stripe/api_operations/request.rb index 0d237b840..f328ee9db 100644 --- a/lib/stripe/api_operations/request.rb +++ b/lib/stripe/api_operations/request.rb @@ -5,14 +5,14 @@ module APIOperations module Request module ClassMethods def execute_resource_request(method, url, - params = {}, opts = {}) + params = {}, opts = {}, usage = []) execute_resource_request_internal( - :execute_request, method, url, params, opts + :execute_request, method, url, params, opts, usage ) end - def execute_resource_request_stream(method, url, - params = {}, opts = {}, + def execute_resource_request_stream(method, url, + params = {}, opts = {}, usage = [], &read_body_chunk_block) execute_resource_request_internal( :execute_request_stream, @@ -20,18 +20,19 @@ def execute_resource_request_stream(method, url, url, params, opts, + usage, &read_body_chunk_block ) end - private def request_stripe_object(method:, path:, params:, opts: {}) - resp, opts = execute_resource_request(method, path, params, opts) + private def request_stripe_object(method:, path:, params:, opts: {}, usage: []) + resp, opts = execute_resource_request(method, path, params, opts, usage) Util.convert_to_stripe_object_with_params(resp.data, params, opts) end private def execute_resource_request_internal(client_request_method_sym, method, url, - params, opts, + params, opts, usage, &read_body_chunk_block) params ||= {} @@ -53,7 +54,7 @@ def execute_resource_request_stream(method, url, client_request_method_sym, method, url, api_base: api_base, api_key: api_key, - headers: headers, params: params, + headers: headers, params: params, usage: usage, &read_body_chunk_block ) @@ -66,6 +67,7 @@ def execute_resource_request_stream(method, url, [resp, opts_to_persist] end + # TODO (major) # This method used to be called `request`, but it's such a short name # that it eventually conflicted with the name of a field on an API # resource (specifically, `Event#request`), so it was renamed to @@ -111,9 +113,9 @@ def self.included(base) end protected def execute_resource_request(method, url, - params = {}, opts = {}) + params = {}, opts = {}, usage = []) opts = @opts.merge(Util.normalize_opts(opts)) - self.class.execute_resource_request(method, url, params, opts) + self.class.execute_resource_request(method, url, params, opts, usage) end protected def execute_resource_request_stream(method, url, @@ -125,8 +127,8 @@ def self.included(base) ) end - private def request_stripe_object(method:, path:, params:, opts: {}) - resp, opts = execute_resource_request(method, path, params, opts) + private def request_stripe_object(method:, path:, params:, opts: {}, usage: []) + resp, opts = execute_resource_request(method, path, params, opts, usage) Util.convert_to_stripe_object_with_params(resp.data, params, opts) end diff --git a/lib/stripe/api_operations/save.rb b/lib/stripe/api_operations/save.rb index af7d3ada9..8c8efadcf 100644 --- a/lib/stripe/api_operations/save.rb +++ b/lib/stripe/api_operations/save.rb @@ -66,7 +66,7 @@ def save(params = {}, opts = {}) # generated a uri for this object with an identifier baked in values.delete(:id) - resp, opts = execute_resource_request(:post, save_url, values, opts) + resp, opts = execute_resource_request(:post, save_url, values, opts, ["save"]) initialize_from(resp.data, opts) end extend Gem::Deprecate diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 6bb05815f..44d0d4ce4 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -209,9 +209,9 @@ def request end def execute_request(method, path, - api_base: nil, api_key: nil, headers: {}, params: {}) + api_base: nil, api_key: nil, headers: {}, params: {}, usage: []) http_resp, api_key = execute_request_internal( - method, path, api_base, api_key, headers, params + method, path, api_base, api_key, headers, params, usage ) begin @@ -240,7 +240,7 @@ def execute_request(method, path, # passed, then a StripeStreamResponse is returned containing an IO stream # with the response body. def execute_request_stream(method, path, - api_base: nil, api_key: nil, + api_base: nil, api_key: nil, usage: [], headers: {}, params: {}, &read_body_chunk_block) unless block_given? @@ -249,7 +249,7 @@ def execute_request_stream(method, path, end http_resp, api_key = execute_request_internal( - method, path, api_base, api_key, headers, params, &read_body_chunk_block + method, path, api_base, api_key, headers, params, usage, &read_body_chunk_block ) # When the read_body_chunk_block is given, we no longer have access to the @@ -428,7 +428,7 @@ def self.maybe_gc_connection_managers end private def execute_request_internal(method, path, - api_base, api_key, headers, params, + api_base, api_key, headers, params, usage, &read_body_chunk_block) raise ArgumentError, "method should be a symbol" \ unless method.is_a?(Symbol) @@ -490,7 +490,8 @@ def self.maybe_gc_connection_managers end http_resp = - execute_request_with_rescues(method, api_base, headers, context) do + execute_request_with_rescues(method, api_base, headers, usage, context) do + self.class .default_connection_manager(config) .execute_request(method, url, @@ -560,7 +561,7 @@ def self.maybe_gc_connection_managers http_status >= 400 end - private def execute_request_with_rescues(method, api_base, headers, context) + private def execute_request_with_rescues(method, api_base, headers, usage, context) num_retries = 0 begin @@ -586,7 +587,7 @@ def self.maybe_gc_connection_managers if config.enable_telemetry? && context.request_id request_duration_ms = (request_duration * 1000).to_i @last_request_metrics = - StripeRequestMetrics.new(context.request_id, request_duration_ms) + StripeRequestMetrics.new(context.request_id, request_duration_ms, usage: usage) end # We rescue all exceptions from a request so that we have an easy spot to @@ -1038,13 +1039,16 @@ class StripeRequestMetrics # Request duration in milliseconds attr_accessor :request_duration_ms - def initialize(request_id, request_duration_ms) + attr_accessor :usage + + def initialize(request_id, request_duration_ms, usage: []) self.request_id = request_id self.request_duration_ms = request_duration_ms + self.usage = usage end def payload - { request_id: request_id, request_duration_ms: request_duration_ms } + { request_id: request_id, request_duration_ms: request_duration_ms, usage: usage} end end end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index ff5178695..57199a0d6 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -1408,7 +1408,9 @@ class StripeClientTest < Test::Unit::TestCase false end.to_return(body: JSON.generate(object: "charge")) - Stripe::Charge.list + cus = Stripe::Customer.new("cus_xyz") + cus.description = "hello" + cus.save() Stripe::Charge.list assert(!trace_metrics_header.nil?) @@ -1416,6 +1418,7 @@ class StripeClientTest < Test::Unit::TestCase trace_payload = JSON.parse(trace_metrics_header) assert(trace_payload["last_request_metrics"]["request_id"] == "req_123") assert(!trace_payload["last_request_metrics"]["request_duration_ms"].nil?) + assert(trace_payload["last_request_metrics"]["usage"] == ["save"]) end end From fdde37d8f0c9f3acfd83966256618218bf2c2140 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Mon, 27 Nov 2023 11:31:45 -0800 Subject: [PATCH 2/9] README note --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 337e93ccd..4a6604e2d 100644 --- a/README.md +++ b/README.md @@ -293,8 +293,9 @@ API. ### Request latency telemetry -By default, the library sends request latency telemetry to Stripe. These -numbers help Stripe improve the overall latency of its API for all users. +By default, the library sends telemetry to Stripe regarding request latency and feature usage. These +numbers help Stripe improve the overall latency of its API for all users, and +improve popular features. You can disable this behavior if you prefer: From f0c4e792414cc0c501eeb00731d99d8bb92ff0b5 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Tue, 28 Nov 2023 13:36:30 -0800 Subject: [PATCH 3/9] --auto-correct -> --autocorrect --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index fa1d935d7..11ec33f6c 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ update-version: codegen-format: bundle install --quiet - bundle exec rubocop -o /dev/null --auto-correct + bundle exec rubocop -o /dev/null --autocorrect ci-test: bundle install && bundle exec rake test From aa26c78455ae04af3a2fdb9857ff538cbe627d3c Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Tue, 28 Nov 2023 13:36:34 -0800 Subject: [PATCH 4/9] Format --- lib/stripe/api_operations/request.rb | 4 ++-- lib/stripe/stripe_client.rb | 3 +-- test/stripe/stripe_client_test.rb | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/stripe/api_operations/request.rb b/lib/stripe/api_operations/request.rb index f328ee9db..505e45696 100644 --- a/lib/stripe/api_operations/request.rb +++ b/lib/stripe/api_operations/request.rb @@ -11,7 +11,7 @@ def execute_resource_request(method, url, ) end - def execute_resource_request_stream(method, url, + def execute_resource_request_stream(method, url, params = {}, opts = {}, usage = [], &read_body_chunk_block) execute_resource_request_internal( @@ -67,7 +67,7 @@ def execute_resource_request_stream(method, url, [resp, opts_to_persist] end - # TODO (major) + # TODO: (major) # This method used to be called `request`, but it's such a short name # that it eventually conflicted with the name of a field on an API # resource (specifically, `Event#request`), so it was renamed to diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 44d0d4ce4..dd41f7dd5 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -491,7 +491,6 @@ def self.maybe_gc_connection_managers http_resp = execute_request_with_rescues(method, api_base, headers, usage, context) do - self.class .default_connection_manager(config) .execute_request(method, url, @@ -1048,7 +1047,7 @@ def initialize(request_id, request_duration_ms, usage: []) end def payload - { request_id: request_id, request_duration_ms: request_duration_ms, usage: usage} + { request_id: request_id, request_duration_ms: request_duration_ms, usage: usage } end end end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 57199a0d6..12296493a 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -1410,7 +1410,7 @@ class StripeClientTest < Test::Unit::TestCase cus = Stripe::Customer.new("cus_xyz") cus.description = "hello" - cus.save() + cus.save Stripe::Charge.list assert(!trace_metrics_header.nil?) From 580a37755a9ce88e9b276850ac209a8138668d99 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein <52928443+richardm-stripe@users.noreply.github.com> Date: Tue, 28 Nov 2023 15:29:56 -0800 Subject: [PATCH 5/9] Update lib/stripe/stripe_client.rb Co-authored-by: anniel-stripe <97691964+anniel-stripe@users.noreply.github.com> --- lib/stripe/stripe_client.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index dd41f7dd5..a03a4d776 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -1038,6 +1038,7 @@ class StripeRequestMetrics # Request duration in milliseconds attr_accessor :request_duration_ms + # a list of strings describing behaviors we track attr_accessor :usage def initialize(request_id, request_duration_ms, usage: []) From 0455e39169e782a63abdc0eb828e12e2e2c3963d Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Wed, 29 Nov 2023 12:50:38 -0800 Subject: [PATCH 6/9] standard docstring --- lib/stripe/stripe_client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index a03a4d776..7383fdec1 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -1038,7 +1038,7 @@ class StripeRequestMetrics # Request duration in milliseconds attr_accessor :request_duration_ms - # a list of strings describing behaviors we track + # list of names of tracked behaviors associated with this request attr_accessor :usage def initialize(request_id, request_duration_ms, usage: []) From b44211190b6914c790e5dfaaf9f030d92c85b046 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 1 Dec 2023 12:54:58 -0800 Subject: [PATCH 7/9] Omit usage if empty --- lib/stripe/stripe_client.rb | 6 +++++- test/stripe/stripe_client_test.rb | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 7383fdec1..8497b14a0 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -1048,7 +1048,11 @@ def initialize(request_id, request_duration_ms, usage: []) end def payload - { request_id: request_id, request_duration_ms: request_duration_ms, usage: usage } + ret = { request_id: request_id, request_duration_ms: request_duration_ms} + if !usage.nil? && !usage.empty? + ret[:usage] = usage + end + ret end end end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 12296493a..9de7931de 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -1416,9 +1416,17 @@ class StripeClientTest < Test::Unit::TestCase assert(!trace_metrics_header.nil?) trace_payload = JSON.parse(trace_metrics_header) + assert(trace_payload["last_request_metrics"]["request_id"] == "req_123") assert(!trace_payload["last_request_metrics"]["request_duration_ms"].nil?) assert(trace_payload["last_request_metrics"]["usage"] == ["save"]) + + Stripe::Charge.list + trace_payload = JSON.parse(trace_metrics_header) + assert(trace_payload["last_request_metrics"]["request_id"] == "req_123") + assert(!trace_payload["last_request_metrics"]["request_duration_ms"].nil?) + assert(trace_payload["last_request_metrics"]["usage"].nil?) + end end From 3fdaa23ae49d42766ece2e783b3f75509c1c4e1c Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 1 Dec 2023 12:58:22 -0800 Subject: [PATCH 8/9] Format --- lib/stripe/stripe_client.rb | 6 ++---- test/stripe/stripe_client_test.rb | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 8497b14a0..219b483c7 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -1048,10 +1048,8 @@ def initialize(request_id, request_duration_ms, usage: []) end def payload - ret = { request_id: request_id, request_duration_ms: request_duration_ms} - if !usage.nil? && !usage.empty? - ret[:usage] = usage - end + ret = { request_id: request_id, request_duration_ms: request_duration_ms } + ret[:usage] = usage if !usage.nil? && !usage.empty? ret end end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 9de7931de..990f7ce4c 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -1426,7 +1426,6 @@ class StripeClientTest < Test::Unit::TestCase assert(trace_payload["last_request_metrics"]["request_id"] == "req_123") assert(!trace_payload["last_request_metrics"]["request_duration_ms"].nil?) assert(trace_payload["last_request_metrics"]["usage"].nil?) - end end From 1af9e1a864923baf234a4857e876067eea38ac40 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein <52928443+richardm-stripe@users.noreply.github.com> Date: Fri, 1 Dec 2023 13:28:28 -0800 Subject: [PATCH 9/9] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4a6604e2d..f96c75a8e 100644 --- a/README.md +++ b/README.md @@ -291,7 +291,7 @@ Stripe.set_app_info('MyAwesomePlugin', version: '1.2.34', url: 'https://myawesom This information is passed along when the library makes calls to the Stripe API. -### Request latency telemetry +### Telemetry By default, the library sends telemetry to Stripe regarding request latency and feature usage. These numbers help Stripe improve the overall latency of its API for all users, and