Skip to content

Commit

Permalink
Merge pull request #10196 from rabbitmq/mergify/bp/v3.11.x/pr-10195
Browse files Browse the repository at this point in the history
Escape prometheus core metric label values (backport #9656) (backport #10195)
  • Loading branch information
michaelklishin committed Dec 21, 2023
2 parents dd588e3 + 7f55981 commit ac82d22
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 11 deletions.
4 changes: 2 additions & 2 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,8 @@ erlang_package.hex_package(
erlang_package.hex_package(
name = "prometheus",
build_file = "@rabbitmq-server//bazel:BUILD.prometheus",
sha256 = "2a99bb6dce85e238c7236fde6b0064f9834dc420ddbd962aac4ea2a3c3d59384",
version = "4.10.0",
sha256 = "719862351aabf4df7079b05dc085d2bbcbe3ac0ac3009e956671b1d5ab88247d",
version = "4.11.0",
)

erlang_package.hex_package(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
counter_metric/2,
untyped_metric/2]).

-import(prometheus_text_format, [escape_label_value/1]).

-include_lib("prometheus/include/prometheus.hrl").
-include_lib("rabbit_common/include/rabbit.hrl").

Expand Down Expand Up @@ -408,22 +410,24 @@ label(L) when is_binary(L) ->
L;
label(M) when is_map(M) ->
maps:fold(fun (K, V, Acc = <<>>) ->
<<Acc/binary, K/binary, "=\"", V/binary, "\"">>;
<<Acc/binary, K/binary, "=\"", (escape_label_value(V))/binary, "\"">>;
(K, V, Acc) ->
<<Acc/binary, ",", K/binary, "=\"", V/binary, "\"">>
<<Acc/binary, ",", K/binary, "=\"", (escape_label_value(V))/binary, "\"">>
end, <<>>, M);
label(#resource{virtual_host = VHost, kind = exchange, name = Name}) ->
<<"vhost=\"", VHost/binary, "\",exchange=\"", Name/binary, "\"">>;
<<"vhost=\"", (escape_label_value(VHost))/binary, "\",",
"exchange=\"", (escape_label_value(Name))/binary, "\"">>;
label(#resource{virtual_host = VHost, kind = queue, name = Name}) ->
<<"vhost=\"", VHost/binary, "\",queue=\"", Name/binary, "\"">>;
<<"vhost=\"", (escape_label_value(VHost))/binary, "\",",
"queue=\"", (escape_label_value(Name))/binary, "\"">>;
label({P, {#resource{virtual_host = QVHost, kind = queue, name = QName},
#resource{virtual_host = EVHost, kind = exchange, name = EName}}}) when is_pid(P) ->
%% channel_queue_exchange_metrics {channel_id, {queue_id, exchange_id}}
<<"channel=\"", (iolist_to_binary(pid_to_list(P)))/binary, "\",",
"queue_vhost=\"", QVHost/binary, "\",",
"queue=\"", QName/binary, "\",",
"exchange_vhost=\"", EVHost/binary, "\",",
"exchange=\"", EName/binary, "\""
"queue_vhost=\"", (escape_label_value(QVHost))/binary, "\",",
"queue=\"", (escape_label_value(QName))/binary, "\",",
"exchange_vhost=\"", (escape_label_value(EVHost))/binary, "\",",
"exchange=\"", (escape_label_value(EName))/binary, "\""
>>;
label({RemoteAddress, Username, Protocol}) when is_binary(RemoteAddress), is_binary(Username),
is_atom(Protocol) ->
Expand Down
66 changes: 66 additions & 0 deletions deps/rabbitmq_prometheus/test/rabbit_prometheus_http_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ all() ->
{group, per_object_endpoint_metrics},
{group, commercial},
{group, detailed_metrics},
{group, special_chars},
{group, authentication}
].

Expand Down Expand Up @@ -62,6 +63,7 @@ groups() ->
exchange_bindings_metric,
exchange_names_metric
]},
{special_chars, [], [core_metrics_special_chars]},
{authentication, [], [basic_auth]}
].

Expand Down Expand Up @@ -206,6 +208,39 @@ init_per_group(commercial, Config0) ->
Config1 = rabbit_ct_helpers:merge_app_env(Config0, ProductConfig),
init_per_group(commercial, Config1, []);

init_per_group(special_chars, Config0) ->
StatsEnv = {rabbit, [{collect_statistics, fine}, {collect_statistics_interval, 100}]},
Config1 = init_per_group(special_chars, rabbit_ct_helpers:merge_app_env(Config0, StatsEnv), []),

VHost = <<"vhost\"\n\\">>,
rabbit_ct_broker_helpers:add_vhost(Config1, 0, VHost, <<"guest">>),
rabbit_ct_broker_helpers:set_full_permissions(Config1, VHost),
VHostConn = rabbit_ct_client_helpers:open_unmanaged_connection(Config1, 0, VHost),
{ok, VHostCh} = amqp_connection:open_channel(VHostConn),

%% new line characters (\r and \n) are removed from queue and
%% exchange names during creation (unlike for vhosts)
QName = <<"queue\"\\">>,
#'queue.declare_ok'{} = amqp_channel:call(VHostCh,
#'queue.declare'{queue = QName,
durable = true
}),
Exchange = <<"exchange\"\\">>,
#'exchange.declare_ok'{} = amqp_channel:call(VHostCh, #'exchange.declare'{exchange = Exchange}),
#'queue.bind_ok'{} = amqp_channel:call(VHostCh, #'queue.bind'{queue = QName, exchange = Exchange, routing_key = QName}),

amqp_channel:call(VHostCh,
#'basic.publish'{exchange = Exchange, routing_key = QName},
#amqp_msg{payload = <<"msg">>}),

Config2 = [{vhost_name, VHost},
{queue_name, QName},
{exchange_name, Exchange},
{connection, VHostConn},
{channel, VHostCh}
|Config1],
init_per_group(special_chars, Config2, []);

init_per_group(authentication, Config) ->
Config1 = rabbit_ct_helpers:merge_app_env(
Config, {rabbitmq_prometheus, [{authentication, [{enabled, true}]}]}),
Expand Down Expand Up @@ -249,6 +284,11 @@ end_per_group(detailed_metrics, Config) ->
amqp_channel:close(VHost2Ch),
amqp_connection:close(?config(vhost2_conn, Config)),

%% Delete queues?
end_per_group_(Config);
end_per_group(special_chars, Config) ->
amqp_channel:close(?config(channel, Config)),
amqp_connection:close(?config(connection, Config)),
%% Delete queues?
end_per_group_(Config);
end_per_group(authentication, Config) ->
Expand Down Expand Up @@ -560,6 +600,32 @@ exchange_names_metric(Config) ->
}, Names),
ok.

core_metrics_special_chars(Config) ->
{_, Body1} = http_get_with_pal(Config, "/metrics/detailed?family=queue_coarse_metrics", [], 200),
?assertMatch(#{rabbitmq_detailed_queue_messages :=
#{#{vhost => "vhost\\\"\\n\\\\",
queue => "queue\\\"\\\\"} := [I]}}
when I == 0; I == 1,
parse_response(Body1)),

{_, Body2} = http_get_with_pal(Config, "/metrics/detailed?family=channel_exchange_metrics", [], 200),
#{rabbitmq_detailed_channel_messages_published_total := LabelValue2} = parse_response(Body2),
?assertMatch([{#{channel := _,
vhost := "vhost\\\"\\n\\\\",
exchange := "exchange\\\"\\\\"}, [I]}]
when I == 0; I == 1,
maps:to_list(LabelValue2)),

{_, Body3} = http_get_with_pal(Config, "/metrics/detailed?family=channel_queue_exchange_metrics", [], 200),
#{rabbitmq_detailed_queue_messages_published_total := LabelValue3} = parse_response(Body3),
?assertMatch([{#{channel := _,
queue_vhost := "vhost\\\"\\n\\\\",
queue := "queue\\\"\\\\",
exchange_vhost := "vhost\\\"\\n\\\\",
exchange := "exchange\\\"\\\\"}, [I]}]
when I == 0; I == 1,
maps:to_list(LabelValue3)),
ok.

basic_auth(Config) ->
http_get(Config, [{"accept-encoding", "deflate"}], 401),
Expand Down
2 changes: 1 addition & 1 deletion rabbitmq-components.mk
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ dep_cowboy = hex 2.8.0
dep_cowlib = hex 2.12.1
dep_credentials_obfuscation = hex 3.4.0
dep_looking_glass = git https://github.com/rabbitmq/looking_glass.git main
dep_prometheus = hex 4.10.0
dep_prometheus = hex 4.11.0
dep_ra = hex 2.6.3
dep_ranch = hex 2.1.0
dep_recon = hex 2.5.3
Expand Down

0 comments on commit ac82d22

Please sign in to comment.