Adding notifications support for various lifecycle conditions

Author: cryptomail

Gemfile.lock

@@ -2,6 +2,7 @@ PATH
   remote: .
   specs:
     zendesk_api (3.1.1)
+      activesupport
       base64
       faraday (> 2.0.0)
       faraday-multipart

lib/zendesk_api/client.rb

@@ -19,6 +19,7 @@
 require_relative "middleware/response/parse_json"
 require_relative "middleware/response/raise_error"
 require_relative "middleware/response/logger"
+require_relative "middleware/response/zendesk_request_event"
 require_relative "delegator"
 
 module ZendeskAPI
@@ -166,6 +167,7 @@ def build_connection
       Faraday.new(config.options) do |builder|
         # response
         builder.use ZendeskAPI::Middleware::Response::RaiseError
+        builder.use ZendeskAPI::Middleware::Response::ZendeskRequestEvent, self if config.instrumentation.respond_to?(:instrument)
         builder.use ZendeskAPI::Middleware::Response::Callback, self
         builder.use ZendeskAPI::Middleware::Response::Logger, config.logger if config.logger
         builder.use ZendeskAPI::Middleware::Response::ParseIsoDates
@@ -181,7 +183,7 @@ def build_connection
         set_authentication(builder, config)
 
         if config.cache
-          builder.use ZendeskAPI::Middleware::Request::EtagCache, cache: config.cache
+          builder.use ZendeskAPI::Middleware::Request::EtagCache, {cache: config.cache, instrumentation: config.instrumentation}
         end
 
         builder.use ZendeskAPI::Middleware::Request::Upload
@@ -193,7 +195,8 @@ def build_connection
           builder.use ZendeskAPI::Middleware::Request::Retry,
             logger: config.logger,
             retry_codes: config.retry_codes,
-            retry_on_exception: config.retry_on_exception
+            retry_on_exception: config.retry_on_exception,
+            instrumentation: config.instrumentation
         end
         if config.raise_error_when_rate_limited
           builder.use ZendeskAPI::Middleware::Request::RaiseRateLimited, logger: config.logger

lib/zendesk_api/configuration.rb

@@ -54,6 +54,9 @@ class Configuration
     # specify if you want a (network layer) exception to elicit a retry
     attr_accessor :retry_on_exception
 
+    # specify if you wnat instrumentation to be used
+    attr_accessor :instrumentation
+
     def initialize
       @client_options = {}
       @use_resource_cache = true

lib/zendesk_api/middleware/request/etag_cache.rb

@@ -1,4 +1,5 @@
 require "faraday/middleware"
+require "active_support/notifications"
 
 module ZendeskAPI
   module Middleware
@@ -9,6 +10,7 @@ module Request
       class EtagCache < Faraday::Middleware
         def initialize(app, options = {})
           @app = app
+          @instrumentation = options[:instrumentation] if options[:instrumentation].respond_to?(:instrument)
           @cache = options[:cache] ||
             raise("need :cache option e.g. ActiveSupport::Cache::MemoryStore.new")
           @cache_key_prefix = options.fetch(:cache_key_prefix, :faraday_etags)
@@ -41,8 +43,18 @@ def call(environment)
                 content_length: cached[:response_headers][:content_length],
                 content_encoding: cached[:response_headers][:content_encoding]
               )
+              @instrumentation&.instrument("zendesk.cache_hit",
+                {
+                  endpoint: env[:url].path,
+                  status: env[:status]
+                })
             elsif env[:status] == 200 && env[:response_headers]["Etag"] # modified and cacheable
               @cache.write(cache_key(env), env.to_hash)
+              @instrumentation&.instrument("zendesk.cache_miss",
+                {
+                  endpoint: env[:url].path,
+                  status: env[:status]
+                })
             end
           end
         end

lib/zendesk_api/middleware/request/retry.rb

@@ -15,44 +15,74 @@ def initialize(app, options = {})
           @logger = options[:logger]
           @error_codes = (options.key?(:retry_codes) && options[:retry_codes]) ? options[:retry_codes] : DEFAULT_ERROR_CODES
           @retry_on_exception = (options.key?(:retry_on_exception) && options[:retry_on_exception]) ? options[:retry_on_exception] : false
+          @instrumentation = options[:instrumentation]
         end
 
         def call(env)
+          # Duplicate env for retries but keep attempt counter persistent
           original_env = env.dup
+          original_env[:call_attempt] ||= 0
+
           exception_happened = false
+          response = nil
+
           if @retry_on_exception
             begin
               response = @app.call(env)
-            rescue => e
+            rescue => ex
               exception_happened = true
+              exception = ex
             end
           else
+            # Allow exceptions to propagate normally when not retrying
             response = @app.call(env)
           end
 
