openwisp / openwisp-monitoring

Network monitoring system written in Python and Django, designed to be extensible, programmable, scalable and easy to use by end users: once the system is configured, monitoring checks, alerts and metric collection happens automatically.
https://openwisp.io/docs/dev/monitoring/
Other
164 stars 111 forks source link

[feature] Add an Alert Settings section in device admin #52

Closed nepython closed 4 years ago

nepython commented 4 years ago

Add an Alert Settings section in the device admin which allows to see what alerts are available for a device and which also allow to modify the threshold or disable/enable the alarm.

nepython commented 4 years ago

I was trying to figure out what can be done here. It won't be as simple as #53 (PR #148), as we need to display alertsettings related to a device but there doesn't exist any direct relation between these two models. They are related by metric and django doesn't provide an easy way to do this.

There are few options that we may use:

The first and third approach (adds a dependency) seem better to me, in case there is some other better way please let me know, I have tried almost everything I could get. Inlines have many limitations too like list_display doesn't work other wise I could have tried adding a column. Please let me know which approach seems better?

PabloCastellano commented 4 years ago

Since AlertSettings.metric already links to the Metric model and Metric contains a GenericForeignKey. Did you try defining a reverse generic relation?

From the documentation:

The relation on the related object back to this object doesn’t exist by default. Setting related_query_name creates a relation from the related object back to this one. This allows querying and filtering from the related object.

This way you can track back [ Device ]-- Generic FK---->[ Metric ]----- FK ----->[ AlertSettings ] and the Django inline models should work

nepython commented 4 years ago

Reverse generic relations need to be declared in order to use GenericTabularInline, so that's not a problem here. I am not sure if I get you correctly but related_query_name is to be used for reverse querying as explained in https://stackoverflow.com/a/36166644.

This is the structure currently, I tried tracing it back but the only way it worked was by declaring the three fields as mentioned in https://github.com/openwisp/openwisp-monitoring/issues/52#issuecomment-650563847

class AbstractMetric(TimeStampedEditableModel):
    content_type = models.ForeignKey(
        ContentType, on_delete=models.CASCADE, null=True, blank=True
    )
    object_id = models.CharField(max_length=36, db_index=True, blank=True)
    content_object = GenericForeignKey('content_type', 'object_id')
    # ...

class AbstractAlertSettings(TimeStampedEditableModel):
    metric = models.OneToOneField(
        get_model_name('monitoring', 'Metric'), on_delete=models.CASCADE
    )
    # ...

class DeviceData(AbstractDeviceData, Device):
    checks = GenericRelation(get_model_name('check', 'Check'))
    metrics = GenericRelation(get_model_name('monitoring', 'Metric'))
    # ...

Let me know if I am missing anything.

PabloCastellano commented 4 years ago

@nepython What is the current problem you are trying to solve? By reading your comment I think you are mixing multiple ones.

Let's go step by step. Can you answer the following questions?

  1. Are you able to retrieve the metrics associated to a device?
  2. Are you able to retrieve the Alertsettings for those metrics?
  3. Did you find any issue displaying the alertsettings fields in the device page?

Also notice that the generic FK from Metric points to the Device model, not DeviceData. Is that the issue?

https://github.com/openwisp/openwisp-monitoring/blob/23f3105c539950d1f547f5dcefcda1228c09bcf1/openwisp_monitoring/device/api/views.py#L191-L200

nepython commented 4 years ago
  1. Are you able to retrieve the metrics associated to a device?

Yes

  1. Are you able to retrieve the Alertsettings for those metrics?

Not entirely, alertsettings can be pointed to as a readonly field only. If I put it as a editable field then it simply raises an error, no such field (I find it weird as to why is this the case).

  1. Did you find any issue displaying the alertsettings fields in the device page?

I am not able to retrieve alertsettings fields via the inline. There are many limitations, few are:

Also notice that the generic FK from Metric points to the Device model, not DeviceData. Is that the issue?

Thanks for correcting me but that's not the issue.

PabloCastellano commented 4 years ago

@nepython Ok. I think I understand you better now. Now, can you provide a draft Pull Request showing some AlertSettings fields if possible? So I can help you iterating.