Merge pull request #12444 from rabbitmq/improve-feature-flags-subsyst…

…em-logging rabbit_feature_flags: Improve logging
rabbitmq · Oct 4, 2024 · 9645c81 · 9645c81
2 parents 5bc0bcd + d8ae8af
commit 9645c81
Show file tree

Hide file tree

Showing 3 changed files with 120 additions and 43 deletions.
diff --git a/deps/rabbit/src/rabbit_feature_flags.erl b/deps/rabbit/src/rabbit_feature_flags.erl
@@ -744,7 +744,7 @@ get_stability(FeatureName) when is_atom(FeatureName) ->
         undefined    -> undefined;
         FeatureProps -> get_stability(FeatureProps)
     end;
-get_stability(FeatureProps) when  ?IS_FEATURE_FLAG(FeatureProps) ->
+get_stability(FeatureProps) when ?IS_FEATURE_FLAG(FeatureProps) ->
     maps:get(stability, FeatureProps, stable);
 get_stability(FeatureProps) when ?IS_DEPRECATION(FeatureProps) ->
     Phase = rabbit_deprecated_features:get_phase(FeatureProps),

diff --git a/deps/rabbit/src/rabbit_ff_controller.erl b/deps/rabbit/src/rabbit_ff_controller.erl
@@ -440,17 +440,10 @@ check_node_compatibility_task1(NodeA, NodesA, NodeB, NodesB, NodeAAsVirigin)
         {ok, InventoryA0} ->
             InventoryA = virtually_reset_inventory(
                            InventoryA0, NodeAAsVirigin),
-            ?LOG_DEBUG(
-               "Feature flags: inventory of node `~ts`:~n~tp",
-               [NodeA, InventoryA],
-               #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
+            log_inventory(NodeA, InventoryA),
             case collect_inventory_on_nodes(NodesB) of
                 {ok, InventoryB} ->
-                    ?LOG_DEBUG(
-                       "Feature flags: inventory of node "
-                       "`~ts`:~n~tp",
-                       [NodeB, InventoryB],
-                       #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
+                    log_inventory(NodeB, InventoryB),
                     case are_compatible(InventoryA, InventoryB) of
                         true ->
                             ?LOG_NOTICE(
@@ -485,6 +478,59 @@ check_node_compatibility_task1(NodeA, NodesA, NodeB, NodesB, NodeAAsVirigin)
             {error, {aborted_feature_flags_compat_check, Error}}
     end.
 
+log_inventory(
+  FromNode,
+  #{feature_flags := FeatureFlags, states_per_node := StatesPerNode}) ->
+    ?LOG_DEBUG(
+       begin
+           AllFeatureNames = lists:sort(maps:keys(FeatureFlags)),
+           Nodes = lists:sort(maps:keys(StatesPerNode)),
+           LongestFeatureName = lists:foldl(
+                                  fun(FeatureName, MaxLength) ->
+                                          Length = length(
+                                                     atom_to_list(
+                                                       FeatureName)),
+                                          if
+                                              Length > MaxLength -> Length;
+                                              true               -> MaxLength
+                                          end
+                                  end, 0, AllFeatureNames),
+           NodeInitialPrefix = lists:duplicate(LongestFeatureName + 1, $\s),
+           {Header,
+            HeaderTail} = lists:foldl(
+                            fun(Node, {String, Prefix}) ->
+                                    String1 = io_lib:format(
+                                                "~ts~ts  ,-- ~ts~n",
+                                                [String, Prefix, Node]),
+                                    NextPrefix = Prefix ++ "  |",
+                                    {String1, NextPrefix}
+                            end, {"", NodeInitialPrefix}, Nodes),
+           lists:flatten(
+             io_lib:format(
+               "Feature flags: inventory queried from node `~ts`:~n",
+               [FromNode]) ++
+             Header ++
+             HeaderTail ++
+             [io_lib:format("~n~*ts:", [LongestFeatureName, FeatureName]) ++
+              [io_lib:format(
+                 "  ~s",
+                 [begin
+                      State = maps:get(
+                                FeatureName,
+                                maps:get(Node, StatesPerNode),
+                                false),
+                      case State of
+                          true           -> "x";
+                          state_changing -> "~";
+                          false          -> " "
+                      end
+                  end])
+               || Node <- Nodes]
+              || FeatureName <- AllFeatureNames] ++
+             [])
+       end,
+       #{domain_ => ?RMQLOG_DOMAIN_FEAT_FLAGS}).
+
 -spec list_nodes_clustered_with(Node) -> Ret when
       Node :: node(),
       Ret :: Members | Error,
@@ -842,12 +888,14 @@ enable_many(#{states_per_node := _} = Inventory, FeatureNames) ->
       Ret :: ok | {error, Reason},
       Reason :: term().
 
-enable_many_locked(#{states_per_node := _} = Inventory, [FeatureName | Rest]) ->
+enable_many_locked(
+  #{states_per_node := _} = Inventory, [FeatureName | Rest]) ->
     case enable_if_supported(Inventory, FeatureName) of
         {ok, Inventory1} -> enable_many_locked(Inventory1, Rest);
         Error            -> Error
     end;
-enable_many_locked(_Inventory, []) ->
+enable_many_locked(
+  _Inventory, []) ->
     ok.
 
 -spec enable_if_supported(Inventory, FeatureName) -> Ret when

diff --git a/deps/rabbit/src/rabbit_ff_registry_factory.erl b/deps/rabbit/src/rabbit_ff_registry_factory.erl
@@ -443,37 +443,66 @@ do_initialize_registry(#{feature_flags := AllFeatureFlags,
                          written_to_disk := WrittenToDisk} = Inventory) ->
     %% We log the state of those feature flags.
     ?LOG_DEBUG(
-      lists:flatten(
-        "Feature flags: list of feature flags found:\n" ++
-        [io_lib:format(
-           "Feature flags:   [~ts] ~ts~n",
-           [case maps:get(FeatureName, FeatureStates, false) of
-                true           -> "x";
-                state_changing -> "~";
-                false          -> " "
-            end,
-            FeatureName])
-         || FeatureName <- lists:sort(maps:keys(AllFeatureFlags)),
-            ?IS_FEATURE_FLAG(maps:get(FeatureName, AllFeatureFlags))] ++
-        "Feature flags: list of deprecated features found:\n" ++
-        [io_lib:format(
-           "Feature flags:   [~ts] ~ts~n",
-           [case maps:get(FeatureName, FeatureStates, false) of
-                true           -> "x";
-                state_changing -> "~";
-                false          -> " "
-            end,
-            FeatureName])
-         || FeatureName <- lists:sort(maps:keys(AllFeatureFlags)),
-            ?IS_DEPRECATION(maps:get(FeatureName, AllFeatureFlags))] ++
-        [io_lib:format(
-           "Feature flags: scanned applications: ~tp~n"
-           "Feature flags: feature flag states written to disk: ~ts",
-           [ScannedApps,
-            case WrittenToDisk of
-                true  -> "yes";
-                false -> "no"
-            end])]),
+       begin
+           AllFeatureNames = lists:sort(maps:keys(AllFeatureFlags)),
+           {FeatureNames,
+            DeprFeatureNames} = lists:partition(
+                                  fun(FeatureName) ->
+                                          FeatureProps = maps:get(
+                                                           FeatureName,
+                                                           AllFeatureFlags),
+                                          ?IS_FEATURE_FLAG(FeatureProps)
+                                  end, AllFeatureNames),
+
+           IsRequired = fun(FeatureName) ->
+                                FeatureProps = maps:get(
+                                                 FeatureName,
+                                                 AllFeatureFlags),
+                                required =:=
+                                rabbit_feature_flags:get_stability(
+                                  FeatureProps)
+                        end,
+           {ReqFeatureNames,
+            NonReqFeatureNames} = lists:partition(IsRequired, FeatureNames),
+           {ReqDeprFeatureNames,
+            NonReqDeprFeatureNames} = lists:partition(
+                                        IsRequired, DeprFeatureNames),
+
+           lists:flatten(
+             "Feature flags: list of feature flags found:\n" ++
+             [io_lib:format(
+                "Feature flags:   [~ts] ~ts~n",
+                [case maps:get(FeatureName, FeatureStates, false) of
+                     true           -> "x";
+                     state_changing -> "~";
+                     false          -> " "
+                 end,
+                 FeatureName])
+              || FeatureName <- NonReqFeatureNames] ++
+             "Feature flags: list of deprecated features found:\n" ++
+             [io_lib:format(
+                "Feature flags:   [~ts] ~ts~n",
+                [case maps:get(FeatureName, FeatureStates, false) of
+                     true           -> "x";
+                     state_changing -> "~";
+                     false          -> " "
+                 end,
+                 FeatureName])
+              || FeatureName <- NonReqDeprFeatureNames] ++
+             [io_lib:format(
+                "Feature flags: required feature flags not listed above: ~b~n"
+                "Feature flags: removed deprecated features not listed "
+                "above: ~b~n"
+                "Feature flags: scanned applications: ~0tp~n"
+                "Feature flags: feature flag states written to disk: ~ts",
+                [length(ReqFeatureNames),
+                 length(ReqDeprFeatureNames),
+                 ScannedApps,
+                 case WrittenToDisk of
+                     true  -> "yes";
+                     false -> "no"
+                 end])])
+       end,
       #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}
      ),