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

Updating logic to consider occurrence_watermark when handling resolve events #2

Merged
merged 2 commits into from
Aug 22, 2016

Conversation

cwjohnston
Copy link
Contributor

Sensu 0.26 adds the occurrences_watermark event attribute which tracks the
"high water mark" for number of occurrences at the current severity.

This logic asserts that any resolve event should be handled if its
occurrences_watermark value is equal to or greater than the configured
check occurrences.

… events

Sensu 0.26 adds the `occurrences_watermark` event attribute which tracks the
"high water mark" for number of occurrences at the current severity.

This logic asserts that any resolve event should be handled if its
occurrence_watermark value is equal to or greater than the configured
check occurrences.
@cwjohnston
Copy link
Contributor Author

Closes #1

cc @fessyfoo for review

@@ -22,8 +22,11 @@ def description
def event_filtered?(event)
check = event[:check]
occurrences = check[:occurrences] || 1
watermark = event[:occurrences_watermark] || 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the default? Not configurable or optional with 0.26.

Copy link
Contributor Author

@cwjohnston cwjohnston Aug 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@portertech I think I added the default to make things work before I updated the event spec. Although gem extension loading is new in 0.26, I figured there's an outside chance that someone will try to manually install this filter extension in their extensions directory on an older version.

I'll remove the default if you prefer.

The occurrences_watermark attribute added in 0.26 is neither configurable nor
optional. As this gem is intended for use with the gem extension loading also
added in 0.26, this extension will assume that events have the
occurrences_watermark attribute.
@portertech
Copy link
Contributor

Looks good 👍

@fessyfoo
Copy link

fessyfoo commented Aug 23, 2016

skimmed this, got distracted, intended to come back and think it through harder. On first pass it definitely looked good. tx!

the irony here is i've been thinking about this occurrences issue with you, but we're using Yelp/sensu_handlers, so we're going to need to implement a similar solution for deprecating the overridden filter_repeated. ( /cc @solarkennedy fyi )

@fessyfoo
Copy link

fessyfoo commented Oct 10, 2016

noting for cross reference this solves the issue with filtered resolution events described in sensu/sensu-extensions#11 (comment) and actually several other tickets. (that i should link here)

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.

3 participants