Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design how incident relations should work #313

Open
hmpf opened this issue Aug 20, 2021 · 5 comments
Open

Design how incident relations should work #313

hmpf opened this issue Aug 20, 2021 · 5 comments
Labels
API Affects Argus' REST API data model Affects the data model and/or SQL schema discussion Requires developer feedback/discussion before implementation enhancement New feature or request frontend Has/needs companion issue in frontend notification Affects the notification system user feedback needed Feedback from end users required to solve this

Comments

@hmpf
Copy link
Contributor

hmpf commented Aug 20, 2021

class IncidentRelationType(models.Model):
    name = models.TextField()

    class Meta:
        ordering = ["name"]

    def __str__(self):
        return self.name


class IncidentRelation(models.Model):
    # "+" prevents creating a backwards relation
    incident1 = models.ForeignKey(to=Incident, on_delete=models.CASCADE, related_name="+")
    incident2 = models.ForeignKey(to=Incident, on_delete=models.CASCADE, related_name="+")
    type = models.ForeignKey(to=IncidentRelationType, on_delete=models.PROTECT, related_name="incident_relations")
    description = models.TextField(blank=True)

    def __str__(self):
        return f"Incident #{self.incident1.pk} {self.type} #{self.incident2.pk}"

See also PR #312, which so far ensures that incident1 and incident2 cannot be identical.

If name is "duplicate", str(IncidentRelation) would look something like "Incident #32 duplicate #17", which is not very useful.

@hmpf hmpf added enhancement New feature or request discussion Requires developer feedback/discussion before implementation frontend Has/needs companion issue in frontend data model Affects the data model and/or SQL schema API Affects Argus' REST API notification Affects the notification system labels Aug 20, 2021
@hmpf
Copy link
Contributor Author

hmpf commented Aug 20, 2021

It would be useful to see who or what created the relation. This could be readable from the API but probably not changeable.

@hmpf
Copy link
Contributor Author

hmpf commented Aug 20, 2021

The main elephant is the room is IncidentRelationType. Do we need it at all? If we need it, who gets to decide what types exists? How do we render them? What types are relevant?

Assuming we will need at least three types:

  1. x is a duplicate of y
  2. x is caused by y
  3. x has the same cause as y

One of the purposes of IncidentRelations is to be able to hide ("roll up") certain incidents. In the cases of type 1 and 3, which do we hide? One suggestion is whichever is newest. In the case of 2, we should probably hide x and maybe increase the severity of y. (Then log that increase somehow.) Maybe all the incidents should be hidden and given a relation to a new incident created by the source argus?

If a source that is excluded from notificationfilters is a cause of something that does trigger a notification, shouldn't that still lead to a notification?

What should actually be stored in the model? If just "name", what is a good/relevant name?

@hmpf
Copy link
Contributor Author

hmpf commented Aug 20, 2021

if we decide to render different types differently, there are at least two ways to go about it. One is to have the format to use in a field on IncidentRelationType:

class IncidentRelationType(models.Model):
    name = models.TextField()
    format = models.TextField(default="{incident1} is related to {incident2} somehow")
    ..

class IncidentRelation(models.Model):
    ..

    def __str__(self):
        return self.type.format.format(incident1=self.incident1, incident2=self.incident2)

Pro: Easy to add new types
Con: Hard to have other different behavior per type, for instance validation, log messages

Another is to use proxy models: one per type, with separate str-methods:

class IncidentRelation(models.Model):
    ..

class CauseIncidentRelation(IncidentRelation):

    class Meta:
        proxy = True

    def __str__(self):
        return f"{self.incident2} is caused by {self.incident1}

Pro: Easy to add other type-specific ways of doing things
Con: Each new type needs at least a patch-release and needs to be deployed

@hmpf
Copy link
Contributor Author

hmpf commented Aug 20, 2021

What happens as parts of a relation gets acked or closed or have other events? If we create a meta-incident, can it be automatically closed?

@hmpf
Copy link
Contributor Author

hmpf commented Aug 20, 2021

If we have types like "causes/caused by", it becomes important to get which incident is the cause and which is the result correctly, and this needs to be visible in the API somehow. For types like "has the same (unknown) cause", this doesn't matter, and it should behave differently in the frontend.

@hmpf hmpf added the user feedback needed Feedback from end users required to solve this label Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Affects Argus' REST API data model Affects the data model and/or SQL schema discussion Requires developer feedback/discussion before implementation enhancement New feature or request frontend Has/needs companion issue in frontend notification Affects the notification system user feedback needed Feedback from end users required to solve this
Projects
None yet
Development

No branches or pull requests

1 participant