Skip to content

Commit

Permalink
Escape prometheus core metric label values
Browse files Browse the repository at this point in the history
For example special characters like double quotes are allowed in queue
names, in which case detailed metrics could produce unparsable text
format output.
  • Loading branch information
gomoripeti committed Nov 9, 2023
1 parent 5b18e9d commit 709e46c
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 8 deletions.
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
57 changes: 57 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,37 @@ 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),

QName = <<"queue\"\n\\">>,
#'queue.declare_ok'{} = amqp_channel:call(VHostCh,
#'queue.declare'{queue = QName,
durable = true
}),
Exchange = <<"exchange\"\n\\">>,
#'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:cast(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 +282,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 +598,25 @@ 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 :=
#{#{queue => "queue\\\"\\\\",
vhost => "vhost\\\"\\n\\\\"} := [0]}}, parse_response(Body1)),

%% temp for debugging
[_|_] = rabbit_ct_broker_helpers:rpc(Config, 0, ets, tab2list, [channel_exchange_metrics]),
{_, Body2} = http_get_with_pal(Config, "/metrics/detailed?family=channel_exchange_metrics", [], 200),
?assertMatch(#{rabbitmq_channel_messages_published_total :=
#{#{exchange => "exchange\\\"\\\\",
vhost => "vhost\\\"\\n\\\\"} := [0]}}, parse_response(Body2)),
{_, Body3} = http_get_with_pal(Config, "/metrics/detailed?family=channel_queue_exchange_metrics", [], 200),
?assertMatch(#{rabbitmq_queue_messages_published_total :=
#{#{exchange => "exchange\\\"\\\\",
exchange_vhost => "vhost\\\"\\n\\\\",
queue => "queue\\\"\\\\",
queue_vhost => "vhost\\\"\\n\\\\"} := [0]}}, parse_response(Body3)),
ok.

basic_auth(Config) ->
http_get(Config, [{"accept-encoding", "deflate"}], 401),
Expand Down

0 comments on commit 709e46c

Please sign in to comment.