From 3f53e0172da9178cdb88a445a28e2c22a53d81f9 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 12 May 2025 19:01:09 -0400 Subject: [PATCH] Remove connection counts and limits from public API health checks Returning the connection limit and active count are not really necessary for these checks. Instead of returning them in the response to the health check we log a warning when the connection limit is exceeded. --- ...m_health_check_below_node_connection_limit.erl | 15 ++++++++------- ...gmt_wm_health_check_ready_to_serve_clients.erl | 15 +++++++++------ .../test/rabbit_mgmt_http_health_checks_SUITE.erl | 6 ------ 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_below_node_connection_limit.erl b/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_below_node_connection_limit.erl index df2cf1882c22..d0661a6cea38 100644 --- a/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_below_node_connection_limit.erl +++ b/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_below_node_connection_limit.erl @@ -11,6 +11,8 @@ -export([to_json/2, content_types_provided/2]). -export([variances/2]). +-include_lib("kernel/include/logger.hrl"). + -include("rabbit_mgmt.hrl"). -include_lib("rabbitmq_management_agent/include/rabbit_mgmt_records.hrl"). @@ -34,16 +36,15 @@ to_json(ReqData, Context) -> Limit = rabbit_misc:get_env(rabbit, connection_max, infinity), case ActiveConns < Limit of true -> - rabbit_mgmt_util:reply( - #{status => ok, - limit => Limit, - connections => ActiveConns}, ReqData, Context); + rabbit_mgmt_util:reply(#{status => ok}, ReqData, Context); false -> + ?LOG_WARNING( + "Node connection limit is reached. Active connections: ~w, " + "limit: ~w", + [ActiveConns, Limit]), Body = #{ status => failed, - reason => <<"node connection limit is reached">>, - limit => Limit, - connections => ActiveConns + reason => <<"node connection limit is reached">> }, {Response, ReqData1, Context1} = rabbit_mgmt_util:reply( Body, ReqData, Context), diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_ready_to_serve_clients.erl b/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_ready_to_serve_clients.erl index 762bb2d1e692..37c1c5711481 100644 --- a/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_ready_to_serve_clients.erl +++ b/deps/rabbitmq_management/src/rabbit_mgmt_wm_health_check_ready_to_serve_clients.erl @@ -16,6 +16,8 @@ -export([to_json/2, content_types_provided/2]). -export([variances/2]). +-include_lib("kernel/include/logger.hrl"). + -include("rabbit_mgmt.hrl"). -include_lib("rabbitmq_management_agent/include/rabbit_mgmt_records.hrl"). @@ -33,8 +35,8 @@ content_types_provided(ReqData, Context) -> to_json(ReqData, Context) -> case check() of - {ok, Body} -> - rabbit_mgmt_util:reply(Body, ReqData, Context); + ok -> + rabbit_mgmt_util:reply(#{status => ok}, ReqData, Context); {error, Body} -> {Response, ReqData1, Context1} = rabbit_mgmt_util:reply( Body, ReqData, Context), @@ -62,13 +64,14 @@ check() -> Limit = rabbit_misc:get_env(rabbit, connection_max, infinity), case ActiveConns < Limit of true -> - {ok, #{status => ok, - limit => Limit, - connections => ActiveConns}}; + ok; false -> + ?LOG_WARNING( + "Node connection limit is reached. Active " + "connections: ~w, limit: ~w", + [ActiveConns, Limit]), {error, #{status => failed, reason => <<"node connection limit is reached">>, - limit => Limit, connections => ActiveConns}} end; [] -> diff --git a/deps/rabbitmq_management/test/rabbit_mgmt_http_health_checks_SUITE.erl b/deps/rabbitmq_management/test/rabbit_mgmt_http_health_checks_SUITE.erl index b3304d3d9b99..384e09dfa98f 100644 --- a/deps/rabbitmq_management/test/rabbit_mgmt_http_health_checks_SUITE.erl +++ b/deps/rabbitmq_management/test/rabbit_mgmt_http_health_checks_SUITE.erl @@ -476,8 +476,6 @@ below_node_connection_limit_test(Config) -> Path = "/health/checks/below-node-connection-limit", Check0 = http_get(Config, Path, ?OK), ?assertEqual(<<"ok">>, maps:get(status, Check0)), - ?assertEqual(0, maps:get(connections, Check0)), - ?assertEqual(<<"infinity">>, maps:get(limit, Check0)), %% Set the connection limit low and open 'limit' connections. Limit = 10, @@ -489,8 +487,6 @@ below_node_connection_limit_test(Config) -> Body0 = http_get_failed(Config, Path), ?assertEqual(<<"failed">>, maps:get(<<"status">>, Body0)), - ?assertEqual(10, maps:get(<<"limit">>, Body0)), - ?assertEqual(10, maps:get(<<"connections">>, Body0)), %% Clean up the connections and reset the limit. [catch rabbit_ct_client_helpers:close_connection(C) || C <- Connections], @@ -519,8 +515,6 @@ ready_to_serve_clients_test(Config) -> Body1 = http_get_failed(Config, Path), ?assertEqual(<<"failed">>, maps:get(<<"status">>, Body1)), - ?assertEqual(10, maps:get(<<"limit">>, Body1)), - ?assertEqual(10, maps:get(<<"connections">>, Body1)), %% Clean up the connections and reset the limit. [catch rabbit_ct_client_helpers:close_connection(C) || C <- Connections],