fix(notification/rule): Include the edge of the boundary we are observing. #19392
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brettbuddin
force-pushed
the
bb/notification-trigger-alignment
branch
from
595fbf1
to
e931334
Compare
|
@lyondhill All @aanthony1243 and @wolffcm's work 😄 We will need to think about regenerating existing Notification Tasks once this gets up into IDPE. |
This might not be too difficult. Could we just cause an update to the check that regenerated the task? |
|
@lyondhill That's what I'm thinking. |
brettbuddin
force-pushed
the
bb/notification-trigger-alignment
branch
from
29c056f
to
1890404
Compare
GeorgeMac
approved these changes
Aug 24, 2020
Cubone21
pushed a commit
that referenced
this pull request
Aug 25, 2020
…ving. (#19392) * fix(notification/rule): Include the edge of the boundary we are observing. * chore(changelog): Add 19392 to changelog.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We've been witnessing some failed observations of Check triggers in Notification Tasks. In
from(start: -10m, stop: now()), thestopis exclusive to the time range—this is so consecutive windows likeA -> B,B -> C, don't have overlap. If a Notification Task's schedule boundaries align with Check trigger observations, we will miss observations if they occur on that boundary.This change includes the left-hand boundary in what the Notification Task sees of Check triggers.