From d40ceddb6ea9e2077c60f9d3e21c1ab7be4a0a4d Mon Sep 17 00:00:00 2001
From: Michael Martin <flrgh@protonmail.com>
Date: Tue, 10 Dec 2024 17:18:55 -0800
Subject: [PATCH] fix(prometheus): use configure() handler to toggle
 upstream_health_metrics (#13983)

* fix(prometheus): use configure() handler to toggle upstream_health_metrics

This setting was added before we had the :configure() phase handler and
relied instead on the :log() handler to toggle itself. I suspect that
there is some buggy behavior in this approach, but it's hard to craft a
test case to reliably confirm. In any case, this commit updates the
plugin to use the :configure() handler to toggle the setting, as this is
now the idiomatic way of performing this task.

* tests(prometheus): swap setup/teardown for their lazy variants

* docs(prometheus): add changelog entry

* docs(prometheus): fix changelog entry

* Revert "tests(prometheus): swap setup/teardown for their lazy variants"

This reverts commit f97ab64295cd9313b942fca1748ed4e61bb1831a.
---
 .../prometheus-upstream-metrics-toggle.yml    |  3 ++
 kong/plugins/prometheus/exporter.lua          | 37 +++++++++----------
 kong/plugins/prometheus/handler.lua           |  6 ---
 3 files changed, 21 insertions(+), 25 deletions(-)
 create mode 100644 changelog/unreleased/kong/prometheus-upstream-metrics-toggle.yml

diff --git a/changelog/unreleased/kong/prometheus-upstream-metrics-toggle.yml b/changelog/unreleased/kong/prometheus-upstream-metrics-toggle.yml
new file mode 100644
index 000000000000..4c67209dd79c
--- /dev/null
+++ b/changelog/unreleased/kong/prometheus-upstream-metrics-toggle.yml
@@ -0,0 +1,3 @@
+message: "**Prometheus**: Use the :configure() handler to toggle upstream_health_metrics"
+type: bugfix
+scope: Plugin
diff --git a/kong/plugins/prometheus/exporter.lua b/kong/plugins/prometheus/exporter.lua
index 83e3d05a917f..4c37287da5ab 100644
--- a/kong/plugins/prometheus/exporter.lua
+++ b/kong/plugins/prometheus/exporter.lua
@@ -23,8 +23,9 @@ local role = kong.configuration.role
 local KONG_LATENCY_BUCKETS = { 1, 2, 5, 7, 10, 15, 20, 30, 50, 75, 100, 200, 500, 750, 1000, 3000, 6000 }
 local UPSTREAM_LATENCY_BUCKETS = { 25, 50, 80, 100, 250, 400, 700, 1000, 2000, 5000, 10000, 30000, 60000 }
 local AI_LLM_PROVIDER_LATENCY_BUCKETS = { 250, 500, 1000, 1500, 2000, 2500, 3000, 3500, 4000, 4500, 5000, 10000, 30000, 60000 }
-local IS_PROMETHEUS_ENABLED
 
+local IS_PROMETHEUS_ENABLED = false
+local export_upstream_health_metrics = false
 
 local metrics = {}
 -- prometheus.lua instance
@@ -206,7 +207,21 @@ end
 
 
 local function configure(configs)
-  IS_PROMETHEUS_ENABLED = configs ~= nil
+  -- everything disabled by default
+  IS_PROMETHEUS_ENABLED = false
+  export_upstream_health_metrics = false
+
+  if configs ~= nil then
+    IS_PROMETHEUS_ENABLED = true
+
+    for i = 1, #configs do
+      -- export upstream health metrics if any plugin has explicitly enabled them
+      if configs[i].upstream_health_metrics then
+        export_upstream_health_metrics = true
+        break
+      end
+    end
+  end
 end
 
 
@@ -402,16 +417,6 @@ local function log(message, serialized)
   end
 end
 
--- The upstream health metrics is turned on if at least one of
--- the plugin turns upstream_health_metrics on.
--- Due to the fact that during scrape time we don't want to
--- iterrate over all plugins to find out if upstream_health_metrics
--- is turned on or not, we will need a Kong reload if someone
--- turned on upstream_health_metrics on and off again, to actually
--- stop exporting upstream health metrics
-local should_export_upstream_health_metrics = false
-
-
 local function metric_data(write_fn)
   if not prometheus or not metrics then
     kong.log.err("prometheus: plugin is not initialized, please make sure ",
@@ -449,7 +454,7 @@ local function metric_data(write_fn)
   local phase = get_phase()
 
   -- only export upstream health metrics in traditional mode and data plane
-  if role ~= "control_plane" and should_export_upstream_health_metrics then
+  if role ~= "control_plane" and export_upstream_health_metrics then
     -- erase all target/upstream metrics, prevent exposing old metrics
     metrics.upstream_target_health:reset()
 
@@ -562,11 +567,6 @@ local function get_prometheus()
   return prometheus
 end
 
-local function set_export_upstream_health_metrics(set_or_not)
-  should_export_upstream_health_metrics = set_or_not
-end
-
-
 return {
   init        = init,
   init_worker = init_worker,
@@ -575,5 +575,4 @@ return {
   metric_data = metric_data,
   collect     = collect,
   get_prometheus = get_prometheus,
-  set_export_upstream_health_metrics = set_export_upstream_health_metrics,
 }
diff --git a/kong/plugins/prometheus/handler.lua b/kong/plugins/prometheus/handler.lua
index 3666b406f009..49078afceeac 100644
--- a/kong/plugins/prometheus/handler.lua
+++ b/kong/plugins/prometheus/handler.lua
@@ -58,12 +58,6 @@ function PrometheusHandler:log(conf)
     serialized.ai_metrics = message.ai
   end
 
-  if conf.upstream_health_metrics then
-    exporter.set_export_upstream_health_metrics(true)
-  else
-    exporter.set_export_upstream_health_metrics(false)
-  end
-
   exporter.log(message, serialized)
 end