feat(prometheus): expose controlplane connectivity state as a gauge #14020

aryan9600 · 2024-12-13T11:14:32Z

Summary

Add a new Prometheus gauge metric control_plane_connected. Similar to datastore_reachable gauge, 0 means the connection is not healthy; 1 means that the connection is healthy. We mark the connection as unhealthy under the following circumstances:

Failure while establihing a websocket connection
Failure while sending basic information to controlplane
Failure while sending ping to controlplane
Failure while receiving a packet from the websocket connection

This is helpful for users running a signficant number of gateways to be alerted about potential issues any gateway(s) may be facing while talking to the controlplane.

Checklist

The Pull Request has tests
A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #[issue number]

kong/plugins/prometheus/exporter.lua

Add a new Prometheus gauge metric `control_plane_reachable`. Similar to `datastore_reachable` gauge, 0 means the connection is not healthy; 1 means that the connection is healthy. We mark the connection as unhealthy under the following circumstances: * Failure while establihing a websocket connection * Failure while sending basic information to controlplane * Failure while sending ping to controlplane * Failure while receiving a packet from the websocket connection This is helpful for users running a signficant number of gateways to be alerted about potential issues any gateway(s) may be facing while talking to the controlplane. Signed-off-by: Sanskar Jaiswal <[email protected]>

…viour Signed-off-by: Sanskar Jaiswal <[email protected]>

Signed-off-by: Sanskar Jaiswal <[email protected]>

flrgh · 2025-01-11T00:15:56Z

kong/clustering/data_plane.lua

-    ngx_log(ngx_INFO, _log_prefix, "sent ping frame to control plane with hash: ", hash, log_suffix)
+  else
+    set_control_plane_connected(true)
+    if hash ~= prev_hash then


If we keep the code as is, I suggest moving the comment string on line 155 so that it continues to point to the correct context

Suggested change

if hash ~= prev_hash then

-- only log a ping if the hash changed

if hash ~= prev_hash then

flrgh · 2025-01-11T00:18:27Z

kong/clustering/data_plane.lua

+local function set_control_plane_connected(reachable)
+  local ok, err = ngx.shared.kong:safe_set("control_plane_connected", reachable)
+  if not ok then
+    ngx_log(ngx_ERR, _log_prefix, "failed to set controlplane_reachable key in shm to " .. reachable .. " :", err)


Please do not use string concatenation in ngx.log() (docs)

Suggested change

ngx_log(ngx_ERR, _log_prefix, "failed to set controlplane_reachable key in shm to " .. reachable .. " :", err)

ngx_log(ngx_ERR, _log_prefix, "failed to set controlplane_reachable key in shm to ", reachable, " :", err)

flrgh · 2025-01-11T00:36:05Z

kong/clustering/data_plane.lua

@@ -74,11 +74,20 @@ function _M.new(clustering)
 end


+local function set_control_plane_connected(reachable)
+  local ok, err = ngx.shared.kong:safe_set("control_plane_connected", reachable)


set_control_plane_connected() is called in a lot of places. There are a couple risks to consider in this implementation:

Since we can't reliably test all of the code paths that lead to a set_control_plane_connected(false) call, and it would be easy for a future maintainer to refactor/modify the code and forget to call set_control_plane_connected(false) in some crucial path.

An unhandled exception (easy to accidentally introduce these as well) could cause a call to set_control_plane_connected(false) could be missed.

Either of these could lead to a scenario where the gauge is set to 1 even though control plane connectivity is unhealthy.

I think we should consider using a TTL ("exptime" in the docs) for this value. This will have the effect of causing the metric to become 0 if set_control_plane_connected(true) has not been called within the last $TTL seconds, because the value has expired. The value of the TTL should probably be derived from PING_INTERVAL or PING_WAIT.

flrgh · 2025-01-11T00:37:33Z

kong/plugins/prometheus/exporter.lua

@@ -65,6 +66,14 @@ local function init()
                                          "0 is unreachable",
                                          nil,
                                          prometheus.LOCAL_STORAGE)
