Skip to content

Commit

Permalink
Merge pull request #696 from stripe/akropp-add-client-telemetry
Browse files Browse the repository at this point in the history
Adding metadata header to client requests
  • Loading branch information
brandur-stripe authored Nov 12, 2018
2 parents e2f4fa2 + ab248ec commit 9bc61c6
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 2 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ Gemfile.lock
tags
/.bundle/
coverage/
.idea/
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Metrics/BlockLength:
# Offense count: 8
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 626
Max: 659

# Offense count: 11
Metrics/CyclomaticComplexity:
Expand Down
10 changes: 10 additions & 0 deletions lib/stripe.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ module Stripe
@open_timeout = 30
@read_timeout = 80

@enable_telemetry = false

class << self
attr_accessor :stripe_account, :api_key, :api_base, :verify_ssl_certs, :api_version, :client_id, :connect_base, :uploads_base,
:open_timeout, :read_timeout
Expand Down Expand Up @@ -225,6 +227,14 @@ def self.max_network_retries=(val)
@max_network_retries = val.to_i
end

def self.enable_telemetry?
@enable_telemetry
end

def self.enable_telemetry=(val)
@enable_telemetry = val
end

# Sets some basic information about the running application that's sent along
# with API requests. Useful for plugin authors to identify their plugin when
# communicating with Stripe.
Expand Down
27 changes: 26 additions & 1 deletion lib/stripe/stripe_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class StripeClient
def initialize(conn = nil)
self.conn = conn || self.class.default_conn
@system_profiler = SystemProfiler.new
@last_request_metrics = nil
end

def self.active_client
Expand Down Expand Up @@ -113,7 +114,6 @@ def request

def execute_request(method, path,
api_base: nil, api_key: nil, headers: {}, params: {})

api_base ||= Stripe.api_base
api_key ||= Stripe.api_key

Expand Down Expand Up @@ -218,6 +218,9 @@ def execute_request_with_rescues(api_base, context)
resp = yield
context = context.dup_from_response(resp)
log_response(context, request_start, resp.status, resp.body)
if Stripe.enable_telemetry?
@last_request_metrics = StripeRequestMetrics.new(context.request_id, Time.now - request_start)
end

# We rescue all exceptions from a request so that we have an easy spot to
# implement our retry logic across the board. We'll re-raise if it's a type
Expand Down Expand Up @@ -425,6 +428,10 @@ def request_headers(api_key, method)
"Content-Type" => "application/x-www-form-urlencoded",
}

if Stripe.enable_telemetry? && !@last_request_metrics.nil?
headers["X-Stripe-Client-Telemetry"] = JSON.generate(last_request_metrics: @last_request_metrics.payload)
end

# It is only safe to retry network failures on post and delete
# requests if we add an Idempotency-Key header
if %i[post delete].include?(method) && Stripe.max_network_retries > 0
Expand Down Expand Up @@ -594,5 +601,23 @@ def user_agent
}.delete_if { |_k, v| v.nil? }
end
end

# StripeRequestMetrics tracks metadata to be reported to stripe for metrics collection
class StripeRequestMetrics
# The Stripe request ID of the response.
attr_accessor :request_id

# Request duration
attr_accessor :request_duration

def initialize(request_id, request_duration)
self.request_id = request_id
self.request_duration = request_duration
end

def payload
{ request_id: request_id, request_duration: request_duration }
end
end
end
end
45 changes: 45 additions & 0 deletions test/stripe/stripe_client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,51 @@ class StripeClientTest < Test::Unit::TestCase
end
end
end

context "#telemetry" do
teardown do
# make sure to always set telemetry back to false
# to not mutate global state
Stripe.enable_telemetry = false
end

should "not send metrics if enable trace flag is not set" do
Stripe.enable_telemetry = false

trace_metrics_header = nil
stub_request(:get, "#{Stripe.api_base}/v1/charges")
.with do |req|
trace_metrics_header = req.headers["X-Stripe-Client-Telemetry"]
false
end.to_return(body: JSON.generate(object: "charge"))

Stripe::Charge.list
assert(trace_metrics_header.nil?)

Stripe::Charge.list
assert(trace_metrics_header.nil?)
end

should "send metrics if enabled telemetry is true" do
Stripe.enable_telemetry = true

trace_metrics_header = nil
stub_request(:get, "#{Stripe.api_base}/v1/charges")
.with do |req|
trace_metrics_header = req.headers["X-Stripe-Client-Telemetry"]
false
end.to_return(body: JSON.generate(object: "charge"))

Stripe::Charge.list
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"].nil?)
end
end
end

class SystemProfilerTest < Test::Unit::TestCase
Expand Down

0 comments on commit 9bc61c6

Please sign in to comment.