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] Notify on Critical Only #300

Closed
jimmybrancaccio opened this issue Jun 14, 2022 · 13 comments · Fixed by #308
Closed

[FEAT] Notify on Critical Only #300

jimmybrancaccio opened this issue Jun 14, 2022 · 13 comments · Fixed by #308
Labels
enhancement New feature or request good first issue Good for newcomers waiting for response

Comments

@jimmybrancaccio
Copy link

Is your feature request related to a problem? Please describe.

I only have 3 servers added to my Scrutiny instance with a collective 22 drives and I get at least 20 emails per day about various drives in failing states. It's been about 2 weeks since I've had Scrutiny running but I'm at notification overload and at this point I just delete all the emails.

Describe the solution you'd like

To assist with this would it be possible to implement a feature that sends notification on when a critical measurement is failing? For example just notify on these measurements:

scrutiny

Additional context

N/A

@AnalogJ
Copy link
Owner

AnalogJ commented Jun 14, 2022

oh this is definitely a good quality of life feature. I'll definitely add support for filtering notification triggers.

@AnalogJ AnalogJ added enhancement New feature or request good first issue Good for newcomers labels Jun 14, 2022
@AnalogJ
Copy link
Owner

AnalogJ commented Jun 15, 2022

I'm playing around with a couple of different config file syntax options, but I think something like this might make sense:

notify:
  urls:
    - "discord://token@channel"
    - "telegram://token@telegram?channels=channel-1[,channel-2,...]"
    - "pushover://shoutrrr:apiToken@userKey/?priority=1&devices=device1[,device2, ...]"
    ...
  filter_attributes: 'all' # options: 'all' or  'critical'
  level: 'fail' # options: 'fail', 'fail_scrutiny', 'fail_smart'
  

@jimmybrancaccio @gofaster What do you think?

@jimmybrancaccio
Copy link
Author

@AnalogJ Yeah, I like that! It makes sense and would definitely meet my need here.

On a side note, I like how that implementation applies specifically to notifications. A similar method could be used to apply it to the web interface as well ☺️ I planned on opening another feature request to address that, just haven't had a moment yet.

@gofaster
Copy link

@AnalogJ Those filter criteria look great. They provide helpful granularity without getting too complicated.

@AnalogJ
Copy link
Owner

AnalogJ commented Jun 15, 2022

@jimmybrancaccio @gofaster thanks for the feedback!

@jimmybrancaccio there's already an issue tracking the WebUI work in #275

AnalogJ added a commit that referenced this issue Jun 20, 2022
 - failing attribute type (Critical vs All)
 - failure reason (Smart, Scrutiny, Both)

 fixes #300
@AnalogJ
Copy link
Owner

AnalogJ commented Jun 20, 2022

This notification filtering code is now available in the beta branch (and can be tested by using the beta-omnibus docker image tag).

@jimmybrancaccio @gofaster Can you try it out and get back to me? (once the docker images finish builing: https://github.com/AnalogJ/scrutiny/actions/runs/2529933463)

Thanks!

@jimmybrancaccio
Copy link
Author

Hey @AnalogJ! I apologize for the delay in response - crazy busy week. That said, is it safe to assume the beta-web and beta-collector images would have this in them (I use the hub/spoke method). If so, I'll update and let you know ASAP! Thank you!

@AnalogJ
Copy link
Owner

AnalogJ commented Jun 24, 2022

@jimmybrancaccio correct, the beta-* images have the fix.

@jimmybrancaccio
Copy link
Author

I just wanted to quick report back that I do believe it worked. I didn't get a zillion emails overnight 😂

Thank you @AnalogJ for hopping on this so quickly and getting the functionality added!

@AnalogJ
Copy link
Owner

AnalogJ commented Jun 26, 2022

Great, thanks @jimmybrancaccio !

@AnalogJ
Copy link
Owner

AnalogJ commented Aug 3, 2022

FYI @jimmybrancaccio @gofaster
The notify.filter_attributes and notify.level options in the config file will be removed in v0.5.0 and replaced by settings stored in the database.

When #352 is released, scrutiny will throw an error during startup stating that those keys must be removed from the config file.

Sorry about that -- I'm consolidating all the settings in the database (finally) and rolling out the changes to the frontend (as we discussed).

Appreciate your understanding

@jimmybrancaccio
Copy link
Author

@AnalogJ Not a problem at all! Thank you very much for giving a heads up! 💙

@AnalogJ
Copy link
Owner

AnalogJ commented Aug 4, 2022

FYI, v0.5.0 has been released, and notify.filter_attributes and notify.level have been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers waiting for response
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants