perf(rate-limiting) increment only configured time limits

kong/plugins/rate-limiting/handler.lua

@@ -77,14 +77,16 @@ function RateLimitingHandler:access(conf)
   local fault_tolerant = conf.fault_tolerant
 
   -- Load current metric for configured period
-  local usage, stop, err = get_usage(conf, api_id, identifier, current_timestamp, {
+  local limits = {
     second = conf.second,
     minute = conf.minute,
     hour = conf.hour,
     day = conf.day,
     month = conf.month,
     year = conf.year
-  })
+  }
+
+  local usage, stop, err = get_usage(conf, api_id, identifier, current_timestamp, limits)
   if err then
     if fault_tolerant then
       ngx_log(ngx.ERR, "failed to get usage: ", tostring(err))
@@ -106,13 +108,13 @@ function RateLimitingHandler:access(conf)
     end
   end
 
-  local incr = function(premature, conf, api_id, identifier, current_timestamp, value)
+  local incr = function(premature, conf, limits, api_id, identifier, current_timestamp, value)
     if premature then return end
-    policies[policy].increment(conf, api_id, identifier, current_timestamp, value)
+    policies[policy].increment(conf, limits, api_id, identifier, current_timestamp, value)
   end
 
-  -- Increment metrics for all periods if the request goes through
-  local ok, err = ngx_timer_at(0, incr, conf, api_id, identifier, current_timestamp, 1)
+  -- Increment metrics for configured periods if the request goes through
+  local ok, err = ngx_timer_at(0, incr, conf, limits, api_id, identifier, current_timestamp, 1)
   if not ok then
     ngx_log(ngx.ERR, "failed to create timer: ", err)
   end

kong/plugins/rate-limiting/policies/cluster.lua

@@ -8,27 +8,29 @@ local ERR = ngx.ERR
 
 return {
   ["cassandra"] = {
-    increment = function(db, api_id, identifier, current_timestamp, value)
+    increment = function(db, limits, api_id, identifier, current_timestamp, value)
       local periods = timestamp.get_timestamps(current_timestamp)
 
       for period, period_date in pairs(periods) do
-        local res, err = db:query([[
-          UPDATE ratelimiting_metrics
-          SET value = value + ?
-          WHERE api_id = ? AND
-                identifier = ? AND
-                period_date = ? AND
-                period = ?
-        ]], {
-          db.cassandra.counter(value),
-          db.cassandra.uuid(api_id),
-          identifier,
-          db.cassandra.timestamp(period_date),
-          period,
-        })
-        if not res then
-          log(ERR, "[rate-limiting] cluster policy: could not increment ",
-                   "cassandra counter for period '", period, "': ", err)
+        if limits[period] then
+          local res, err = db:query([[
+            UPDATE ratelimiting_metrics
+            SET value = value + ?
+            WHERE api_id = ? AND
+                  identifier = ? AND
+                  period_date = ? AND
+                  period = ?
+          ]], {
+            db.cassandra.counter(value),
+            db.cassandra.uuid(api_id),
+            identifier,
+            db.cassandra.timestamp(period_date),
+            period,
+          })
+          if not res then
+            log(ERR, "[rate-limiting] cluster policy: could not increment ",
+                     "cassandra counter for period '", period, "': ", err)
+          end
         end
       end
 
@@ -56,15 +58,17 @@ return {
     end,
   },
   ["postgres"] = {
-    increment = function(db, api_id, identifier, current_timestamp, value)
+    increment = function(db, limits, api_id, identifier, current_timestamp, value)
       local buf = {}
       local periods = timestamp.get_timestamps(current_timestamp)
 
       for period, period_date in pairs(periods) do
-        buf[#buf+1] = fmt([[
-          SELECT increment_rate_limits('%s', '%s', '%s', to_timestamp('%s')
-          at time zone 'UTC', %d)
-        ]], api_id, identifier, period, period_date/1000, value)
+        if limits[period] then
+          buf[#buf+1] = fmt([[
+            SELECT increment_rate_limits('%s', '%s', '%s', to_timestamp('%s')
+            at time zone 'UTC', %d)
+          ]], api_id, identifier, period, period_date/1000, value)
+        end
       end
 
       local res, err = db:query(concat(buf, ";"))

kong/plugins/rate-limiting/policies/init.lua

