Watcher: migrate PagerDuty v1 events API to v2 API #32285

hub-cap · 2018-07-23T15:35:01Z

The PagerDuty v1 API is EOL and will stop accepting new accounts
shortly. This commit swaps out the watcher use of the v1 API with the
new v2 API. It does not change anything about the existing watcher
API.

Closes #32243

The PagerDuty v1 API is EOL and will stop accepting new accounts shortly. This commit swaps out the watcher use of the v1 API with the new v2 API. It does not change anything about the existing watcher API. Closes elastic#32243

elasticmachine · 2018-07-23T15:35:03Z

Pinging @elastic/es-core-infra

spinscale

Looks like much fewer changes than expected. Testing with a v2 token needs to be done as well.

Do you think it's worth adding some tests?

spinscale · 2018-07-23T16:08:40Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java

+                            }
+
+                            // TODO externalize this into something user editable
+                            builder.field("source", "watcher");


maybe have the watch id in here (if possible) to easily pinpoint the source?

spinscale · 2018-07-23T16:09:35Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java

+
+                            if (attachPayload) {
+                                builder.startObject("custom_details");
+                                builder.field(Fields.PAYLOAD.getPreferredName());


replace two lines with

builder.field(Fields.PAYLOAD.getPreferredName(), payload, params);

spinscale · 2018-07-23T16:13:55Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java

+                        {
+                            builder.field("summary", description);
+
+                            if (attachPayload) {


Just to make sure: This gets parsed properly on the other side if you leave it out (and dont have the custom_details structure)?

custom_details is not mandatory, so it will def leave it out. Ill add a null check for payload as well in this statement

…r_pdv2

hub-cap · 2018-07-25T01:05:42Z

implemented the changes. Yes, ill do some manual verification once i obtain a v2 token. Also, what kind of test did you have in mind besides the 3rd party ones? Thats what i was using to verify up to now. cc @spinscale

hub-cap · 2018-07-25T14:41:39Z

@elasticmachine test this please

spinscale · 2018-07-27T07:58:21Z

testing was mostly referred to proper JSON construction (check if fields are not written if certain elements are null etc).

…t emits valid json

hub-cap · 2018-08-01T17:11:00Z

...c/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEventContext.java

@@ -92,6 +92,85 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
        return builder.endObject();
    }

+    public static IncidentEventContext parse(XContentParser parser) throws IOException {


These are straight copys of the Template's parse method, but without the TextTemplate mess. These will be used in the future when i finally finish removing the Templates :)

spinscale · 2018-08-02T06:58:35Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java

@@ -39,7 +39,8 @@
 public class IncidentEvent implements ToXContentObject {


maybe also fix the above links, as they probably link to the old documentation?

spinscale · 2018-08-02T07:12:44Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java

                .build();
    }

+    public XContentBuilder buildAPIXContent(XContentBuilder builder, Params params, String serviceKey,


package visibility?

spinscale · 2018-08-02T07:12:56Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java

@@ -93,46 +94,96 @@ public int hashCode() {
        return result;
    }

-    public HttpRequest createRequest(final String serviceKey, final Payload payload) throws IOException {
+    public HttpRequest createRequest(final String serviceKey, final Payload payload, final String watchId) throws IOException {


packabe visibility?

spinscale · 2018-08-02T07:31:58Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java

+     */
+    private void toXContentV2Contexts(XContentBuilder builder, ToXContent.Params params,
+                                      IncidentEventContext[] contexts) throws IOException {
+        // contexts can be either links or images, and the v2 api needs them separate


how about making this more readable (at least to me :-)

Map<IncidentEventContext.Type, List<IncidentEventContext>> groupByType = Arrays.stream(contexts).collect(Collectors.groupingBy(ctx -> ctx.type)); List<IncidentEventContext> linkContexts = groupByType.get(IncidentEventContext.Type.LINK); if (linkContexts != null && linkContexts.isEmpty() == false) { builder.startArray(Fields.LINKS.getPreferredName()); for (IncidentEventContext context : linkContexts) { context.toXContent(builder, params); } builder.endArray(); } List<IncidentEventContext> imagesContexts = groupByType.get(IncidentEventContext.Type.IMAGE); if (imagesContexts != null && imagesContexts.isEmpty() == false) { builder.startArray(Fields.IMAGES.getPreferredName()); for (IncidentEventContext context : imagesContexts) { context.toXContent(builder, params); } builder.endArray(); }

dang i like those streams.. I still am a streaming noob. ty.

spinscale · 2018-08-02T07:42:23Z

...src/test/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEventTests.java

+
+        String currentFieldName = null;
+        XContentParser.Token token;
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {


that's a lot of parsing code, can you use the ObjectPath class instead. Going through this while loop in a test is pretty tedious to understand what's going on.

ObjectPath objectPath = ObjectPath.createFromXContent(jsonBuilder.contentType().xContent(), BytesReference.bytes(jsonBuilder)); actualServiceKey = objectPath.evaluate(IncidentEvent.Fields.ROUTING_KEY.getPreferredName()); ...

oh this is way nicer. tyvm.

spinscale

I left some more comments regarding readability, but I think we're almost here.

spinscale · 2018-08-02T16:38:57Z

...src/test/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEventTests.java

+
+        List<IncidentEventContext> actualLinks = new ArrayList<>();
+        List<Map<String, String>> linkMap = (List<Map<String, String>>) objectPath.evaluate(IncidentEvent.Fields.LINKS.getPreferredName());
+        if (linkMap != null) {


assertThat(linkMap, is(notNullValue))

if the map is not always non-null, then check this implicitely by checking another boolean

It can be null if the links are not present. They are randomly set to null in the setup code. So this can totally come back as null, which in that case it will still evaluate the images/links empty List's against the setup's empty Lists, ensuring they are the same.

spinscale · 2018-08-02T16:39:11Z

...src/test/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEventTests.java

+
+        List<IncidentEventContext> actualImages = new ArrayList<>();
+        List<Map<String, String>> imgMap = (List<Map<String, String>>) objectPath.evaluate(IncidentEvent.Fields.IMAGES.getPreferredName());
+        if (imgMap != null) {


assertThat(imgMap, is(notNullValue))

spinscale

left minor comments to improve the test, but LGTM otherwise

danielmitterdorfer · 2018-08-09T14:37:15Z

Removing the "blocker" status due to #32243 (comment).

…e-types * elastic/master: (199 commits) Watcher: Remove unused hipchat render method (elastic#32211) Watcher: Remove extraneous auth classes (elastic#32300) Watcher: migrate PagerDuty v1 events API to v2 API (elastic#32285) [TEST] Select free port for Minio (elastic#32837) MINOR: Remove `IndexTemplateFilter` (elastic#32841) Core: Add java time version of rounding classes (elastic#32641) Aggregations/HL Rest client fix: missing scores (elastic#32774) HLRC: Add Delete License API (elastic#32586) INGEST: Create Index Before Pipeline Execute (elastic#32786) Fix NOOP bulk updates (elastic#32819) Remove client connections from TcpTransport (elastic#31886) Increase logging testRetentionPolicyChangeDuringRecovery AwaitsFix case-functions.sql-spec Mute security-cli tests in FIPS JVM (elastic#32812) SCRIPTING: Support BucketAggScript return null (elastic#32811) Unmute WildFly tests in FIPS JVM (elastic#32814) [TEST] Force a stop to save rollup state before continuing (elastic#32787) [test] disable packaging tests for suse boxes Mute IndicesRequestIT#testBulk [ML][DOCS] Refer to rules feature as custom rules (elastic#32785) ...

…listeners * elastic/master: Watcher: Remove unused hipchat render method (elastic#32211) Watcher: Remove extraneous auth classes (elastic#32300) Watcher: migrate PagerDuty v1 events API to v2 API (elastic#32285) [TEST] Select free port for Minio (elastic#32837) MINOR: Remove `IndexTemplateFilter` (elastic#32841) Core: Add java time version of rounding classes (elastic#32641) Aggregations/HL Rest client fix: missing scores (elastic#32774) HLRC: Add Delete License API (elastic#32586) INGEST: Create Index Before Pipeline Execute (elastic#32786) Fix NOOP bulk updates (elastic#32819) Remove client connections from TcpTransport (elastic#31886) Increase logging testRetentionPolicyChangeDuringRecovery AwaitsFix case-functions.sql-spec Mute security-cli tests in FIPS JVM (elastic#32812) SCRIPTING: Support BucketAggScript return null (elastic#32811) Unmute WildFly tests in FIPS JVM (elastic#32814) [TEST] Force a stop to save rollup state before continuing (elastic#32787) [test] disable packaging tests for suse boxes Mute IndicesRequestIT#testBulk [ML][DOCS] Refer to rules feature as custom rules (elastic#32785)

* elastic/master: Revert "cluster formation DSL - Gradle integration - part 2 (#32028)" (#32876) cluster formation DSL - Gradle integration - part 2 (#32028) Introduce global checkpoint listeners (#32696) Move connection profile into connection manager (#32858) [ML] Temporarily disabling rolling-upgrade tests Use generic AcknowledgedResponse instead of extended classes (#32859) [ML] Removing old per-partition normalization code (#32816) Use JDK 10 for 6.4 BWC builds (#32866) Removed flaky test. Looks like randomisation makes these assertions unreliable. [test] mute IndexShardTests.testDocStats Introduce the dissect library (#32297) Security: remove password hash bootstrap check (#32440) Move validation to server for put user requests (#32471) [ML] Add high level REST client docs for ML put job endpoint (#32843) Test: Fix forbidden uses in test framework (#32824) Painless: Change fqn_only to no_import (#32817) [test] mute testSearchWithSignificantTermsAgg Watcher: Remove unused hipchat render method (#32211) Watcher: Remove extraneous auth classes (#32300) Watcher: migrate PagerDuty v1 events API to v2 API (#32285)

The PagerDuty v1 API is EOL and will stop accepting new accounts shortly. This commit swaps out the watcher use of the v1 API with the new v2 API. It does not change anything about the existing watcher API. Closes #32243

Watcher: migrate PagerDuty v1 events API to v2 API

9705705

The PagerDuty v1 API is EOL and will stop accepting new accounts shortly. This commit swaps out the watcher use of the v1 API with the new v2 API. It does not change anything about the existing watcher API. Closes elastic#32243

hub-cap added >bug blocker critical v7.0.0 :Data Management/Watcher v6.4.0 v5.6.11 labels Jul 23, 2018

spinscale reviewed Jul 23, 2018

View reviewed changes

hub-cap added 4 commits July 24, 2018 19:56

Merge branch 'master' of github.com:elastic/elasticsearch into watche…

155b036

…r_pdv2

Add watchID to source

426c81d

making the payload check better

003896a

Cleanup xcontent

d3e5921

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

1166ec0

hub-cap added 2 commits July 25, 2018 11:28

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

942e62d

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

fdab5e3

hub-cap added 6 commits July 30, 2018 11:00

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

4012f03

Fix test changing due to contexts becoming links/images

6644f2b

Move payload generation for PD API to a helper method and test that i…

71629ae

…t emits valid json

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

c67d742

Precommit header fix

fa1fc07

Removing commented code

dc7edb5

hub-cap commented Aug 1, 2018

View reviewed changes

Remove commented code

a0e68bb

spinscale reviewed Aug 2, 2018

View reviewed changes

spinscale suggested changes Aug 2, 2018

View reviewed changes

hub-cap added 2 commits August 2, 2018 10:58

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

f4ee457

Cleanup test and use streams in one method

6a23d31

spinscale reviewed Aug 2, 2018

View reviewed changes

spinscale approved these changes Aug 2, 2018

View reviewed changes

danielmitterdorfer removed the blocker label Aug 9, 2018

hub-cap added v6.5.0 >enhancement and removed v5.6.11 v6.4.0 critical >bug labels Aug 13, 2018

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

ade99d6

hub-cap merged commit 4c90a61 into elastic:master Aug 14, 2018

hub-cap added the backport pending label Aug 14, 2018

hub-cap removed the backport pending label Aug 15, 2018

Benny-Git mentioned this pull request Sep 26, 2018

Watcher: Allow defining PagerDuty severity field #34073

Open

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

This was referenced Oct 11, 2022

Support all event fields in the Pagerduty action. elastic/kibana#63933

Closed

Documentation for pagerduty watcher action needs updating re PagerDuty Events API v2 #90799

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Watcher: migrate PagerDuty v1 events API to v2 API #32285

Watcher: migrate PagerDuty v1 events API to v2 API #32285

hub-cap commented Jul 23, 2018

elasticmachine commented Jul 23, 2018

spinscale left a comment

spinscale Jul 23, 2018

spinscale Jul 23, 2018

spinscale Jul 23, 2018

hub-cap Jul 25, 2018

hub-cap commented Jul 25, 2018 •

edited

Loading

hub-cap commented Jul 25, 2018

spinscale commented Jul 27, 2018

hub-cap Aug 1, 2018

spinscale Aug 2, 2018

spinscale Aug 2, 2018

spinscale Aug 2, 2018

spinscale Aug 2, 2018

hub-cap Aug 2, 2018

spinscale Aug 2, 2018

hub-cap Aug 2, 2018

spinscale left a comment

spinscale Aug 2, 2018

spinscale Aug 2, 2018

hub-cap Aug 2, 2018 •

edited

Loading

spinscale Aug 2, 2018

spinscale left a comment

danielmitterdorfer commented Aug 9, 2018

		@@ -39,7 +39,8 @@
		public class IncidentEvent implements ToXContentObject {

Watcher: migrate PagerDuty v1 events API to v2 API #32285

Watcher: migrate PagerDuty v1 events API to v2 API #32285

Conversation

hub-cap commented Jul 23, 2018

elasticmachine commented Jul 23, 2018

spinscale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hub-cap commented Jul 25, 2018 • edited Loading

hub-cap commented Jul 25, 2018

spinscale commented Jul 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spinscale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hub-cap Aug 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spinscale left a comment

Choose a reason for hiding this comment

danielmitterdorfer commented Aug 9, 2018

hub-cap commented Jul 25, 2018 •

edited

Loading

hub-cap Aug 2, 2018 •

edited

Loading