-          if exception_happened || @error_codes.include?(response.env[:status])
+          if exception_happened
+            original_env[:call_attempt] += 1
+            seconds_left = DEFAULT_RETRY_AFTER.to_i
+            @logger&.warn "An exception happened, waiting #{seconds_left} seconds... #{exception}"
+            instrument_retry(original_env, "exception", seconds_left)
+            sleep_with_logging(seconds_left)
+            return @app.call(original_env)
+          end
 
-            if exception_happened
-              seconds_left = DEFAULT_RETRY_AFTER.to_i
-              @logger&.warn "An exception happened, waiting #{seconds_left} seconds... #{e}"
-            else
-              seconds_left = (response.env[:response_headers][:retry_after] || DEFAULT_RETRY_AFTER).to_i
-            end
+          # Loop through consecutive retryable responses (e.g., multiple 429s)
+          while response && @error_codes.include?(response.env[:status])
+            original_env[:call_attempt] += 1
+            seconds_left = (response.env[:response_headers][:retry_after] || DEFAULT_RETRY_AFTER).to_i
+            @logger&.warn "You may have been rate limited. Retrying in #{seconds_left} seconds..."
+            instrument_retry(original_env, (response.env[:status] == 429) ? "rate_limited" : "server_error", seconds_left)
+            sleep_with_logging(seconds_left)
+            response = @app.call(original_env)
+          end
 
-            @logger&.warn "You have been rate limited. Retrying in #{seconds_left} seconds..."
+          response
+        end
 
-            seconds_left.times do |i|
-              sleep 1
-              time_left = seconds_left - i
-              @logger&.warn "#{time_left}..." if time_left > 0 && time_left % 5 == 0
-            end
+        private
 
-            @logger&.warn ""
+        def instrument_retry(env, reason, delay)
+          return unless @instrumentation
+          @instrumentation.instrument(
+            "zendesk.retry",
+            {
+              attempt: env[:call_attempt],
+              endpoint: env[:url].path,
+              method: env[:method],
+              reason: reason,
+              delay: delay
+            }
+          )
+        end
 
-            @app.call(original_env)
-          else
-            response
+        def sleep_with_logging(seconds_left)
+          seconds_left.times do |i|
+            sleep 1 if seconds_left > 0
+            time_left = seconds_left - i
+            @logger&.warn "#{time_left}..." if time_left > 0 && time_left % 5 == 0
           end
+          @logger&.warn "" if seconds_left > 0
         end
       end
     end

lib/zendesk_api/middleware/response/zendesk_request_event.rb

@@ -0,0 +1,41 @@
+require "faraday/response"
+
+module ZendeskAPI
+  module Middleware
+    module Response
+      # @private
+      class ZendeskRequestEvent < Faraday::Middleware
+        def initialize(app, client)
+          super(app)
+          @client = client
+        end
+
+        def call(env)
+          start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+          @app.call(env).on_complete do |response_env|
+            end_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+            duration = (end_time - start_time) * 1000.0
+            instrumentation = @client.config.instrumentation
+            if instrumentation
+              instrumentation.instrument("zendesk.request",
+                {duration: duration,
+                 endpoint: response_env[:url].path,
+                 method: response_env[:method],
+                 status: response_env[:status]})
+              if response_env[:status] < 500
+                instrumentation.instrument("zendesk.rate_limit",
+                  {
+                    endpoint: response_env[:url].path,
+                    status: response_env[:status],
+                    threshold: response_env[:response_headers] ? response_env[:response_headers]["X-Rate-Limit-Remaining"] : nil,
+                    limit: response_env[:response_headers] ? response_env[:response_headers]["X-Rate-Limit"] : nil,
+                    reset: response_env[:response_headers] ? response_env[:response_headers]["X-Rate-Limit-Reset"] : nil
+                  })
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/core/middleware/request/etag_cache_spec.rb

@@ -1,3 +1,6 @@
+require "core/spec_helper"
+require "active_support/cache"
+
 describe ZendeskAPI::Middleware::Request::EtagCache do
   it "caches" do
     client.config.cache.size = 1
@@ -16,4 +19,62 @@
       expect(response.headers[header]).to eq(first_response.headers[header])
     end
   end