@@ -23,16 +23,18 @@ local EXPIRATIONS = {
 
 return {
   ["local"] = {
-    increment = function(conf, api_id, identifier, current_timestamp, value)
+    increment = function(conf, limits, api_id, identifier, current_timestamp, value)
       local periods = timestamp.get_timestamps(current_timestamp)
       for period, period_date in pairs(periods) do
-        local cache_key = get_local_key(api_id, identifier, period_date, period)
-        cache.sh_add(cache_key, 0, EXPIRATIONS[period])
-
-        local _, err = cache.sh_incr(cache_key, value)
-        if err then
-          ngx_log("[rate-limiting] could not increment counter for period '"..period.."': "..tostring(err))
-          return nil, err
+        if limits[period] then
+          local cache_key = get_local_key(api_id, identifier, period_date, period)
+          cache.sh_add(cache_key, 0, EXPIRATIONS[period])
+
+          local _, err = cache.sh_incr(cache_key, value)
+          if err then
+            ngx_log("[rate-limiting] could not increment counter for period '"..period.."': "..tostring(err))
+            return nil, err
+          end
         end
       end
 
@@ -49,9 +51,9 @@ return {
     end
   },
   ["cluster"] = {
-    increment = function(conf, api_id, identifier, current_timestamp, value)
+    increment = function(conf, limits, api_id, identifier, current_timestamp, value)
       local db = singletons.dao.db
-      local ok, err = policy_cluster[db.name].increment(db, api_id, identifier,
+      local ok, err = policy_cluster[db.name].increment(db, limits, api_id, identifier,
                                                         current_timestamp, value)
       if not ok then
         ngx_log(ngx.ERR, "[rate-limiting] cluster policy: could not increment ",
@@ -70,7 +72,7 @@ return {
     end
   },
   ["redis"] = {
-    increment = function(conf, api_id, identifier, current_timestamp, value)
+    increment = function(conf, limits, api_id, identifier, current_timestamp, value)
       local red = redis:new()
       red:set_timeout(conf.redis_timeout)
       local ok, err = red:connect(conf.redis_host, conf.redis_port)
@@ -97,23 +99,25 @@ return {
 
       local periods = timestamp.get_timestamps(current_timestamp)
       for period, period_date in pairs(periods) do
-        local cache_key = get_local_key(api_id, identifier, period_date, period)
-        local exists, err = red:exists(cache_key)
-        if err then
-          ngx_log(ngx.ERR, "failed to query Redis: ", err)
-          return nil, err
-        end
-
-        red:init_pipeline((not exists or exists == 0) and 2 or 1)
-        red:incrby(cache_key, value)
-        if not exists or exists == 0 then
-          red:expire(cache_key, EXPIRATIONS[period])
-        end
-
-        local _, err = red:commit_pipeline()
-        if err then
-          ngx_log(ngx.ERR, "failed to commit pipeline in Redis: ", err)
-          return nil, err
+        if limits[period] then
+          local cache_key = get_local_key(api_id, identifier, period_date, period)
+          local exists, err = red:exists(cache_key)
+          if err then
+            ngx_log(ngx.ERR, "failed to query Redis: ", err)
+            return nil, err
+          end
+
+          red:init_pipeline((not exists or exists == 0) and 2 or 1)
+          red:incrby(cache_key, value)
+          if not exists or exists == 0 then
+            red:expire(cache_key, EXPIRATIONS[period])
+          end
+
+          local _, err = red:commit_pipeline()
+          if err then
+            ngx_log(ngx.ERR, "failed to commit pipeline in Redis: ", err)
+            return nil, err
+          end
         end
       end
 

spec/03-plugins/24-rate-limiting/02-policies_spec.lua

@@ -35,8 +35,17 @@ describe("Plugin: rate-limiting (policies)", function()
       local current_timestamp = 1424217600
       local periods = timestamp.get_timestamps(current_timestamp)
 
+      local limits = {
+        second = 100,
+        minute = 100,
+        hour = 100,
+        day = 100,
+        month = 100,
+        year = 100
+      }
+
       -- First increment
-      assert(cluster_policy.increment(nil, api_id, identifier, current_timestamp, 1))
+      assert(cluster_policy.increment(nil, limits, api_id, identifier, current_timestamp, 1))
 
       -- First select
       for period, period_date in pairs(periods) do
@@ -46,7 +55,7 @@ describe("Plugin: rate-limiting (policies)", function()
       end
 
       -- Second increment
-      assert(cluster_policy.increment(nil, api_id, identifier, current_timestamp, 1))
+      assert(cluster_policy.increment(nil, limits, api_id, identifier, current_timestamp, 1))
 
       -- Second select
       for period, period_date in pairs(periods) do
@@ -60,7 +69,7 @@ describe("Plugin: rate-limiting (policies)", function()
       periods = timestamp.get_timestamps(current_timestamp)
 
       -- Third increment
-      assert(cluster_policy.increment(nil, api_id, identifier, current_timestamp, 1))
+      assert(cluster_policy.increment(nil, limits, api_id, identifier, current_timestamp, 1))
 
       -- Third select with 1 second delay
       for period, period_date in pairs(periods) do

spec/03-plugins/25-response-rate-limiting/02-policies_spec.lua

@@ -1,6 +1,6 @@
 local uuid = require("kong.tools.utils").uuid
 local helpers = require "spec.helpers"
-local policies = require "kong.plugins.rate-limiting.policies"
+local policies = require "kong.plugins.response-ratelimiting.policies"
 local timestamp = require "kong.tools.timestamp"
 
 describe("Plugin: response-ratelimiting (policies)", function()