Merge pull request #9656 from cloudamqp/prometheus_escape_label

Escape prometheus core metric label values

Author: michaelklishin

MODULE.bazel

@@ -315,8 +315,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(

deps/rabbitmq_prometheus/src/collectors/prometheus_rabbitmq_core_metrics_collector.erl

@@ -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").
 
@@ -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) ->

deps/rabbitmq_prometheus/test/rabbit_prometheus_http_SUITE.erl

@@ -24,6 +24,7 @@ all() ->
         {group, per_object_endpoint_metrics},
         {group, commercial},
         {group, detailed_metrics},
+        {group, special_chars},
         {group, authentication}
     ].
 
@@ -62,6 +63,7 @@ groups() ->
                                      exchange_bindings_metric,
                                      exchange_names_metric
         ]},
+       {special_chars, [], [core_metrics_special_chars]},
        {authentication, [], [basic_auth]}
     ].
 
@@ -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}]}]}),
@@ -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) ->
@@ -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),

rabbitmq-components.mk

@@ -117,7 +117,7 @@ dep_credentials_obfuscation = hex 3.4.0
 dep_khepri = hex 0.10.0
 dep_khepri_mnesia_migration = hex 0.2.1
 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.7.0
 dep_ranch = hex 2.1.0
 dep_recon = hex 2.5.3