Merge pull request #919 from 3scale/metrics-backend-calls-type

Metrics: add the type of 3scale backend call (auth, authrep, report)

CHANGELOG.md

@@ -10,11 +10,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Prometheus metrics for: the 3scale batching policy and the upstream API [PR #902](https://github.com/3scale/apicast/pull/902), [PR #918](https://github.com/3scale/apicast/pull/918)
 - Support for path in the upstream URL [PR #905](https://github.com/3scale/apicast/pull/905)
-- OIDC Authentication policy (only useable directly by the configuration file) [PR #904](https://github.com/3scale/apicast/pull/904)
+- OIDC Authentication policy (only usable directly by the configuration file) [PR #904](https://github.com/3scale/apicast/pull/904)
 
 ### Changed
 
-- Renamed the `backend_response` Prometheus metric to `threescale_backend_response` to avoid confusion with the upstream response [PR #917](https://github.com/3scale/apicast/pull/917)
+- The `threescale_backend_calls` Prometheus metric now includes the response (used to be in `backend_response`) and also the kind of call (auth, authrep, report)[PR #919](https://github.com/3scale/apicast/pull/919)
 
 ## [3.3.0-cr2] - 2018-09-25
 

gateway/src/apicast/backend_client.lua

@@ -20,7 +20,7 @@ local http_ng = require('resty.http_ng')
 local user_agent = require('apicast.user_agent')
 local resty_url = require('resty.url')
 local resty_env = require('resty.env')
-local threescale_backend_status_counters = require('apicast.metrics.3scale_backend_status')
+local backend_calls_metrics = require('apicast.metrics.3scale_backend_calls')
 
 local http_proxy = require('resty.http.proxy')
 local http_ng_ngx = require('resty.http_ng.backend.ngx')
@@ -98,8 +98,8 @@ function _M:new(service, http_client)
   }, mt)
 end
 
-local function inc_backend_status_metric(status)
-  threescale_backend_status_counters.inc(status)
+local function inc_metrics(endpoint, status)
+  backend_calls_metrics.report(endpoint, status)
 end
 
 local function build_args(args)
@@ -138,8 +138,6 @@ local function call_backend_transaction(self, path, options, ...)
 
   ngx.log(ngx.INFO, 'backend client uri: ', url, ' ok: ', res.ok, ' status: ', res.status, ' body: ', res.body, ' error: ', res.error)
 
-  inc_backend_status_metric(res.status)
-
   return res
 end
 
@@ -213,7 +211,11 @@ function _M:authrep(...)
 
   local using_oauth = self.version == 'oauth'
   local auth_uri = authrep_path(using_oauth)
-  return call_backend_transaction(self, auth_uri, authorize_options(using_oauth), ...)
+  local res = call_backend_transaction(self, auth_uri, authorize_options(using_oauth), ...)
+
+  inc_metrics('authrep', res.status)
+
+  return res
 end
 
 --- Call authorize (oauth_authorize) on backend.
@@ -226,7 +228,11 @@ function _M:authorize(...)
 
   local using_oauth = self.version == 'oauth'
   local auth_uri = auth_path(using_oauth)
-  return call_backend_transaction(self, auth_uri, authorize_options(using_oauth), ...)
+  local res = call_backend_transaction(self, auth_uri, authorize_options(using_oauth), ...)
+
+  inc_metrics('auth', res.status)
+
+  return res
 end
 
 function _M:report(reports_batch)
@@ -236,7 +242,7 @@ function _M:report(reports_batch)
   local report_body = format_transactions(reports_batch)
   local res = http_client.post(report_uri, report_body)
 
-  inc_backend_status_metric(res.status)
+  inc_metrics('report', res.status)
 
   return res
 end

gateway/src/apicast/metrics/3scale_backend_calls.lua

@@ -0,0 +1,28 @@
+local prometheus = require('apicast.prometheus')
+
+local format = string.format
+
+local _M = {}
+
+local threescale_backend_call = prometheus(
+  'counter',
+  'threescale_backend_calls',
+  "Calls to the 3scale backend",
+  { 'endpoint', 'status' }
+)
+
+local function label_for_status(status)
+  if not status or status == '' or status == 0 then
+    return 'invalid_status'
+  else
+    return format("%dxx", status/100)
+  end
+end
+
+function _M.report(endpoint, status)
+  if threescale_backend_call then
+    threescale_backend_call:inc(1, { endpoint, label_for_status(status) })
+  end
+end
+
+return _M

spec/backend_client_spec.lua

@@ -1,15 +1,19 @@
 local _M = require('apicast.backend_client')
 local configuration = require('apicast.configuration')
-local test_backend_client = require 'resty.http_ng.backend.test'
+local http_ng = require 'resty.http_ng'
 local ReportsBatch = require 'apicast.policy.3scale_batcher.reports_batch'
+local backend_calls_metrics = require 'apicast.metrics.3scale_backend_calls'
 
 describe('backend client', function()
 
   local test_backend
   local options_header_oauth = 'rejection_reason_header=1'
   local options_header_no_oauth = 'rejection_reason_header=1&no_body=1'
 
-  before_each(function() test_backend = test_backend_client.new() end)
+  before_each(function()
+    test_backend = http_ng.backend()
+    stub(backend_calls_metrics, 'report')
+  end)
 
   describe('authrep', function()
     it('works without params', function()
@@ -90,6 +94,17 @@ describe('backend client', function()
 
       assert.equal(200, res.status)
     end)
+
+    it('reports the call with the status', function()
+      local service = configuration.parse_service({ id = '42' })
+      local status = 200
+      test_backend.expect({}).respond_with({ status = status })
+      local backend_client = assert(_M:new(service, test_backend))
+
+      backend_client:authrep()
+
+      assert.stub(backend_calls_metrics.report).was_called_with('authrep', status)
+    end)
   end)
 
   describe('authorize', function()
@@ -172,103 +187,125 @@ describe('backend client', function()
       assert.equal(200, res.status)
     end)
 
-    describe('report', function()
-      describe('when the service is configured to use app IDs', function()
-        it('makes the call to backend with the right params', function()
-          local service = configuration.parse_service({
-            id = '42',
-            backend_version = '2',
-            proxy = { backend = { endpoint = 'http://example.com' } },
-            backend_authentication_type = 'auth', backend_authentication_value = 'val'
-          })
-
-          -- It's tricky to test with several reports because they can go in
-          -- any order in the request.
-          local reports = { { app_id = 'id1', metric = 'm1', value = 1 } }
-
-          local transactions = {}
-          transactions["transactions[0][app_id]"] = 'id1'
-          transactions["transactions[0][usage][m1]"] = 1
-
-          local reports_batch = ReportsBatch.new(service.id, reports)
-
-          test_backend.expect{
-            url = 'http://example.com/transactions.xml?' ..
-                ngx.encode_args({ auth = service.backend_authentication.value,
-                                  service_id = service.id }),
-            body = ngx.encode_args(transactions)
-          }.respond_with{ status = 200 }
-
-          local backend_client = assert(_M:new(service, test_backend))
-          local res = backend_client:report(reports_batch)
-          assert.equal(200, res.status)
-        end)
+    it('reports the call with the status', function()
+      local service = configuration.parse_service({ id = '42' })
+      local status = 200
+      test_backend.expect({}).respond_with({ status = status })
+      local backend_client = assert(_M:new(service, test_backend))
+
+      backend_client:authorize()
+
+      assert.stub(backend_calls_metrics.report).was_called_with('auth', status)
+    end)
+  end)
+
+  describe('report', function()
+    describe('when the service is configured to use app IDs', function()
+      it('makes the call to backend with the right params', function()
+        local service = configuration.parse_service({
+          id = '42',
+          backend_version = '2',
+          proxy = { backend = { endpoint = 'http://example.com' } },
+          backend_authentication_type = 'auth', backend_authentication_value = 'val'
+        })
+
+        -- It's tricky to test with several reports because they can go in
+        -- any order in the request.
+        local reports = { { app_id = 'id1', metric = 'm1', value = 1 } }
+
+        local transactions = {}
+        transactions["transactions[0][app_id]"] = 'id1'
+        transactions["transactions[0][usage][m1]"] = 1
+
+        local reports_batch = ReportsBatch.new(service.id, reports)
+
+        test_backend.expect{
+          url = 'http://example.com/transactions.xml?' ..
+              ngx.encode_args({ auth = service.backend_authentication.value,
+                                service_id = service.id }),
+          body = ngx.encode_args(transactions)
+        }.respond_with{ status = 200 }
+
+        local backend_client = assert(_M:new(service, test_backend))
+        local res = backend_client:report(reports_batch)
+        assert.equal(200, res.status)
       end)
+    end)
 
-      describe('when the service is configured to use user keys', function()
-        it('makes the call to backend with the right params', function()
-          local service = configuration.parse_service({
-            id = '42',
-            backend_version = '1',
-            proxy = { backend = { endpoint = 'http://example.com' } },
-            backend_authentication_type = 'auth', backend_authentication_value = 'val'
-          })
-
-          -- It's tricky to test with several reports because they can go in
-          -- any order in the request.
-          local reports = { { user_key = 'uk1', metric = 'm1', value = 1 } }
-
-          local transactions = {}
-          transactions["transactions[0][user_key]"] = 'uk1'
-          transactions["transactions[0][usage][m1]"] = 1
-
-          local reports_batch = ReportsBatch.new(service.id, reports)
-
-          test_backend.expect{
-            url = 'http://example.com/transactions.xml?' ..
-                ngx.encode_args({ auth = service.backend_authentication.value,
-                                  service_id = service.id }),
-            body = ngx.encode_args(transactions)
-          }.respond_with{ status = 200 }
-
-          local backend_client = assert(_M:new(service, test_backend))
-          local res = backend_client:report(reports_batch)
-          assert.equal(200, res.status)
-        end)
+    describe('when the service is configured to use user keys', function()
+      it('makes the call to backend with the right params', function()
+        local service = configuration.parse_service({
+          id = '42',
+          backend_version = '1',
+          proxy = { backend = { endpoint = 'http://example.com' } },
+          backend_authentication_type = 'auth', backend_authentication_value = 'val'
+        })
+
+        -- It's tricky to test with several reports because they can go in
+        -- any order in the request.
+        local reports = { { user_key = 'uk1', metric = 'm1', value = 1 } }
+
+        local transactions = {}
+        transactions["transactions[0][user_key]"] = 'uk1'
+        transactions["transactions[0][usage][m1]"] = 1
+
+        local reports_batch = ReportsBatch.new(service.id, reports)
+
+        test_backend.expect{
+          url = 'http://example.com/transactions.xml?' ..
+              ngx.encode_args({ auth = service.backend_authentication.value,
+                                service_id = service.id }),
+          body = ngx.encode_args(transactions)
+        }.respond_with{ status = 200 }
+
+        local backend_client = assert(_M:new(service, test_backend))
+        local res = backend_client:report(reports_batch)
+        assert.equal(200, res.status)
       end)
+    end)
 
-      describe('when the service is configured to use oauth tokens', function()
-        it('makes the call to backend with the right params', function()
-          local service = configuration.parse_service({
-            id = '42',
-            backend_version = 'oauth',
-            proxy = { backend = { endpoint = 'http://example.com' } },
-            backend_authentication_type = 'auth', backend_authentication_value = 'val'
-          })
-
-          -- It's tricky to test with several reports because they can go in
-          -- any order in the request.
-          local reports = { { access_token = 'token', metric = 'm1', value = 1 } }
-
-          local transactions = {}
-          transactions["transactions[0][access_token]"] = 'token'
-          transactions["transactions[0][usage][m1]"] = 1
-
-          local reports_batch = ReportsBatch.new(service.id, reports)
-
-          test_backend.expect{
-            url = 'http://example.com/transactions.xml?' ..
-                ngx.encode_args({ auth = service.backend_authentication.value,
-                                  service_id = service.id }),
-            body = ngx.encode_args(transactions)
-          }.respond_with{ status = 200 }
-
-          local backend_client = assert(_M:new(service, test_backend))
-          local res = backend_client:report(reports_batch)
-          assert.equal(200, res.status)
-        end)
+    describe('when the service is configured to use oauth tokens', function()
+      it('makes the call to backend with the right params', function()
+        local service = configuration.parse_service({
+          id = '42',
+          backend_version = 'oauth',
+          proxy = { backend = { endpoint = 'http://example.com' } },
+          backend_authentication_type = 'auth', backend_authentication_value = 'val'
+        })
+
+        -- It's tricky to test with several reports because they can go in
+        -- any order in the request.
+        local reports = { { access_token = 'token', metric = 'm1', value = 1 } }
+
+        local transactions = {}
+        transactions["transactions[0][access_token]"] = 'token'
+        transactions["transactions[0][usage][m1]"] = 1
+
+        local reports_batch = ReportsBatch.new(service.id, reports)
+
+        test_backend.expect{
+          url = 'http://example.com/transactions.xml?' ..
+              ngx.encode_args({ auth = service.backend_authentication.value,
+                                service_id = service.id }),
+          body = ngx.encode_args(transactions)
+        }.respond_with{ status = 200 }
+
+        local backend_client = assert(_M:new(service, test_backend))
+        local res = backend_client:report(reports_batch)
+        assert.equal(200, res.status)
       end)
     end)
+
+    it('reports the call with the status', function()
+      local service = configuration.parse_service({ id = '42' })
+      local status = 200
+      test_backend.expect({}).respond_with({ status = status })
+      local backend_client = assert(_M:new(service, test_backend))
+
+      backend_client:report(ReportsBatch.new(service.id, {}))
+
+      assert.stub(backend_calls_metrics.report).was_called_with('report', status)
+    end)
   end)
 
   describe('store_oauth_token', function()