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

Add built-in occurrences filter extension #11

Merged
merged 5 commits into from
Jul 20, 2016
Merged

Conversation

portertech
Copy link
Contributor

@portertech portertech commented Jul 19, 2016

Closes #10

@agoddard
Copy link

<3

event[:occurrences] = 9
@extension.safe_run(event) do |output, status|
expect(status).to eq(0)
event[:check][:refresh] = 12
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that this is testing an event should not match the filter with refresh of 12, do we need a positive match case for refresh as well?

@portertech portertech merged commit 5b52cb3 into master Jul 20, 2016
@portertech portertech deleted the feature/occurrences branch July 20, 2016 20:02
@fessyfoo
Copy link

I'm curious if this filter intended to solve the problems that the sensu-plugins occurrence filter had:

As it stands now I think it replicates the same issue. I assume that's expected?

If it's not expected, consider this sequence of events:

event.occurrences >check.occurrences event not filtered, handler handles create:

{
  "check": {
    "occurrences": 2,
    "status": 2,
    "history": [ 0, 0, 2, 2, 2]
  },
  "occurrences": 3,
  "action": "create"
}

next the status changes state from crit to warn and event.occurrences is reset to 1. The event is filtered. maybe that's ok.

{
  "check": {
    "occurrences": 2,
    "status": 1,
    "history": [ 0, 2, 2, 2, 1]
  },
  "occurrences": 1,
  "action": "create"
}

next comes the resolution event. status is 0, and has stored event so event.occurrences has not incremented, and it's still below the check.occurrences threshold so it is filtered leading to confusing situation where on call personnel may have received create notification but not resolve notification. That's at least a significant part of the confusion in the above mentioned bugs.

{
  "check": {
    "occurrences": 2,
    "status": 0,
    "history": [ 2, 2, 2, 1, 0]
  },
  "occurrences": 1,
  "action": "resolve"
}

Hope that helps make the subtle issue clear to anyone who didn't see that bit already.

related comment: sensu-plugins/sensu-plugin#134 (comment)

I'm curious why the proposed built in occurrences filter or even regular sensu filters would be in any better position to solve those issues.

@cwjohnston
Copy link
Contributor

I've replied to the above question in sensu-plugins/sensu-plugin#134 (comment)

@fessyfoo
Copy link

noting that as of #12 built in extensions have moved out of this repo to individual repos under the sensu-extensions github project.

in particular the occurrences filter has moved to https://github.com/sensu-extensions/sensu-extensions-occurrences and now includes a fix for the filtered resolves issue described above.

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.

4 participants