From 3e4c63d5025cca570560ee66f47fc3eeb73cf014 Mon Sep 17 00:00:00 2001
From: Harry <harrybagdi@gmail.com>
Date: Fri, 14 Dec 2018 08:55:54 -0800
Subject: [PATCH] fix: skip sending status code after body is sent

This commit fixes a bug where the plugin tries to send HTTP status code
after the response body is sent for `/metrics` endpoint.

This resulted in error logs in Kong.
A test has been added to ensure that no error logs are generated when
`/metrics` is scraped by Prometheus.

Another test has been added to ensure that the response body only
contains either metrics or comments describing the metrics.
This test is added to catch regression for
https://github.com/Kong/kong/pull/4077, where Kong injected HTML
generated by the default Lapis handler.

Fixes #32
From #33
---
 kong/plugins/prometheus/exporter.lua |  1 -
 spec/02-access_spec.lua              | 33 +++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/kong/plugins/prometheus/exporter.lua b/kong/plugins/prometheus/exporter.lua
index 5373264f03ab..6ec92ed81f39 100644
--- a/kong/plugins/prometheus/exporter.lua
+++ b/kong/plugins/prometheus/exporter.lua
@@ -121,7 +121,6 @@ local function collect()
   end
 
   prometheus:collect()
-  return kong.response.exit(200)
 end
 
 
diff --git a/spec/02-access_spec.lua b/spec/02-access_spec.lua
index 2a11d022314e..1ba029c5d817 100644
--- a/spec/02-access_spec.lua
+++ b/spec/02-access_spec.lua
@@ -39,7 +39,7 @@ describe("Plugin: prometheus (access)", function()
       proxy_client:close()
     end
     if admin_client then
-      proxy_client:close()
+      admin_client:close()
     end
 
     helpers.stop_kong()
@@ -96,4 +96,35 @@ describe("Plugin: prometheus (access)", function()
       assert.not_match("[error]", line, nil, true)
     end
   end)
+
+  it("does not log error during a scrape", function()
+    -- cleanup logs
+    local test_error_log_path = helpers.test_conf.nginx_err_logs
+    os.execute(":> " .. test_error_log_path)
+
+    local res = assert(admin_client:send {
+      method  = "GET",
+      path    = "/metrics",
+    })
+    assert.res_status(200, res)
+
+    -- make sure no errors
+    local logs = pl_file.read(test_error_log_path)
+    for line in logs:gmatch("[^\r\n]+") do
+      assert.not_match("[error]", line, nil, true)
+    end
+  end)
+
+  it("scrape response has metrics and comments only", function()
+    local res = assert(admin_client:send {
+      method  = "GET",
+      path    = "/metrics",
+    })
+    local body = assert.res_status(200, res)
+
+    for line in body:gmatch("[^\r\n]+") do
+      assert.matches("^[#|kong]", line)
+    end
+
+  end)
 end)