+  if role == "data_plane" then
+    metrics.cp_connected = prometheus:gauge("control_plane_connected",
+                                          "Kong connected to controlplane, " ..


Suggested change

"Kong connected to controlplane, " ..

"Kong connected to control plane, " ..

flrgh · 2025-01-11T00:43:34Z

spec/03-plugins/26-prometheus/04-status_api_spec.lua

+    local body = get_metrics()
+    assert.matches('kong_control_plane_connected 1', body, nil, true)
+
+    helpers.stop_kong("prom_cp")
+    local body  = get_metrics()
+    assert.matches('kong_control_plane_connected 0', body, nil, true)


To my eyes, these look like eventual conditions (i.e. get_metrics() may not return the expected value on the first call). You should retry them.

Suggested change

local body = get_metrics()

assert.matches('kong_control_plane_connected 1', body, nil, true)

helpers.stop_kong("prom_cp")

local body = get_metrics()

assert.matches('kong_control_plane_connected 0', body, nil, true)

assert.eventually(function()

local body = get_metrics()

assert.matches('kong_control_plane_connected 1', body, nil, true)

end).has_no_error("metric kong_control_plane_connected => 1")

helpers.stop_kong("prom_cp")

assert.eventually(function()

local body = get_metrics()

assert.matches('kong_control_plane_connected 0', body, nil, true)

end).has_no_error("metric kong_control_plane_connected => 0")

flrgh · 2025-01-11T00:46:45Z

spec/03-plugins/26-prometheus/04-status_api_spec.lua

+    local body = get_metrics()
+    assert.matches('kong_control_plane_connected 1', body, nil, true)
+
+    helpers.stop_kong("prom_cp")


note: this line of code may be skipped if the prior assertion fails, which could leak the prom_cp instance and potentially interfere with other running tests. You should always ensure that it is stopped by code within your teardown() function. This may or may not require tracking whether or not you reached the helpers.stop_kong("prom_cp") call in the test code--I'm not sure.

flrgh · 2025-01-11T00:52:32Z

spec/03-plugins/26-prometheus/04-status_api_spec.lua

+    helpers.stop_kong("prom_dp")
+  end)
+
+  it("exposes controlplane connectivity status", function ()


idea: instead of starting prom_cp in setup(), what if you defer startup until it("exposes control plane connectivity status"):

assert kong_control_plane_connected 0

start prom_cp

assert kong_control_plane_connected 1

stop prom_cp

assert kong_control_plane_connected 0

This would be a trivial change to make, but it gives us more test coverage.

flrgh · 2025-01-11T00:53:35Z

spec/03-plugins/26-prometheus/04-status_api_spec.lua

+    helpers.stop_kong("prom_dp")
+  end)
+
+  it("exposes controlplane connectivity status", function ()


Suggested change

it("exposes controlplane connectivity status", function ()

it("exposes control plane connectivity status", function ()

pull-request-size bot added the size/M label Dec 13, 2024

github-actions bot assigned aryan9600 Dec 13, 2024

github-actions bot added core/clustering plugins/prometheus cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Dec 13, 2024

aryan9600 force-pushed the cp-conn-prom-metric branch 3 times, most recently from c2d6278 to 697c1e6 Compare December 23, 2024 13:32

pull-request-size bot added size/L and removed size/M labels Dec 23, 2024

aryan9600 requested review from chronolaw and bungle December 23, 2024 13:32

aryan9600 marked this pull request as ready for review December 23, 2024 16:21

aryan9600 force-pushed the cp-conn-prom-metric branch from 697c1e6 to de4a868 Compare January 2, 2025 11:08

RobSerafini requested review from gszr and flrgh January 7, 2025 19:22

hbagdi reviewed Jan 7, 2025

View reviewed changes

kong/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved

aryan9600 added 3 commits January 10, 2025 15:54

tests(prometheus): add tests to assert controlplane connectivity beha…

7ef8547

…viour Signed-off-by: Sanskar Jaiswal <[email protected]>

chore: rename control_plane_reachable to control_plane_connected

ee922f2

Signed-off-by: Sanskar Jaiswal <[email protected]>

aryan9600 force-pushed the cp-conn-prom-metric branch from de4a868 to ee922f2 Compare January 10, 2025 10:24

flrgh reviewed Jan 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(prometheus): expose controlplane connectivity state as a gauge #14020

feat(prometheus): expose controlplane connectivity state as a gauge #14020

aryan9600 commented Dec 13, 2024 •

edited

Loading

flrgh Jan 11, 2025

flrgh Jan 11, 2025

flrgh Jan 11, 2025 •

edited

Loading

flrgh Jan 11, 2025

flrgh Jan 11, 2025

flrgh Jan 11, 2025

flrgh Jan 11, 2025 •

edited

Loading

flrgh Jan 11, 2025

	if hash ~= prev_hash then
	-- only log a ping if the hash changed
	if hash ~= prev_hash then

	ngx_log(ngx_ERR, _log_prefix, "failed to set controlplane_reachable key in shm to " .. reachable .. " :", err)
	ngx_log(ngx_ERR, _log_prefix, "failed to set controlplane_reachable key in shm to ", reachable, " :", err)

	"Kong connected to controlplane, " ..
	"Kong connected to control plane, " ..

	it("exposes controlplane connectivity status", function ()
	it("exposes control plane connectivity status", function ()

feat(prometheus): expose controlplane connectivity state as a gauge #14020

Are you sure you want to change the base?

feat(prometheus): expose controlplane connectivity state as a gauge #14020

Conversation

aryan9600 commented Dec 13, 2024 • edited Loading

Summary

Checklist

Issue reference

flrgh Jan 11, 2025

Choose a reason for hiding this comment

flrgh Jan 11, 2025

Choose a reason for hiding this comment

flrgh Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

flrgh Jan 11, 2025

Choose a reason for hiding this comment

flrgh Jan 11, 2025

Choose a reason for hiding this comment

flrgh Jan 11, 2025

Choose a reason for hiding this comment

flrgh Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

flrgh Jan 11, 2025

Choose a reason for hiding this comment

aryan9600 commented Dec 13, 2024 •

edited

Loading

flrgh Jan 11, 2025 •

edited

Loading

flrgh Jan 11, 2025 •

edited

Loading