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

Update limit monitoring with checks for lowering repeat notifications #92

Closed
jordanpadams opened this issue Nov 2, 2018 · 6 comments
Closed
Labels
enhancement New feature or request

Comments

@jordanpadams
Copy link
Contributor

RIght now, if a limit trip continues for an extended period of time, you get bombarded with email notifications. This feature will allow for the ability to receive only every 30th (?) notification.

@aywaldron
Copy link
Contributor

aywaldron commented Nov 19, 2018

After talking with @MJJoyce I'm thinking of adding the following config options to config.notifications.

  • Option for number of times limit exceeded before notification triggered. This would be for cases where limits are known to be exceeded briefly before returning to normal, so a notification is not required if it is in the error range only briefly.
  • Option for frequency of notifications (in terms of time or number of consecutive errors). Would reset each time value exceeds limits again.

Both of these would require keeping track of how long (time and number of packets) a value has been out of limits, and what range the last packet was in.

    notifications:
        email:
            triggers:
                - limit-warn
                - limit-error
            recipients:
                - [email protected]
        options:
            frequency:
                packets: 30  # every 30 packets
                seconds: 120  # every 2 minutes
                # if both are specified, use whichever is most frequent
                # would like suggestions on what the defaults should be
            threshold: 
                packets: 5  # notifies only if out of range for 5+ consecutive packets
                seconds: 60  # notifies only if out of range for 60+ seconds
                # if both specified, use whichever happens first
                # default both to 0

@MJJoyce
Copy link
Member

MJJoyce commented Nov 19, 2018

@aywaldron,

I think this sounds good in general. I would recommend keeping the thresholds / frequency options specified in packets instead of allowing both packets and seconds. The limit checks run each packet anyway so it's a natural fit. Thoughts?

@jordanpadams
Copy link
Contributor Author

@MJJoyce if i remember correctly, the ground limit monitoring runs in a while True loop, so it potentially runs faster than a per packet basis

@jordanpadams
Copy link
Contributor Author

@aywaldron also, by "frequency" do you mean how often AIT checks the limits? or how often it will send a notification when a limit has tripped?

@aywaldron
Copy link
Contributor

aywaldron commented Nov 19, 2018

@jordanpadams I mean how often it sends the notification. The limit would still be checked on every packet.

@MJJoyce No problem just keeping to packet numbers; whatever you think will be most useful. I'm not sure if there are ever cases where packets stop being received or the rate of packets slows down or speeds up and you still want to be notified at the same time interval.

@MJJoyce
Copy link
Member

MJJoyce commented Nov 19, 2018

@jordanpadams, good call, you're correct. I misremembered.

I could be convinced either way. At the moment I just don't see the clearest use case for the time / packet split and keeping it as packet-only seems like it would simplify the logic. So I'm +1 to whatever y'all decide on!

aywaldron added a commit that referenced this issue Nov 21, 2018
…otifications

Implements new limit trip notification 'threshold' config.
Notification only sent once per any number of consecutive
limit trips, and only after limit trip 'threshold' (required
number of consecutive tripping packets) has been met.
aywaldron added a commit that referenced this issue Nov 21, 2018
…otifications

Implements new limit trip notification 'frequency' config
by triggering notifications every 'frequency' consecutive
limit-tripping packet, not taking 'threshold' into account.
aywaldron added a commit that referenced this issue Dec 4, 2018
…otifications

Implements new limit trip notification 'threshold' config.
Notification only sent once per any number of consecutive
limit trips, and only after limit trip 'threshold' (required
number of consecutive tripping packets) has been met.
aywaldron added a commit that referenced this issue Dec 4, 2018
…otifications

Implements new limit trip notification 'frequency' config
by triggering notifications every 'frequency' consecutive
limit-tripping packet, not taking 'threshold' into account.
aywaldron added a commit that referenced this issue Dec 4, 2018
aywaldron added a commit that referenced this issue Dec 4, 2018
…otifications

Implements new limit trip notification 'threshold' config.
Notification only sent once per any number of consecutive
limit trips, and only after limit trip 'threshold' (required
number of consecutive tripping packets) has been met.
aywaldron added a commit that referenced this issue Dec 4, 2018
…otifications

Implements new limit trip notification 'frequency' config
by triggering notifications every 'frequency' consecutive
limit-tripping packet, not taking 'threshold' into account.
aywaldron added a commit that referenced this issue Dec 4, 2018
aywaldron added a commit that referenced this issue Dec 11, 2018
Issue #92: Update monitoring with notification throttling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants