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

feat: show which children are down for group notifications #5192

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

julian-piehl
Copy link
Contributor

@julian-piehl julian-piehl commented Oct 12, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Before if a group fails you just get the message "Child inaccessible".
Now you can see which childs are failing and what their messages are.

Before After
image image

Fixes #4258, #3278

Type of change

Please delete any options that are not relevant.

  • User interface (UI)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • My changes generates no new warnings
  • My code needed automated testing. I have added them (this is optional task)

@julian-piehl
Copy link
Contributor Author

Is it a good idea to display the details? Or should it just show the names of the childs?
I thought showing the whole message could be useful if using nested groups, but maybe it could get messy with some notification services.

@CommanderStorm
Copy link
Collaborator

I don't think showing the details is helpful.
I think of the case where 10 or maybe 100 children are down.. that would be pretty bad..

=> Let's limit ourselves to a (truncated if too many are down) list of down direct children

@julian-piehl
Copy link
Contributor Author

julian-piehl commented Oct 13, 2024

I don't think showing the details is helpful. I think of the case where 10 or maybe 100 children are down.. that would be pretty bad..

=> Let's limit ourselves to a (truncated if too many are down) list of down direct children

True, that could become too extreme with too many child elements.
What do you think—how many characters should it be truncated to?

I just had another idea: We could make it more compact and directly provide the actual names of the monitors that have problems.
For example:

Group A <-- Notification is only enabled here
├─ Group B
│  ├─ Group C
│  │  ├─ Monitor 1 <-- Imagine this is down

Currently the message would be something like:

Some Children are having problems (Group B)
- Group B:
  Some Children are having problems (Group C)
  - Group C:
    Some Children are having problems (Monitor 1)
    - Monitor 1:
      Something went wrong

We could maybe change it so it would look something like:

Some Children are having problems (Group B / Group C / Monitor 1)
- Group B / Group C / Monitor 1:
  Something went wrong

Maybe in combination with truncate? Or we omit the details completely and just show the whole path to the children failing?

The only problem I see in this idea is, that if you have many nested monitors it could take a bit longer to generate the message because its iterating through them.

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Oct 13, 2024

could take a bit longer to generate the message because its iterating through them.

Should be possible in one DB-call via a recursive CTE, see #5193 (review) for an example.

Currently the message would be something like

I think we can trim out the redundant messaging. What do you think about this? (with a pretty agressive limit of 2 groups/monitors per level for demonstration purposes)

Some Children are having problems:
|-> Group A:
    |-> Group A1:
        |-> Monitor 1
        |-> Monitor 2
        ... and 298 more monitors
    |-> Group A2:
        |-> Monitor 3
    ... and 4 more groups
|-> Group A:
    |-> Monitor 4
... and 2 more groups

@CommanderStorm CommanderStorm changed the title feat: show group status feat: show which children are down for group notifications Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

notifications which group a child is in
2 participants