fix PagerDuty timestamp validation

[ersin-erdal]

fixes: #116173

PagerDuty requires ISO 8601 format for the timestamp, native JS Date.parse method does not support some ISO-8601 formats. With this PR, we use moment for date validation.

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[ymao1]

Nice job adding this validation to the server side! We should also fix it on the client side, where validation occurs in x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/pagerduty/pagerduty.tsx

[ersin-erdal]

After a discussion with Patrick, i changed the code a bit.
Now, it accepts all the valid date formats but converts it to ISO 8601 before sending it to PagerDuty.
In order to be consistent, Implemented the same moment validation method in frontend too.

[ymao1]

When I try 2021-10-22 5:01:32, I am getting this warning:

Deprecation warning: value provided is not in a recognized RFC2822 or ISO format. moment construction falls back to js Date(), which is not reliable across all browsers and versions. Non RFC2822/ISO date formats are discouraged and will be removed in an upcoming major release. Please refer to http://momentjs.com/guides/#/warnings/js-date/ for more info.
Arguments:
[0] _isAMomentObject: true, _isUTC: false, _useUTC: false, _l: undefined, _i: 2021-10-22 5:01:32, _f: undefined, _strict: undefined, _locale: [object Object]

Although I see that it is formatted correctly as 2021-10-22T09:01:32.000Z in the PagerDuty payload. Are we concerned about this deprecation warning at all?

[ersin-erdal]

Yes i saw that warning to, probably the single digit hour (5) caused it.
It's discouraged to pass non ISO formatted date to moment. https://momentjs.com/guides/#/warnings/js-date/
Maybe we should force users to enter a valid ISO formatted date?

[ymao1]

I like not forcing users to format their own timestamps and handling the formatting ourselves but just worried this deprecation will eventually turn into an error. WDYT @pmuellr?

[pmuellr]

I like not forcing users to format their own timestamps and handling the formatting ourselves but just worried this deprecation will eventually turn into an error. WDYT

Ya, it's an SDH generator :-)

I'm wondering if parsing with just Date.parse() would be good enough? These dates are all coming from ES fields, but I guess they could be in different formats stored there. Or are there other options in moment to not have that message displayed?

[ersin-erdal]

That deprecation warning is shown when a non-ISO date string is passed to moment such as: moment("non-iso-date-string"), then moment fall backs to: moment(new Date("non-iso-date-string")). But that was discussed and the warning was added in 2014! See the original discussion: moment/moment#1407
After that, moment decided to not to release any major changes (basically it's done, we (kibana) better to switch to a new library such as date-fns) See the project status: https://momentjs.com/docs/#/-project-status/

So, i don't think that the fall back will be deprecated. And since the code is consistent and working as expected, we can merge this PR.

[ymao1]

So, i don't think that the fall back will be deprecated. And since the code is consistent and working as expected, we can merge this PR.

That's fair :)

[ymao1]

LGTM! Nice job

[ersin-erdal]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: afb0bd2
• Storybooks Preview

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
triggersActionsUi	53.9KB	53.9KB	-3.0B

History

• 💚 Build #15971 succeeded 4123b1b
• 💚 Build #15861 succeeded b501aec

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[pmuellr]

LGTM!

Regarding the discussion of the deprecation message in moment, this may actually be useful, in case we ever hit it and wonder why a date may have been generated the way it was generated. Hopefully it would show up in the logs, but I imagine it's being written directly to stdout, so may not show up in all logging scenarios ...

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation