From: Nitzan Mordechai <nmordech@redhat.com>
Date: Thu, 19 Jun 2025 08:54:43 +0000 (+0000)
Subject: monitor: Enhance historic ops command output and error handling
X-Git-Tag: testing/wip-jcollin-testing-20250905.013620-squid~18^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=6c88d29bf05393a8bae225fc5a7ac5efe1e06af6;p=ceph-ci.git

monitor: Enhance historic ops command output and error handling

Dumping monitor historic operations currently yields no results
and incorrectly issues an error message indicating that
"mon_enable_op_tracker" is not enabled, even when it should be.

This commit addresses these issues by:
- Adding previously missing commands for historic operations.
- Correcting the dump operations check to only issue an error when
  "mon_enable_op_tracker" is genuinely not enabled.
- Tracking "mon_enable_op_tracker" changes
- Refactoring and organizing the historic operations dump command code.
- Improving the appearance and clarity of error messages.

Fixes: https://tracker.ceph.com/issues/71725
Signed-off-by: Nitzan Mordechai <nmordec@ibm.com>
(cherry picked from commit 5edb4099927eb4b2813115ef8756e1160345f823)
---

diff --git a/src/mon/MonCommands.h b/src/mon/MonCommands.h
index 434b125350d..2c0ab671efd 100644
--- a/src/mon/MonCommands.h
+++ b/src/mon/MonCommands.h
@@ -1491,7 +1491,15 @@ COMMAND_WITH_FLAG("dump_historic_ops",
             "show recent ops",
             "mon", "r",
             FLAG(TELL))
+COMMAND_WITH_FLAG("dump_historic_ops_by_duration",
+            "show recent ops sorted by duration",
+            "mon", "r",
+            FLAG(TELL))
 COMMAND_WITH_FLAG("dump_historic_slow_ops",
             "show recent slow ops",
             "mon", "r",
             FLAG(TELL))
+COMMAND_WITH_FLAG("dump_ops_in_flight",
+            "show the ops currently in flight",
+            "mon", "r",
+            FLAG(TELL))
diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index c398854ac58..96c89d3ecc6 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -365,28 +365,45 @@ int Monitor::do_admin_command(
     start_election();
     elector.stop_participating();
     out << "stopped responding to quorum, initiated new election";
-  } else if (command == "ops") {
-    (void)op_tracker.dump_ops_in_flight(f);
   } else if (command == "sessions") {
     f->open_array_section("sessions");
     for (auto p : session_map.sessions) {
       f->dump_object("session", *p);
     }
     f->close_section();
-  } else if (command == "dump_historic_ops") {
-    if (!op_tracker.dump_historic_ops(f)) {
-      err << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \
-        please enable \"mon_enable_op_tracker\", and the tracker will start to track new ops received afterwards.";
-    }
-  } else if (command == "dump_historic_ops_by_duration" ) {
-    if (op_tracker.dump_historic_ops(f, true)) {
-      err << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \
-        please enable \"mon_enable_op_tracker\", and the tracker will start to track new ops received afterwards.";
-    }
-  } else if (command == "dump_historic_slow_ops") {
-    if (op_tracker.dump_historic_slow_ops(f, {})) {
-      err << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \
-        please enable \"mon_enable_op_tracker\", and the tracker will start to track new ops received afterwards.";
+  } else if (command == "dump_ops_in_flight" ||
+             command == "ops" ||
+             command == "dump_historic_ops" ||
+             command == "dump_historic_ops_by_duration" ||
+             command == "dump_historic_slow_ops") {
+    const string error_str = "op_tracker tracking is not enabled now, so no ops are tracked currently, \
+even those get stuck. Please enable \"mon_enable_op_tracker\", and the tracker \
+will start to track new ops received afterwards.";
+    if (command == "dump_historic_ops") {
+      if (!op_tracker.dump_historic_ops(f)) {
+        err << error_str;
+        r = -EINVAL;
+        goto abort;
+      }
+    } else if (command == "dump_historic_ops_by_duration" ) {
+      if (!op_tracker.dump_historic_ops(f, true)) {
+        err << error_str;
+        r = -EINVAL;
+        goto abort;
+      }
+    } else if (command == "dump_historic_slow_ops") {
+      if (!op_tracker.dump_historic_slow_ops(f, {})) {
+        err << error_str;
+        r = -EINVAL;
+        goto abort;
+      }
+    } else if (command == "ops" ||
+               command == "dump_ops_in_flight") {
+      if (!op_tracker.dump_ops_in_flight(f)) {
+        err << error_str;
+        r = -EINVAL;
+        goto abort;
+      }
     }
   } else if (command == "quorum") {
     string quorumcmd;
@@ -637,6 +654,7 @@ const char** Monitor::get_tracked_conf_keys() const
     // debug options - observed, not handled
     "mon_debug_extra_checks",
     "mon_debug_block_osdmap_trim",
+    "mon_enable_op_tracker",
     NULL
   };
   return KEYS;
@@ -678,6 +696,10 @@ void Monitor::handle_conf_change(const ConfigProxy& conf,
       scrub_update_interval(scrub_interval);
     }});
   }
+  
+  if (changed.count("mon_enable_op_tracker")) {
+    op_tracker.set_tracking(conf.get_val<bool>("mon_enable_op_tracker"));
+  }
 }
 
 void Monitor::update_log_clients()