Skip to content

Adds Sensu integration support#29

Merged
ChezCrawford merged 17 commits intomainfrom
adds-sensu-integration-support
Aug 25, 2021
Merged

Adds Sensu integration support#29
ChezCrawford merged 17 commits intomainfrom
adds-sensu-integration-support

Conversation

@rafusel
Copy link
Copy Markdown
Contributor

@rafusel rafusel commented Aug 4, 2021

Adds Sensu integration support.

@rafusel rafusel marked this pull request as ready for review August 5, 2021 22:50
if pagerDutyEventAction, isActionPresent := sensuToPagerDutyEventType[action]; isActionPresent {
return pagerDutyEventAction, nil
}
return sensuToPagerDutyEventType["create"], nil
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just for my own understanding: Is this the expected behavior that Sensu "action" values other than create and resolve will also become PagerDuty trigger?

If so that may warrant a comment?

Copy link
Copy Markdown
Contributor Author

@rafusel rafusel Aug 19, 2021

Choose a reason for hiding this comment

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

Yeah from the current pdagent code that seems like that's what we're doing: https://github.com/PagerDuty/pdagent-integrations/blob/master/bin/pd-sensu#L79-L83, I'll add a comment for sure. Added here: dc34a73

Comment thread pkg/cmdutil/util.go Outdated
@@ -0,0 +1,31 @@
package cmdutil
Copy link
Copy Markdown

@ChezCrawford ChezCrawford Aug 19, 2021

Choose a reason for hiding this comment

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

Maybe a more descriptive name for this file would be json something like that?

Copy link
Copy Markdown
Contributor Author

@rafusel rafusel Aug 19, 2021

Choose a reason for hiding this comment

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

Renamed this file to mapConversion.go for clarity, and since the only thing this file contains now is the StringMapToInterfaceMap function. 2ca6bd8

Comment thread pkg/cmdutil/util.go Outdated

import "strings"

func GetNestedStringField(inputMap map[string]interface{}, selector string) (string, bool) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This might be ever-so-slightly cleaner if we accepted keys ...string instead of having to parse a selector format.

Copy link
Copy Markdown
Contributor Author

@rafusel rafusel Aug 19, 2021

Choose a reason for hiding this comment

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

I'll resolve the other comments on this function since I kind of changed how this is done. I think you're right that getting nested field function was doing a little too much when all we really need it to get something nested one layer deep. Took a different approach here: f9d92db. I decided not to use gjson because this method is used in the zabbix integration which does not use json data. The data should stay in map[string]interface{} format in sensu and zabbix to be able to chare map accessor and validation code

Comment thread pkg/cmdutil/util.go Outdated
@rafusel rafusel force-pushed the adds-sensu-integration-support branch from 9abc27e to 9b90c3d Compare August 19, 2021 14:39
Comment thread pkg/cmdutil/validate.go Outdated
@@ -1,5 +1,21 @@
package cmdutil

func ValidateMapFieldIsString(inputMap map[string]interface{}, selector string) (string, bool) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm going to delete these since they do not seem to be used.

@ChezCrawford ChezCrawford force-pushed the adds-sensu-integration-support branch from 1f8ccb9 to 391da5e Compare August 25, 2021 00:50
Copy link
Copy Markdown

@ChezCrawford ChezCrawford left a comment

Choose a reason for hiding this comment

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

This looks great Lewis. Thanks for sticking with it.

@ChezCrawford ChezCrawford merged commit 519d57b into main Aug 25, 2021
@ChezCrawford ChezCrawford deleted the adds-sensu-integration-support branch August 25, 2021 00:59
@rafusel rafusel restored the adds-sensu-integration-support branch August 25, 2021 14:38
@rafusel rafusel deleted the adds-sensu-integration-support branch August 25, 2021 14:58
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