+
+  context "instrumentation" do
+    let(:instrumentation) { double("Instrumentation") }
+    let(:cache) { ActiveSupport::Cache::MemoryStore.new }
+    let(:status) { nil }
+    let(:middleware) do
+      ZendeskAPI::Middleware::Request::EtagCache.new(
+        ->(env) { Faraday::Response.new(env) },
+        cache: cache,
+        instrumentation: instrumentation
+      )
+    end
+    let(:env) do
+      {
+        url: URI("https://example.zendesk.com/api/v2/blergh"),
+        method: :get,
+        request_headers: {},
+        response_headers: {"Etag" => "x", :x_rate_limit_remaining => 10},
+        status: status,
+        body: {"x" => 1},
+        response_body: {"x" => 1}
+      }
+    end
+    let(:no_instrumentation_middleware) do
+      ZendeskAPI::Middleware::Request::EtagCache.new(
+        ->(env) { Faraday::Response.new(env) },
+        cache: cache,
+        instrumentation: nil
+      )
+    end
+    before do
+      allow(instrumentation).to receive(:instrument)
+    end
+
+    it "emits cache_miss on first request" do
+      expect(instrumentation).to receive(:instrument).with(
+        "zendesk.cache_miss",
+        hash_including(endpoint: "/api/v2/blergh", status: 200)
+      )
+      env[:status] = 200
+      middleware.call(env).on_complete { |_e| 1 }
+    end
+
+    it "don't care on no instrumentation" do
+      env[:status] = 200
+      no_instrumentation_middleware.call(env).on_complete { |_e| 1 }
+    end
+
+    it "emits cache_hit on 304 response" do
+      cache.write(middleware.cache_key(env), env)
+      expect(instrumentation).to receive(:instrument).with(
+        "zendesk.cache_hit",
+        hash_including(endpoint: "/api/v2/blergh", status: 304)
+      )
+      env[:status] = 304
+      middleware.call(env).on_complete { |_e| 1 }
+    end
+  end
 end

spec/core/middleware/request/retry_spec.rb

@@ -116,4 +116,76 @@ def runtime
       end
     end
   end
+
+  context "with instrumentation on retry" do
+    let(:instrumentation) { double("Instrumentation") }
+    let(:middleware) do
+      ZendeskAPI::Middleware::Request::Retry.new(client.connection.builder.app)
+    end
+
+    before do
+      allow(instrumentation).to receive(:instrument)
+      client.config.instrumentation = instrumentation
+      # Inject instrumentation into middleware instance
+      allow_any_instance_of(ZendeskAPI::Middleware::Request::Retry).to receive(:instrumentation).and_return(instrumentation)
+      stub_request(:get, %r{instrumented}).to_return(status: 429, headers: {retry_after: 1}).to_return(status: 200)
+    end
+
+    it "calls instrumentation on retry" do
+      expect(instrumentation).to receive(:instrument).with(
+        "zendesk.retry",
+        hash_including(attempt: 1, endpoint: anything, method: anything, reason: anything, delay: anything)
+      ).at_least(:once)
+      client.connection.get("instrumented")
+    end
+
+    it "calls instrumentation on second retry attempt" do
+      # Override stub for this test to force two rate limit responses before success
+      stub_request(:get, %r{instrumented_twice})
+        .to_return(status: 429, headers: {retry_after: 0})
+        .to_return(status: 429, headers: {retry_after: 0})
+        .to_return(status: 200)
+
+      # Expect instrumentation for attempt 1 and attempt 2
+      expect(instrumentation).to receive(:instrument).with(
+        "zendesk.retry",
+        hash_including(attempt: 1, endpoint: anything, method: anything, reason: "rate_limited", delay: anything)
+      ).at_least(:once)
+      expect(instrumentation).to receive(:instrument).with(
+        "zendesk.retry",
+        hash_including(attempt: 2, endpoint: anything, method: anything, reason: "rate_limited", delay: anything)
+      ).at_least(:once)
+
+      client.connection.get("instrumented_twice")
+    end
+
+    it "calls instrumentation  with first attempt server error, and rate limited on second retry attempt" do
+      # Override stub for this test to force two rate limit responses before success
+      stub_request(:get, %r{instrumented_twice})
+        .to_return(status: 503, headers: {retry_after: 0})
+        .to_return(status: 429, headers: {retry_after: 0})
+        .to_return(status: 200)
+
+      # Expect instrumentation for attempt 1 and attempt 2
+      expect(instrumentation).to receive(:instrument).with(
+        "zendesk.retry",
+        hash_including(attempt: 1, endpoint: anything, method: anything, reason: "server_error", delay: anything)
+      ).at_least(:once)
+      expect(instrumentation).to receive(:instrument).with(
+        "zendesk.retry",
+        hash_including(attempt: 2, endpoint: anything, method: anything, reason: "rate_limited", delay: anything)
+      ).at_least(:once)
+
+      client.connection.get("instrumented_twice")
+    end
+
+    it "does not call instrumentation when no retry occurs" do
+      stub_request(:get, %r{no_retry}).to_return(status: 200)
+      expect(instrumentation).not_to receive(:instrument).with(
+        "zendesk.retry",
+        hash_including(:attempt, :endpoint, :method, :reason, :delay)
+      )
+      client.connection.get("no_retry")
+    end
+  end
 end