Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Measure usage of .save #1293

Merged
merged 10 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
24 changes: 13 additions & 11 deletions lib/stripe/api_operations/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,34 @@ 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 = {},
params = {}, opts = {}, usage = [],
&read_body_chunk_block)
execute_resource_request_internal(
:execute_request_stream,
method,
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 ||= {}

Expand All @@ -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
)

Expand All @@ -66,6 +67,7 @@ def execute_resource_request_stream(method, url,
[resp, opts_to_persist]
end

# TODO: (major)
anniel-stripe marked this conversation as resolved.
Show resolved Hide resolved
# 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
Expand Down Expand Up @@ -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,
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lib/stripe/api_operations/save.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
richardm-stripe marked this conversation as resolved.
Show resolved Hide resolved
initialize_from(resp.data, opts)
end
extend Gem::Deprecate
Expand Down
24 changes: 14 additions & 10 deletions lib/stripe/stripe_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -490,7 +490,7 @@ 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,
Expand Down Expand Up @@ -560,7 +560,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
Expand All @@ -586,7 +586,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
Expand Down Expand Up @@ -1038,13 +1038,17 @@ class StripeRequestMetrics
# Request duration in milliseconds
attr_accessor :request_duration_ms

def initialize(request_id, request_duration_ms)
# a list of strings describing behaviors we track
attr_accessor :usage
richardm-stripe marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down
5 changes: 4 additions & 1 deletion test/stripe/stripe_client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1408,14 +1408,17 @@ 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?)

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

Expand Down