feat: add support for opsgenie #73

JonasPf · 2020-11-10T11:38:05Z

No description provided.

codecov-io · 2020-11-10T11:39:11Z

Codecov Report

Merging #73 (f0c460d) into main (ce75958) will decrease coverage by 3.61%.
The diff coverage is 73.48%.

@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
- Coverage   65.93%   62.32%   -3.62%     
==========================================
  Files          56       65       +9     
  Lines        1371     1858     +487     
==========================================
+ Hits          904     1158     +254     
- Misses        386      576     +190     
- Partials       81      124      +43

Impacted Files	Coverage Δ
pkg/format/format.go	`0.00% <0.00%> (ø)`
pkg/format/format_query.go	`0.00% <0.00%> (ø)`
pkg/services/opsgenie/opsgenie.go	`69.38% <69.38%> (ø)`
pkg/services/opsgenie/opsgenie_entity.go	`71.42% <71.42%> (ø)`
pkg/format/formatter.go	`56.71% <74.52%> (ø)`
pkg/services/opsgenie/opsgenie_config.go	`85.71% <85.71%> (ø)`
pkg/router/servicemap.go	`100.00% <100.00%> (ø)`
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce75958...f0c460d. Read the comment docs.

piksel

Looks good, but I have added some feedback.

pkg/services/opsgenie/opsgenie_json.go

pkg/services/opsgenie/opsgenie_test.go

pkg/services/opsgenie/opsgenie_config.go

JonasPf · 2020-11-12T11:34:41Z

Thanks for the feedback. I won't have time within the next 2 weeks but I'll get back to it after that to fix those things.

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

…channel (used to segfault) (containrrr#83) Co-authored-by: Simon Aronsson <[email protected]>

JSON is too generic. OpsGenie uses the term "payload" in their documentation so I've used that instead.

Shoutrrr uses the term `service` by convention

JonasPf · 2020-12-10T20:59:55Z

Hi All,

I pushed changes that address the issues mentioned above. Apparently I also rebased wrong or something like that because I see a few commits that probably shouldn't be in this branch. What's your general policy here? I can fix this if you are ok with a force-push to this branch. Alternatively I could open a new PR.

piksel · 2020-12-12T12:48:15Z

I would squash this anyway, so it should be fine!

piksel

A lot of great work has been put into this, but I'm not so sure about the raw JSON URLs.
Left some comments.

pkg/services/opsgenie/opsgenie_config.go

docs/services/opsgenie.md

pkg/services/opsgenie/opsgenie_json.go

simskij · 2020-12-20T17:10:23Z

Alright, closing this for the time being then. We try to aim at only implementing services that people actually will use. If anyone actually wants/needs Opsgenie support, please open another ticket and let us know.

Thanks for your contributions!

JonasPf · 2020-12-24T10:03:21Z

Alright, closing this for the time being then. We try to aim at only implementing services that people actually will use. If anyone actually wants/needs Opsgenie support, please open another ticket and let us know.

Thanks for your contributions!

Well, I'm using it, that's why I implemented the integration in the first place ;-)

I'm happy to work on the remaining issues if you plan on merging it, just let me know. Until then, it works well enough for me as it is.

simskij · 2020-12-24T10:58:50Z

I must have mixed issues together! I closed this because I was under the impression that you abandoned the idea of using shoutrrr for your project.

Reopening.

JonasPf · 2020-12-28T10:38:14Z

I must have mixed issues together! I closed this because I was under the impression that you abandoned the idea of using shoutrrr for your project.

Reopening.

No worries, I know I'm sometimes a bit slow to respond. Especially around Christmas :-)

Co-authored-by: nils måsén <[email protected]>

JonasPf · 2021-01-06T20:35:27Z

I just pushed some changes. The service now works well with the generate and verify commands, has a couple more tests and I merged all the changes from master.

Next I'll be working on integrating PropKeyResolver and replacing the raw json fields with more human readable format as suggested by @piksel.

pkg/services/opsgenie/opsgenie_json.go

piksel

Great work overall! The suggested changes are optional, but will increase the test coverage a bit if nothing else :)
Also; I merged the send ArraySlice fix, so now you can correctly test using the CLI (if you rebase/merge main)

JonasPf · 2021-02-03T11:22:50Z

I merged the changes in master. Problem is that formatQuery.BuildQuery() got changed to use Values.Encode(). This breaks a test because it encode a comma. Same problem as mentioned above. I left the failing code in there for you to see it.

generate doesn't work with the Responders and VisibleTo fields. It keeps saying Invalid format for field Responders: field format is not supported which makes sense because formatter.go doesn't know how to deserialize entities.
I don't know how to solve that yet. Need to think about it. Any ideas?

Btw. I found two unrelated issues:

piksel

Nice work!

I will look into creating an interface for de-/serializing structs that the entities can implement.

If you move out the looping from the entity methods to a pair of:

(e *Entity) SetFromProp(propValue string) error {  }

and

(e *Entity) GetPropValue() (string, error) {  }

I will try to get it implemented ASAP

pkg/format/format_query.go

pkg/services/opsgenie/opsgenie_config.go

piksel · 2021-02-03T13:06:32Z

pkg/services/opsgenie/opsgenie_test.go

+
+	When("generating a config from a url with query parameters", func() {
+		It("should populate the config fields with the query parameter values", func() {
+			queryParams := `alias=Life+is+too+short+for+no+alias&description=Every+alert+needs+a+description&actions=An+action&tags=tag1,tag2&details=key:value,key2:value2&entity=An+example+entity&source=The+source&priority=P1&user=Dracula&note=Here+is+a+note&responders=user:Test,team:NOC&visibleTo=user:A+User`


This would make the test work with the encoded query field values:

Suggested change

queryParams := `alias=Life+is+too+short+for+no+alias&description=Every+alert+needs+a+description&actions=An+action&tags=tag1,tag2&details=key:value,key2:value2&entity=An+example+entity&source=The+source&priority=P1&user=Dracula&note=Here+is+a+note&responders=user:Test,team:NOC&visibleTo=user:A+User`

queryParams := `alias=Life+is+too+short+for+no+alias&description=Every+alert+needs+a+description&actions=An+action&tags=tag1%2Ctag2&details=key%3Avalue%2Ckey2%3Avalue2&entity=An+example+entity&source=The+source&priority=P1&user=Dracula&note=Here+is+a+note&responders=user%3ATest%2Cteam%3ANOC&visibleTo=user%3AA+User`

The users won't have to escape all the characters, it will handle both, but the generated ones are at least ensured to always be valid URL query params.

Ok fixed. I misunderstood and thought you wanted to preserve the commas, etc. when generating urls.

If you move out the looping from the entity methods to a pair of

Done, is that what you had in mind?

If you move out the looping from the entity methods to a pair of:

That was one mess of a sentence. But yeah, exactly.

This avoids having to initialize PropKeyResolver twice (once when initializing and once when generating a playload)

JonasPf · 2021-02-11T09:36:10Z

Hey, I'm still aware of this and want to finally test it, just haven't had the time yet. It's not forgotten!

Note that this does not affect how the keys are send to the service, just how we generate and parse URLs.

JonasPf · 2021-02-16T13:04:40Z

Ok , so I merged #137 and tested everything again. generate and verify now work. One tiny thing I noticed: formatter lowercases all keys. What this means is that visibleTo becomes visibleto after running generate. This only seems to affect how the user sees the URL. It still gets send to OpsGenie as visibleTo.

I also manually tested sending a bunch of differently configured alerts to OpsGenie. That worked as expected.

Functionality wise I this is ready to be merged from my perspective.

piksel · 2021-02-17T10:35:08Z

Yeah, exactly. This is kind of the same deal as with the commas etc. The generated URLs have the query values in alphabetical order, uses lowercase keys and are escaped. This is mainly for testing purposes, if the format is uniform and known beforehand, black box tests for the serialization is easier to write and update when something changes.

I'll fix the CA issues and merge #137, hopefully this won't have any conflicts after that, in which case I'll merge this as well. Great job!

Co-authored-by: Jonas Pfannschmidt <[email protected]>

piksel · 2021-02-21T16:04:36Z

Solved merge conflicts and merged as #140!

Great job! 🎉

feat: add support for opsgenie

2da7a9c

piksel reviewed Nov 10, 2020

View reviewed changes

pkg/services/opsgenie/opsgenie_json.go Outdated Show resolved Hide resolved

pkg/services/opsgenie/opsgenie_test.go Outdated Show resolved Hide resolved

pkg/services/opsgenie/opsgenie_config.go Outdated Show resolved Hide resolved

simskij self-requested a review November 15, 2020 10:26

Alexei Tighineanu and others added 10 commits December 10, 2020 20:52

Fix for issue containrrr#71 (containrrr#75)

3530a13

docs: add atighineanu as a contributor (containrrr#81)

c78b769

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

added custom port option for rocketchat (containrrr#80)

b67f9b1

fix for containrrr#70 (containrrr#74)

d3c987d

docs: add ellisab as a contributor (containrrr#82)

ca5e1f2

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

do not rewrite channel name without hashes (containrrr#85)

bdbdcf4

Added few testcases for rawURL passed as path/#####channel and path/#…

0f4ed7c

…channel (used to segfault) (containrrr#83) Co-authored-by: Simon Aronsson <[email protected]>

refactor: rename JSON to AlertPayload

54f0f63

JSON is too generic. OpsGenie uses the term "payload" in their documentation so I've used that instead.

refactor: rename plugin to service

c4ba3b9

Shoutrrr uses the term `service` by convention

feat: add support for setting parameters via query string

c108686

piksel requested changes Dec 12, 2020

View reviewed changes

pkg/services/opsgenie/opsgenie_config.go Show resolved Hide resolved

docs/services/opsgenie.md Outdated Show resolved Hide resolved

pkg/services/opsgenie/opsgenie_json.go Show resolved Hide resolved

simskij closed this Dec 20, 2020

simskij reopened this Dec 24, 2020

JonasPf and others added 5 commits December 28, 2020 17:47

Fix query parameter documentation

88d19ae

Co-authored-by: nils måsén <[email protected]>

Implement GetURL

5277afa

Co-authored-by: nils måsén <[email protected]>

Merge remote-tracking branch 'upstream/master' into opsgenie

6bee1e0

Add opsgenie to servicemap

3a4d5c6

Support generating and verifying opsgenie URLs

993bce8

piksel reviewed Jan 28, 2021

View reviewed changes

pkg/services/opsgenie/opsgenie_json.go Outdated Show resolved Hide resolved

piksel approved these changes Jan 28, 2021

View reviewed changes

JonasPf added 3 commits February 2, 2021 21:22

Remove comment about spaces in URL

584deee

Remove comment about spaces; Add test instead

b168211

Merge branch 'main' into opsgenie

33fddea

piksel approved these changes Feb 3, 2021

View reviewed changes

JonasPf and others added 10 commits February 4, 2021 11:14

Fix IFTTT test by removing obsolete values; Add another test for values

59819e7

Merge branch 'main' into opsgenie

a511157

Fix teams test by removing obsolete key without value

d87e5d4

Fix OpsGenie test by using URL encoded strings for space, comma, colon

3145f46

Simplify getURL by using query.Set/Encode instead of diy-ing it

ec73129

Fix test after simplifying getURL, it encodes commas etc. now

cc9d75d

Move newAlertPayload to Service struct

6390b12

This avoids having to initialize PropKeyResolver twice (once when initializing and once when generating a playload)

Preparation for implementing an interface for de-serializing structs

8f3aac4

Fix linting issues

3305147

feat: add support for struct and map fields

7bd4ba0

piksel mentioned this pull request Feb 7, 2021

feat: add support for struct and map fields #137

Merged

JonasPf added 5 commits February 16, 2021 11:30

Fix: Use default port if no port was specified

2fc0dbc

Merge branch 'feature/format-struct-map-fields' into opsgenie

7e19f85

Remove custom serialization logic; use formatters functionality instead

26bd837

Fix: Expect default port (443) in test

38c1f10

Fix test: formatter lowercases keys

f0c460d

Note that this does not affect how the keys are send to the service, just how we generate and parse URLs.

piksel mentioned this pull request Feb 21, 2021

feat: add support for opsgenie #140

Merged

piksel added a commit that referenced this pull request Feb 21, 2021

feat: add support for opsgenie (#140) (#73)

8aa6f46

Co-authored-by: Jonas Pfannschmidt <[email protected]>

piksel closed this Feb 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for opsgenie #73

feat: add support for opsgenie #73

JonasPf commented Nov 10, 2020

codecov-io commented Nov 10, 2020 •

edited by codecov bot

Loading

piksel left a comment

JonasPf commented Nov 12, 2020

JonasPf commented Dec 10, 2020

piksel commented Dec 12, 2020

piksel left a comment

simskij commented Dec 20, 2020 •

edited

Loading

JonasPf commented Dec 24, 2020 •

edited

Loading

simskij commented Dec 24, 2020

JonasPf commented Dec 28, 2020

JonasPf commented Jan 6, 2021

piksel left a comment

JonasPf commented Feb 3, 2021

piksel left a comment

piksel Feb 3, 2021

JonasPf Feb 4, 2021

JonasPf Feb 4, 2021

piksel Feb 4, 2021 •

edited

Loading

JonasPf commented Feb 11, 2021

JonasPf commented Feb 16, 2021

piksel commented Feb 17, 2021

piksel commented Feb 21, 2021

	queryParams := `alias=Life+is+too+short+for+no+alias&description=Every+alert+needs+a+description&actions=An+action&tags=tag1,tag2&details=key:value,key2:value2&entity=An+example+entity&source=The+source&priority=P1&user=Dracula&note=Here+is+a+note&responders=user:Test,team:NOC&visibleTo=user:A+User`
	queryParams := `alias=Life+is+too+short+for+no+alias&description=Every+alert+needs+a+description&actions=An+action&tags=tag1%2Ctag2&details=key%3Avalue%2Ckey2%3Avalue2&entity=An+example+entity&source=The+source&priority=P1&user=Dracula&note=Here+is+a+note&responders=user%3ATest%2Cteam%3ANOC&visibleTo=user%3AA+User`

feat: add support for opsgenie #73

feat: add support for opsgenie #73

Conversation

JonasPf commented Nov 10, 2020

codecov-io commented Nov 10, 2020 • edited by codecov bot Loading

Codecov Report

piksel left a comment

Choose a reason for hiding this comment

JonasPf commented Nov 12, 2020

JonasPf commented Dec 10, 2020

piksel commented Dec 12, 2020

piksel left a comment

Choose a reason for hiding this comment

simskij commented Dec 20, 2020 • edited Loading

JonasPf commented Dec 24, 2020 • edited Loading

simskij commented Dec 24, 2020

JonasPf commented Dec 28, 2020

JonasPf commented Jan 6, 2021

piksel left a comment

Choose a reason for hiding this comment

JonasPf commented Feb 3, 2021

piksel left a comment

Choose a reason for hiding this comment

piksel Feb 3, 2021

Choose a reason for hiding this comment

JonasPf Feb 4, 2021

Choose a reason for hiding this comment

JonasPf Feb 4, 2021

Choose a reason for hiding this comment

piksel Feb 4, 2021 • edited Loading

Choose a reason for hiding this comment

JonasPf commented Feb 11, 2021

JonasPf commented Feb 16, 2021

piksel commented Feb 17, 2021

piksel commented Feb 21, 2021

codecov-io commented Nov 10, 2020 •

edited by codecov bot

Loading

simskij commented Dec 20, 2020 •

edited

Loading

JonasPf commented Dec 24, 2020 •

edited

Loading

piksel Feb 4, 2021 •

edited

Loading