From c1d0ced565ba4a4f70f97d9f14ced6755486e1fa Mon Sep 17 00:00:00 2001 From: Leena Hai Ha Nguyen <32082113+nguyhh@users.noreply.github.com> Date: Wed, 11 Jan 2023 12:43:23 -0500 Subject: [PATCH 1/5] Improve request events instrumentation (#1162) * Add response field to RequestEndEvent * Add request body field to RequestEventEnd * Add request headers field to RequestEventEnd * Create a RequestContext class * Create a ResponseContext class * Update tests --- lib/stripe/instrumentation.rb | 52 +++++++++++++++++++++++++---- lib/stripe/stripe_client.rb | 42 +++++++++++++---------- test/stripe/instrumentation_test.rb | 22 ++++++++++-- test/stripe/stripe_client_test.rb | 20 ++++++++--- 4 files changed, 104 insertions(+), 32 deletions(-) diff --git a/lib/stripe/instrumentation.rb b/lib/stripe/instrumentation.rb index cc017be7b..a30e4c776 100644 --- a/lib/stripe/instrumentation.rb +++ b/lib/stripe/instrumentation.rb @@ -33,6 +33,10 @@ class RequestEndEvent attr_reader :num_retries attr_reader :path attr_reader :request_id + attr_reader :response_header + attr_reader :response_body + attr_reader :request_header + attr_reader :request_body # Arbitrary user-provided data in the form of a Ruby hash that's passed # from subscribers on `request_begin` to subscribers on `request_end`. @@ -40,19 +44,53 @@ class RequestEndEvent # in `request_end`. attr_reader :user_data - def initialize(duration:, http_status:, method:, num_retries:, path:, - request_id:, user_data: nil) - @duration = duration - @http_status = http_status - @method = method + def initialize(request_context:, response_context:, + num_retries:, user_data: nil) + @duration = request_context.duration + @http_status = response_context.http_status + @method = request_context.method @num_retries = num_retries - @path = path - @request_id = request_id + @path = request_context.path + @request_id = request_context.request_id @user_data = user_data + @response_header = response_context.header + @response_body = response_context.body + @request_header = request_context.header + @request_body = request_context.body freeze end end + class RequestContext + attr_reader :duration + attr_reader :method + attr_reader :path + attr_reader :request_id + attr_reader :body + attr_reader :header + + def initialize(duration:, context:, header:) + @duration = duration + @method = context.method + @path = context.path + @request_id = context.request_id + @body = context.body + @header = header + end + end + + class ResponseContext + attr_reader :http_status + attr_reader :body + attr_reader :header + + def initialize(http_status:, response:) + @http_status = http_status + @header = response ? response.to_hash : nil + @body = response ? response.body : nil + end + end + # This class was renamed for consistency. This alias is here for backwards # compatibility. RequestEvent = RequestEndEvent diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 8623e8a7d..54f06c4c8 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -494,15 +494,16 @@ def self.maybe_gc_connection_managers end end - http_resp = execute_request_with_rescues(method, api_base, context) do - self.class - .default_connection_manager(config) - .execute_request(method, url, - body: body, - headers: headers, - query: query, - &response_block) - end + http_resp = + execute_request_with_rescues(method, api_base, headers, context) do + self.class + .default_connection_manager(config) + .execute_request(method, url, + body: body, + headers: headers, + query: query, + &response_block) + end [http_resp, api_key] end @@ -564,7 +565,7 @@ def self.maybe_gc_connection_managers http_status >= 400 end - private def execute_request_with_rescues(method, api_base, context) + private def execute_request_with_rescues(method, api_base, headers, context) num_retries = 0 begin @@ -587,7 +588,7 @@ def self.maybe_gc_connection_managers log_response(context, request_start, http_status, resp.body, resp) notify_request_end(context, request_duration, http_status, - num_retries, user_data) + num_retries, user_data, resp, headers) if config.enable_telemetry? && context.request_id request_duration_ms = (request_duration * 1000).to_i @@ -614,7 +615,7 @@ def self.maybe_gc_connection_managers log_response_error(error_context, request_start, e) end notify_request_end(context, request_duration, http_status, num_retries, - user_data) + user_data, resp, headers) if self.class.should_retry?(e, method: method, @@ -657,17 +658,24 @@ def self.maybe_gc_connection_managers end private def notify_request_end(context, duration, http_status, num_retries, - user_data) + user_data, resp, headers) return if !Instrumentation.any_subscribers?(:request_end) && !Instrumentation.any_subscribers?(:request) - event = Instrumentation::RequestEndEvent.new( + request_context = Stripe::Instrumentation::RequestContext.new( duration: duration, + context: context, + header: headers + ) + response_context = Stripe::Instrumentation::ResponseContext.new( http_status: http_status, - method: context.method, + response: resp + ) + + event = Instrumentation::RequestEndEvent.new( + request_context: request_context, + response_context: response_context, num_retries: num_retries, - path: context.path, - request_id: context.request_id, user_data: user_data || {} ) Stripe::Instrumentation.notify(:request_end, event) diff --git a/test/stripe/instrumentation_test.rb b/test/stripe/instrumentation_test.rb index be2e0ebf3..b121b329c 100644 --- a/test/stripe/instrumentation_test.rb +++ b/test/stripe/instrumentation_test.rb @@ -58,13 +58,29 @@ class InstrumentationTest < Test::Unit::TestCase context "RequestEventEnd" do should "return a frozen object" do - event = Stripe::Instrumentation::RequestEndEvent.new( + mock_context = stub( duration: 0.1, - http_status: 200, method: :get, - num_retries: 0, path: "/v1/test", request_id: "req_123", + body: "" + ) + + request_context = Stripe::Instrumentation::RequestContext.new( + duration: 0.1, + context: mock_context, + header: nil + ) + + response_context = Stripe::Instrumentation::ResponseContext.new( + http_status: 200, + response: nil + ) + + event = Stripe::Instrumentation::RequestEndEvent.new( + num_retries: 0, + request_context: request_context, + response_context: response_context, user_data: nil ) diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 11c1953d4..0ec61155b 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -1443,19 +1443,25 @@ class StripeClientTest < Test::Unit::TestCase should "notify a subscriber of a successful HTTP request" do events = [] Stripe::Instrumentation.subscribe(:request_end, :test) { |event| events << event } + stub_response_body = JSON.generate(object: "charge") - stub_request(:get, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(object: "charge"), headers: { "Request-ID": "req_123" }) - Stripe::Charge.list + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .with(body: { "amount" => "50", "currency" => "usd", "card" => "sc_token" }) + .to_return(body: stub_response_body, headers: { "Request-ID": "req_123" }) + + Stripe::Charge.create(amount: 50, currency: "usd", card: "sc_token") assert_equal(1, events.size) event = events.first - assert_equal(:get, event.method) + assert_equal(:post, event.method) assert_equal("/v1/charges", event.path) assert_equal(200, event.http_status) assert(event.duration.positive?) assert_equal(0, event.num_retries) assert_equal("req_123", event.request_id) + assert_equal(stub_response_body, event.response_body) + assert_equal("amount=50¤cy=usd&card=sc_token", event.request_body) + assert(event.request_header) end should "notify a subscriber of a StripeError" do @@ -1484,6 +1490,7 @@ class StripeClientTest < Test::Unit::TestCase assert_equal(500, event.http_status) assert(event.duration.positive?) assert_equal(0, event.num_retries) + assert_equal(JSON.generate(error: error), event.response_body) end should "notify a subscriber of a network error" do @@ -1526,8 +1533,9 @@ class StripeClientTest < Test::Unit::TestCase events = [] Stripe::Instrumentation.subscribe(:request, :test) { |event| events << event } + stub_response_body = JSON.generate(object: "charge") stub_request(:get, "#{Stripe.api_base}/v1/charges") - .to_return(body: JSON.generate(object: "charge")) + .to_return(body: stub_response_body) Stripe::Charge.list assert_equal(1, events.size) @@ -1537,6 +1545,8 @@ class StripeClientTest < Test::Unit::TestCase assert_equal(200, event.http_status) assert(event.duration.positive?) assert_equal(0, event.num_retries) + assert_equal(stub_response_body, event.response_body) + assert(event.request_header) end end end From e0a50213e778282dec131b475d077899505104a4 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Thu, 12 Jan 2023 11:20:50 -0600 Subject: [PATCH 2/5] Bump version to 8.1.0 --- CHANGELOG.md | 3 +++ VERSION | 2 +- lib/stripe/version.rb | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe32b01b9..01e1bf6ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 8.1.0 - 2023-01-12 +* [#1162](https://github.com/stripe/stripe-ruby/pull/1162) Improve request events instrumentation + ## 8.0.0 - 2022-11-16 * [#1144](https://github.com/stripe/stripe-ruby/pull/1144) Next major release changes diff --git a/VERSION b/VERSION index ae9a76b92..8104cabd3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.0.0 +8.1.0 diff --git a/lib/stripe/version.rb b/lib/stripe/version.rb index 853117803..0c2d479cd 100644 --- a/lib/stripe/version.rb +++ b/lib/stripe/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Stripe - VERSION = "8.0.0" + VERSION = "8.1.0" end From e11400adc8aa917a494dccb06ed9a2192d461005 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Thu, 12 Jan 2023 11:27:02 -0600 Subject: [PATCH 3/5] Set version to 8.1.0 to simplify merge --- VERSION | 2 +- lib/stripe/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index ea4f9871c..8104cabd3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.1.0-beta.4 +8.1.0 diff --git a/lib/stripe/version.rb b/lib/stripe/version.rb index 8aa48bf54..0c2d479cd 100644 --- a/lib/stripe/version.rb +++ b/lib/stripe/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Stripe - VERSION = "8.1.0-beta.4" + VERSION = "8.1.0" end From 2b841bc109a3f01d4dabd2f018c8c84a3ac70f9b Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Thu, 12 Jan 2023 11:27:02 -0600 Subject: [PATCH 4/5] Reset version to 8.1.0-beta.4 --- VERSION | 2 +- lib/stripe/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index 8104cabd3..ea4f9871c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.1.0 +8.1.0-beta.4 diff --git a/lib/stripe/version.rb b/lib/stripe/version.rb index 0c2d479cd..8aa48bf54 100644 --- a/lib/stripe/version.rb +++ b/lib/stripe/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Stripe - VERSION = "8.1.0" + VERSION = "8.1.0-beta.4" end From fe794f4868ba2ddd9a60fb13bbdf31b4a6eaede3 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Thu, 12 Jan 2023 11:27:18 -0600 Subject: [PATCH 5/5] Codegen for openapi v218 --- OPENAPI_VERSION | 2 +- lib/stripe/object_types.rb | 1 + lib/stripe/resources.rb | 1 + lib/stripe/resources/quote.rb | 4 ++-- lib/stripe/resources/tax/registration.rb | 17 +++++++++++++++++ 5 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 lib/stripe/resources/tax/registration.rb diff --git a/OPENAPI_VERSION b/OPENAPI_VERSION index b87b42e0c..76d4091fa 100644 --- a/OPENAPI_VERSION +++ b/OPENAPI_VERSION @@ -1 +1 @@ -v217 \ No newline at end of file +v218 \ No newline at end of file diff --git a/lib/stripe/object_types.rb b/lib/stripe/object_types.rb index c5a30548e..107456dac 100644 --- a/lib/stripe/object_types.rb +++ b/lib/stripe/object_types.rb @@ -98,6 +98,7 @@ def self.object_names_to_classes SubscriptionItem::OBJECT_NAME => SubscriptionItem, SubscriptionSchedule::OBJECT_NAME => SubscriptionSchedule, Tax::Calculation::OBJECT_NAME => Tax::Calculation, + Tax::Registration::OBJECT_NAME => Tax::Registration, Tax::Transaction::OBJECT_NAME => Tax::Transaction, TaxCode::OBJECT_NAME => TaxCode, TaxId::OBJECT_NAME => TaxId, diff --git a/lib/stripe/resources.rb b/lib/stripe/resources.rb index 41c4880e1..d04a6c248 100644 --- a/lib/stripe/resources.rb +++ b/lib/stripe/resources.rb @@ -85,6 +85,7 @@ require "stripe/resources/subscription_item" require "stripe/resources/subscription_schedule" require "stripe/resources/tax/calculation" +require "stripe/resources/tax/registration" require "stripe/resources/tax/transaction" require "stripe/resources/tax_code" require "stripe/resources/tax_id" diff --git a/lib/stripe/resources/quote.rb b/lib/stripe/resources/quote.rb index a901b2068..b89861447 100644 --- a/lib/stripe/resources/quote.rb +++ b/lib/stripe/resources/quote.rb @@ -32,7 +32,7 @@ def cancel(params = {}, opts = {}) def draft_quote(params = {}, opts = {}) request_stripe_object( method: :post, - path: format("/v1/quotes/%s/draft", { quote: CGI.escape(self["id"]) }), + path: format("/v1/quotes/%s/mark_draft", { quote: CGI.escape(self["id"]) }), params: params, opts: opts ) @@ -140,7 +140,7 @@ def self.cancel(quote, params = {}, opts = {}) def self.draft_quote(quote, params = {}, opts = {}) request_stripe_object( method: :post, - path: format("/v1/quotes/%s/draft", { quote: CGI.escape(quote) }), + path: format("/v1/quotes/%s/mark_draft", { quote: CGI.escape(quote) }), params: params, opts: opts ) diff --git a/lib/stripe/resources/tax/registration.rb b/lib/stripe/resources/tax/registration.rb new file mode 100644 index 000000000..4103f71f1 --- /dev/null +++ b/lib/stripe/resources/tax/registration.rb @@ -0,0 +1,17 @@ +# File generated from our OpenAPI spec +# frozen_string_literal: true + +module Stripe + module Tax + # A Tax `Registration` lets us know that your business is registered to collect tax on payments within a region, enabling you to [automatically collect tax](https://stripe.com/docs/tax). + # + # Stripe will not register on your behalf with the relevant authorities when you create a Tax `Registration` object. For more information on how to register to collect tax, see [our guide](https://stripe.com/docs/tax/registering). + class Registration < APIResource + extend Stripe::APIOperations::Create + extend Stripe::APIOperations::List + include Stripe::APIOperations::Save + + OBJECT_NAME = "tax.registration" + end + end +end