Merge pull request #1293 from stripe/richardm-usage

Measure usage of .save

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

README.md

@@ -291,10 +291,11 @@ 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 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:
 

lib/stripe/api_operations/request.rb

@@ -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 ||= {}
 
@@ -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
 

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

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,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,
@@ -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
@@ -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
@@ -1038,13 +1038,19 @@ class StripeRequestMetrics
       # Request duration in milliseconds
       attr_accessor :request_duration_ms
 
-      def initialize(request_id, request_duration_ms)
+      # list of names of tracked behaviors associated with this request
+      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 }
+        ret = { request_id: request_id, request_duration_ms: request_duration_ms }
+        ret[:usage] = usage if !usage.nil? && !usage.empty?
+        ret
       end
     end
   end

test/stripe/stripe_client_test.rb

@@ -1408,14 +1408,24 @@ 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"])
+
+        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