-
Notifications
You must be signed in to change notification settings - Fork 21
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
Sensu Handler shim for Sensu 2.0 event data. #63
Sensu Handler shim for Sensu 2.0 event data. #63
Conversation
There is still some work required to get this working, it would be good to have someone who knows Ruby better than I do compare it with Jef's recent PR to sensu-plugin. |
sensu_plugin/utils.py
Outdated
event = json.loads(event) | ||
|
||
# Trigger mapping code if enity exists and client does not | ||
if "client" not in event and "entity" in event: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how the python plugin does things in general for setting up handlers, but the skel ruby plugin will pre-populate the client attribute as an empty dict when being used in normally for a handler that depends on sensu-plugin. So in the ruby I specifically check if the client attribute is missing or if the client attribute value is empty.
sensu_plugin/utils.py
Outdated
state = "unknown::2.0_event" | ||
|
||
if "action" not in event: | ||
event['action'] = action_state_mapping[state.lower()] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the ruby i have a fallback to use save state variable as event['action'] if state.lower() is not a valid key in the action_state_mapping. the python change is something like this
if "action" not in event:
if state.lower() in action_state_mapping:
event['action'] = action_state_mapping[state.lower()]
else:
event['action'] = state
sensu_plugin/utils.py
Outdated
history_v2 = event['check']['history'] | ||
event['check']['history_v2'] = history_v2 | ||
legacy_history = [] | ||
for history in event['check']['history']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm always wary in python or ruby about references versus copied values.
I think this is okay, but the unit test logic should definitely try to make sure everything is working correctly.
In the ruby i go out of my way to make sure the history_v2 object is a new duplicated devoid of any references to the existing history object. If you want to duplicate my paranoia this is probably a deepcopy operation instead of just an assignment.
sensu_plugin/utils.py
Outdated
event['check']['history_v2'] = history_v2 | ||
legacy_history = [] | ||
for history in event['check']['history']: | ||
legacy_history.append(str(history['status'])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the ruby i have some fallback logic to record '3' in case the status value is not an integer that can be converted into a string. Just in case history status is nil for some reason or the event is malformed.
Not sure how to write the full fallback logic here in python. The english:
if status is not an integer that can be turned into a string... use the string value "3" which indicates unknown status and move on.
@barryorourke mapping looks good comments are basically falback protections in case the event is malformed. Also do you have a mechanism in the plugin by which you can add a runtime option to SensuHandler and SensuMutator base classes to enable this mapping for any plugin that inherits from this? In the ruby i expose this as both a runtime cmdline option and as an environment variable check in the base Handler and Mutator classes..so any dependent plugin gets access to the mapping free of charge by depending on the latest sensu-plugin. |
@jspaleta Thanks for the feedback, I'll take a look it on Monday. I'm planning to add a run time option to the Handler Class, but ran out of time on Friday. Figured it would be good to get you to take a look at it early so that I can get it out as quick as possible :) We don't have Mutator support in the Python module yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good agree with Jefs comments.
… if the appropriate argument is passed to the handler
Looks legit to me @jspaleta any last review? |
I've now tested this on my development platform and am happy that it is working :) |
The event data model changed in Sensu 2.0, so to make it possible to continue to use the community managed handlers a quick mapping back into Sensu 1.4 event JSON representation can be performed.
This is a port of the recent PR merged into sensu-plugin gem (sensu-plugins/sensu-plugin#190).