A global configuration form should be available to allow setting values like debugging, run on cron, etc.

Debugging

This module should come with debugging options that allow developers to enable excessive logging to aid in tracking down issues with the System Monitor module and its various processes.

Cron

The global configuration form should allow toggling a default cron job implementation for all configured monitors. It should also include a disclaimer that developers should only enable it if they’re only monitoring a few processes. Otherwise they should consider using the drush command implementation.

Checklist Items

  1. Create configuration page for Monitor global settings
  2. Implement debug logging
  3. Create cron hook
CommentFileSizeAuthor
#6 interdiff-3074969-3-6.txt2.23 KBrobpowell
#6 3074969-6.patch7.89 KBrobpowell
#3 3074969-3.patch6.78 KBrobpowell

Comments

merauluka created an issue. See original summary.

merauluka’s picture

merauluka commented
Assigned: merauluka » robpowell

Assigning to Rob.

robpowell’s picture

robpowell
he/him
Location Boston
commented
StatusFileSize
new 3074969-3.patch6.78 KB

This should satisfy 1 and 2 but I need some guidance on #3.

robpowell’s picture

robpowell
he/him
Location Boston
commented
Status: Active » Needs review
merauluka’s picture

merauluka commented
Status: Needs review » Needs work 
@@ -49,20 +54,32 @@ class ConfigForm extends ConfigFormBase {
    * {@inheritdoc}
    */
   public function buildForm(array $form, FormStateInterface $form_state) {
-    $form = parent::buildForm($form, $form_state);
+    $config = $this->config(SystemMonitorSettingsForm::CONFIG_NAME);

I think that this could just be self::CONFIG_NAME, right?

+    $form['debug'] = [
+      '#type' => 'checkbox',
+      '#title' => $this->t('Debugging'),
+      '#default' => isset($config['debug']) ? $config['debug'] : '',
+    ];

This could have a description on it about what this mode does. And a warning about not leaving it enabled it in production for too long since it creates TONS of log messages.

For the cron hook, you should be able to implement hook_cron in the .module file. Load in the cron setting from config to determine if it should run anything, and then add a TODO to add in the actual running of the system monitor tasks.

The ability to run those tasks is what I'm working on in #3087826: Implement logic to run monitors and tasks. And I'm super close to having that ready, but it's not quite there yet.

robpowell’s picture

robpowell
he/him
Location Boston
commented
StatusFileSize
new 3074969-6.patch7.89 KB
new interdiff-3074969-3-6.txt2.23 KB

I made the above changes.

Here is a git lab diff for your convenience: 3074969

robpowell’s picture

robpowell
he/him
Location Boston
commented
Assigned: robpowell » merauluka
Status: Needs work » Needs review
merauluka’s picture

merauluka commented
Status: Needs review » Reviewed & tested by the community

Looks good to me @robpowell!

brianwagner’s picture

brianwagner commented

In SystemMonitorSettingsForm.php::buildForm(), we have two checkboxes: one defaults to "", the other defaults to 0. Do we want both to be bool values? 

 '#default' => isset($config['debug']) ? $config['debug'] : '',

'#default' => isset($config['cron']) ? $config['cron'] : 0,

Also do we need configFactory in SystemMonitorTaskManager.php? It doesn't seem to be implemented yet.

  • robpowell authored 739f0e6 on 8.x-1.x 
    Issue #3074969 by robpowell, merauluka, brianwagner: System Monitor -...
robpowell’s picture

robpowell
he/him
Location Boston
commented

Thanks brianwagner, I updated the default value. For now, let's keep configFactory.

robpowell’s picture

robpowell
he/him
Location Boston
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.