[Synthetics] add default email recovery message #154862

dominiqueclarke · 2023-04-12T18:16:45Z

Summary

Down email

Recovered email

Please note the recoverd typo in the screenshot has been addressed in commits 7c4040f and 56f637b

Testing

This PR is extremely tricky to test. I have deployed this branch to my own cloud account. This ensures that I can control the allowlist setting for emails on cloud. If you want, I can give you the cloud cluster where this branch is hosted and if you give me your email I can add you to my allowlist settings for you to test.

The cloud cluster is current on commit f889500

apmmachine · 2023-04-12T18:17:05Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

x-pack/plugins/synthetics/common/rules/synthetics/translations.ts

x-pack/plugins/synthetics/common/rules/alert_actions.test.ts

elasticmachine · 2023-04-18T23:56:17Z

Pinging @elastic/uptime (Team:uptime)

kibana-ci · 2023-04-19T00:52:52Z

💚 Build Succeeded

Buildkite Build
Commit: 56f637b

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`synthetics`	1227	1228	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`synthetics`	1.2MB	1.2MB	+364.0B

Page load bundle

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

id	before	after	diff
`synthetics`	27.0KB	27.3KB	+286.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	432	435	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	512	515	+3

History

💚 Build #120695 succeeded f889500
💛 Build #119952 was flaky f251c02
💔 Build #119925 failed 3be2644
💔 Build #119916 failed 6d4e3f89dea7f401c1bd7437b34c11500908b84b
💔 Build #119900 failed e42dc2b
💛 Build #119684 was flaky 860c54d

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

shahzad31

LGTM !!

florent-leborgne

Hi, I've suggested a few edits to make some of the messages more conversational.

Feel free to ignore those that are not relevant, especially the suggested changes from "the alert has recovered" to "the alert is no longer active" that might be less accurate. I'm not 100% sure an alert itself can recover, but rather the monitor that triggered the alert, so I tried to find a workaround to that phrasing.

florent-leborgne · 2023-04-19T08:32:37Z

x-pack/plugins/synthetics/common/rules/alert_actions.test.ts

+            text: '',
+          },
+          message:
+            'Alert for monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} has recovered',


Suggested change

'Alert for monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} has recovered',

'The monitor {{context.monitorName}} checking {{{context.monitorUrl}}} from {{context.observerLocation}} has recovered. The alert is no longer active.',

Trying to be more conversational, feel free to ignore if not relevant here.

florent-leborgne · 2023-04-19T08:33:14Z

x-pack/plugins/synthetics/common/rules/alert_actions.test.ts

+          message:
+            'Alert for monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} has recovered',
+          subject:
+            'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} has recovered',


Suggested change

'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} has recovered',

'The monitor {{context.monitorName}} checking {{{context.monitorUrl}}} has recovered',

florent-leborgne · 2023-04-19T08:35:34Z

x-pack/plugins/synthetics/common/rules/alert_actions.test.ts

+            text: '',
+          },
+          message:
+            'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} {{{context.statusMessage}}} The latest error message is {{{context.latestErrorMessage}}}, checked at {{context.checkedAt}}',


Suggested change

'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} {{{context.statusMessage}}} The latest error message is {{{context.latestErrorMessage}}}, checked at {{context.checkedAt}}',

'The monitor {{context.monitorName}} checking {{{context.monitorUrl}}} from {{context.observerLocation}} {{{context.statusMessage}}} The latest error message is {{{context.latestErrorMessage}}}, at {{context.checkedAt}}',

florent-leborgne · 2023-04-19T08:36:01Z

x-pack/plugins/synthetics/common/rules/alert_actions.test.ts

+          },
+          message:
+            'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} {{{context.statusMessage}}} The latest error message is {{{context.latestErrorMessage}}}, checked at {{context.checkedAt}}',
+          subject: 'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} is down',


Suggested change

subject: 'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} is down',

subject: 'The monitor {{context.monitorName}} checking {{{context.monitorUrl}}} is down',

florent-leborgne · 2023-04-19T08:36:27Z

x-pack/plugins/synthetics/common/rules/alert_actions.test.ts

+            eventAction: 'trigger',
+            severity: 'error',
+            summary:
+              'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} {{{context.statusMessage}}} The latest error message is {{{context.latestErrorMessage}}}',


Suggested change

'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} {{{context.statusMessage}}} The latest error message is {{{context.latestErrorMessage}}}',

'The monitor {{context.monitorName}} checking {{{context.monitorUrl}}} from {{context.observerLocation}} {{{context.statusMessage}}} The latest error message is {{{context.latestErrorMessage}}}',

florent-leborgne · 2023-04-19T08:39:47Z

x-pack/plugins/synthetics/common/rules/legacy_uptime/translations.ts

+    'xpack.synthetics.alerts.monitorStatus.defaultRecoveryMessage',
+    {
+      defaultMessage:
+        'Alert for monitor {monitorName} with url {monitorUrl} from {observerLocation} has recovered',


Suggested change

'Alert for monitor {monitorName} with url {monitorUrl} from {observerLocation} has recovered',

'The alert for the monitor {monitorName} checking {monitorUrl} from {observerLocation} is no longer active',

florent-leborgne · 2023-04-19T08:41:59Z

x-pack/plugins/synthetics/common/rules/legacy_uptime/translations.ts

+    },
+  }),
+  defaultRecoveryMessage: i18n.translate('xpack.synthetics.alerts.tls.defaultRecoveryMessage', {
+    defaultMessage: `Alert for TLS certificate {commonName} from issuer {issuer} has recovered`,


Suggested change

defaultMessage: `Alert for TLS certificate {commonName} from issuer {issuer} has recovered`,

defaultMessage: `The alert for TLS certificate {commonName} from issuer {issuer} is no longer active`,

Feel free to ignore my changes from "has recovered" to "is no longer active". Since the subject is "the alert", it sounds strange to me that an alert recovers, so suggesting an alternative if "the alert" has to remain the main subject. This comment also applies to similar text starting with "the alert".

florent-leborgne · 2023-04-19T08:44:04Z

x-pack/plugins/synthetics/common/rules/legacy_uptime/translations.ts

+  defaultActionMessage: i18n.translate(
+    'xpack.synthetics.alerts.durationAnomaly.defaultActionMessage',
+    {
+      defaultMessage: `Abnormal ({severity} level) response time detected on {monitor} with url {monitorUrl} at {anomalyStartTimestamp}. Anomaly severity score is {severityScore}.


Suggested change

defaultMessage: `Abnormal ({severity} level) response time detected on {monitor} with url {monitorUrl} at {anomalyStartTimestamp}. Anomaly severity score is {severityScore}.

defaultMessage: `Abnormal ({severity} level) response time detected on {monitor} on the URL {monitorUrl} at {anomalyStartTimestamp}. The anomaly severity score is {severityScore}.

florent-leborgne · 2023-04-19T08:44:38Z

x-pack/plugins/synthetics/common/rules/legacy_uptime/translations.ts

+    'xpack.synthetics.alerts.durationAnomaly.defaultActionMessage',
+    {
+      defaultMessage: `Abnormal ({severity} level) response time detected on {monitor} with url {monitorUrl} at {anomalyStartTimestamp}. Anomaly severity score is {severityScore}.
+Response times as high as {slowestAnomalyResponse} have been detected from location {observerLocation}. Expected response time is {expectedResponseTime}.`,


Suggested change

Response times as high as {slowestAnomalyResponse} have been detected from location {observerLocation}. Expected response time is {expectedResponseTime}.`,

Response times as high as {slowestAnomalyResponse} have been detected from {observerLocation}. The expected response time is {expectedResponseTime}.`,

florent-leborgne · 2023-04-19T08:49:22Z

x-pack/plugins/synthetics/common/rules/legacy_uptime/translations.ts

+  defaultRecoveryMessage: i18n.translate(
+    'xpack.synthetics.alerts.durationAnomaly.defaultRecoveryMessage',
+    {
+      defaultMessage: `Alert for abnormal ({severity} level) response time detected on monitor {monitor} with url {monitorUrl} from location {observerLocation} at {anomalyStartTimestamp} has recovered`,


Suggested change

defaultMessage: `Alert for abnormal ({severity} level) response time detected on monitor {monitor} with url {monitorUrl} from location {observerLocation} at {anomalyStartTimestamp} has recovered`,

defaultMessage: `The alert for abnormal ({severity} level) response time detected at {anomalyStartTimestamp} on the monitor {monitor} checking {monitorUrl} from {observerLocation} is no longer active.`,

Wondering if we couldn't say something simpler like "The abnormal response time detected at {anomalyStartTimestamp} on the monitor {monitor} is now back to normal."

dominiqueclarke · 2023-04-19T13:04:13Z

x-pack/plugins/synthetics/common/rules/synthetics/translations.ts

@@ -33,6 +33,16 @@ export const SyntheticsMonitorStatusTranslations = {
      },
    }
  ),
+  defaultRecoverySubjectMessage: i18n.translate(


@florent-leborgne

The translations you reviewed are all part of the legacy Uptime app. It would be helpful if you could review the translations in this file, which contain our new translations. I would honestly be extremely grateful if you reviewed the entire file, as we've discussed internally that the content needs work https://github.com/elastic/kibana/blob/56f637b82edf81b7ab6dafd54c197f1b945c487b/x-pack/plugins/synthetics/common/rules/synthetics/translations.ts

I'd happily do that. What do you think should be improved in this file? It looks fairly ok to me though I have a few ideas to simplify, but I'd also like to know what you've discussed internally :)

I think one of the things that came up internally was that the subject line for email connectors read weird.

kibana/x-pack/plugins/synthetics/common/rules/synthetics/translations.ts

Line 29 in 56f637b

defaultMessage: 'The monitor {monitorName} checking {monitorUrl} is down.',

. It's different than the legacy Uptime one. Perhaps I should take a look at your feedback for the legacy Uptime subject line. Personally The monitor vs Your monitor sounds strange to me.

Ultimately though, these translations were merged in without any content review so we're kinda behind and want to make sure it gets reviewed before GA.

I can try to set up some time to compare them side by side and make them more consistent with each other? I'd just need to know the precise files to compare and tweak to submit a PR, if you can help me with that. I'd hopefully be able to do that by the end of this week or early next week.

@florent-leborgne Sure thing. A few things to clarify though:

I don't think we're concerned with them being more consistent. Uptime is going away and if there are better translations for Synthetics, we're happy to go with that without updating Uptime.

I see when you reviewed Uptime, you did use The monitor in the subject line, which is what Synthetics has currently. I'm happy to go with this as I was likely the only one who thought it sounded weird and it's clear to me now that the phrase is approved by you.

Given that we don't need consistency between Uptime and Synthetics, you don't need to put up a PR to make the two consistent if you don't feel it's necessary, so long as the synthetics/translations.ts file meets your standards we're happy.

If you do still want to compare and put up a PR, the two files are

kibana/x-pack/plugins/synthetics/common/rules/synthetics/translations.ts

Line 29 in 56f637b

defaultMessage: 'The monitor {monitorName} checking {monitorUrl} is down.',

and https://github.com/elastic/kibana/blob/56f637b82edf81b7ab6dafd54c197f1b945c487b/x-pack/plugins/synthetics/common/rules/legacy_uptime/translations.ts (Please note, there are more translations in legacy_uptime/translations because there are more rule types in Uptime)

As far as what I'll do with this PR moving forward:

The translations you originally reviewed are actually from legacy Uptime and have been established for a while now. I'm not 100% sure if we want to change them. I'll go ahead and check with our team lead and if he approves I will apply those changes to legacy Uptime, otherwise I will not apply the changes.

Since you are volunteering to put up a PR and don't have any pressing suggestions for the synthetics/translations.ts file, I will go ahead and merge this within the next few hours (need to do some more testing myself) and allow you to put up a secondary PR (if you desire, I mentioned above that may not be necessary).

Ok, I reviewed all content I could find in the PR. Feel free to not apply my suggestions if those are for the part of the feature that is going away soon :)

I'm pretty OK with what I saw in the translations file, so we could maybe keep it this way for now, until we get more data or more time to maybe try to simplify it? I'm totally ok to replace "The monitor" with "Your monitor". It also sounds good to me.

Let me know what you think.

## Summary Resolves elastic#153891 <img width="1334" alt="Screen Shot 2023-04-18 at 7 50 20 PM" src="https://user-images.githubusercontent.com/11356435/232928889-09de824f-14ef-426d-8732-6da7fc12eb37.png"> Down email Recovered email <img width="1339" alt="Screen Shot 2023-04-18 at 7 50 30 PM" src="https://user-images.githubusercontent.com/11356435/232928874-4e7c0cbb-b7ec-4684-b0d2-88ec993697df.png"> Please note the `recoverd` typo in the screenshot has been addressed in commits [7c4040f](elastic@7c4040f) and [56f637b](elastic@56f637b) ### Testing This PR is extremely tricky to test. I have deployed this branch to my own cloud account. This ensures that I can control the allowlist setting for emails on cloud. If you want, I can give you the cloud cluster where this branch is hosted and if you give me your email I can add you to my allowlist settings for you to test. The cloud cluster is current on commit [f889500](elastic@f889500) (cherry picked from commit 0b71c74)

) # Backport This will backport the following commits from `main` to `8.7`: - [Synthetics] add default email recovery message (#154862) (0b71c74)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Dominique Clarke <dominique.clarke@elastic.co>

* 8.7: (93 commits) [8.7] [Controls] Use EUI Selectable for Field search (#151231) (#155454) [8.7] [Synthetics] Fix performance breakdown link from error details page (#155393) (#155427) [8.7] [DOCS] Remove or move book-scoped attributes (#155210) (#155426) [8.7] [Synthetics] add default email recovery message (#154862) (#155418) [8.7] [Uptime] Add both both ip filters for view host in uptime location for host and monitor (#155382) (#155399) [8.7] Setup Node.js environment before instrumenting Kibana with APM. (#155063) (#155300) [8.7] [Discover] Address react warnings for legacy table (#154579) (#155345) [8.7] [Fleet] Fix logs useless rerender (#155305) (#155310) [8.7] [kbn-failed-test-reporter-cli] truncate report message to fix github api call failure (#155141) (#155286) [8.7][APM] Fleet migration support for bundled APM package (#153159) (#155281) [8.7] [Enterprise Search] Fix Connector scheduling show week information correctly (#155191) (#155227) [8.7] [Synthetics] Fix pending count in case of location filtering (#155200) (#155225) [8.7] [Controls] Add Expensive Queries Fallback (#155082) (#155189) [8.7] [data view field editor] Runtime field code editor - move state out of controller (#155107) (#155150) [8.7] [FullStory] Update snippet (#153570) (#155138) [8.7] [Security Solution][Exceptions] - Fix exception operator logic when mapping conflict (#155071) (#155094) [DOCS] Adds 8.7.1 release notes (#154844) [8.7] Sync bundled packages with Package Storage (#155042) [APM] plugin description (#154811) Update api.asciidoc (#155021) ...

synthetics - add default email recovery message

2bc218e

dominiqueclarke added 3 commits April 13, 2023 05:18

adjust recovery action logic

860c54d

add dynamic subject line for recovery and add tests

e42dc2b

adjust types

3be2644

dominiqueclarke force-pushed the fix/synthetics-default-email-recovery branch from 6d4e3f8 to 3be2644 Compare April 13, 2023 18:39

dominiqueclarke added 2 commits April 13, 2023 16:21

Merge branch 'main' into fix/synthetics-default-email-recovery

f251c02

Merge branch 'main' into fix/synthetics-default-email-recovery

f889500

dominiqueclarke added bug Fixes for quality problems that affect the customer experience Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.8.0 labels Apr 18, 2023

dominiqueclarke commented Apr 18, 2023

View reviewed changes

x-pack/plugins/synthetics/common/rules/synthetics/translations.ts Outdated Show resolved Hide resolved

dominiqueclarke commented Apr 18, 2023

View reviewed changes

x-pack/plugins/synthetics/common/rules/alert_actions.test.ts Outdated Show resolved Hide resolved

dominiqueclarke added 2 commits April 18, 2023 19:54

Update x-pack/plugins/synthetics/common/rules/alert_actions.test.ts

7c4040f

Update x-pack/plugins/synthetics/common/rules/synthetics/translations.ts

56f637b

dominiqueclarke added the release_note:skip Skip the PR/issue when compiling release notes label Apr 18, 2023

dominiqueclarke requested a review from florent-leborgne April 18, 2023 23:56

dominiqueclarke marked this pull request as ready for review April 18, 2023 23:56

dominiqueclarke requested a review from a team as a code owner April 18, 2023 23:56

dominiqueclarke changed the title ~~synthetics - add default email recovery message~~ [Synthetics] add default email recovery message Apr 18, 2023

shahzad31 approved these changes Apr 19, 2023

View reviewed changes

florent-leborgne reviewed Apr 19, 2023

View reviewed changes

dominiqueclarke commented Apr 19, 2023

View reviewed changes

dominiqueclarke added the v8.7.2 label Apr 20, 2023

dominiqueclarke merged commit 0b71c74 into elastic:main Apr 20, 2023

dominiqueclarke deleted the fix/synthetics-default-email-recovery branch April 20, 2023 14:51

kibanamachine mentioned this pull request Apr 20, 2023

[8.7] [Synthetics] add default email recovery message (#154862) #155418

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Synthetics] add default email recovery message #154862

[Synthetics] add default email recovery message #154862

dominiqueclarke commented Apr 12, 2023 •

edited by kibanamachine

Loading

apmmachine commented Apr 12, 2023

elasticmachine commented Apr 18, 2023

kibana-ci commented Apr 19, 2023

ESLint disabled line counts

Total ESLint disabled count

shahzad31 left a comment

florent-leborgne left a comment

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

florent-leborgne Apr 19, 2023

dominiqueclarke Apr 19, 2023

florent-leborgne Apr 19, 2023 •

edited

Loading

dominiqueclarke Apr 19, 2023

florent-leborgne Apr 19, 2023

dominiqueclarke Apr 19, 2023

florent-leborgne Apr 19, 2023

	'Alert for monitor {{context.monitorName}} with url {{{context.monitorUrl}}} from {{context.observerLocation}} has recovered',
	'The monitor {{context.monitorName}} checking {{{context.monitorUrl}}} from {{context.observerLocation}} has recovered. The alert is no longer active.',

	'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} has recovered',
	'The monitor {{context.monitorName}} checking {{{context.monitorUrl}}} has recovered',

	subject: 'Monitor {{context.monitorName}} with url {{{context.monitorUrl}}} is down',
	subject: 'The monitor {{context.monitorName}} checking {{{context.monitorUrl}}} is down',

	'Alert for monitor {monitorName} with url {monitorUrl} from {observerLocation} has recovered',
	'The alert for the monitor {monitorName} checking {monitorUrl} from {observerLocation} is no longer active',

	defaultMessage: `Alert for TLS certificate {commonName} from issuer {issuer} has recovered`,
	defaultMessage: `The alert for TLS certificate {commonName} from issuer {issuer} is no longer active`,

	defaultMessage: `Abnormal ({severity} level) response time detected on {monitor} with url {monitorUrl} at {anomalyStartTimestamp}. Anomaly severity score is {severityScore}.
	defaultMessage: `Abnormal ({severity} level) response time detected on {monitor} on the URL {monitorUrl} at {anomalyStartTimestamp}. The anomaly severity score is {severityScore}.

	Response times as high as {slowestAnomalyResponse} have been detected from location {observerLocation}. Expected response time is {expectedResponseTime}.`,
	Response times as high as {slowestAnomalyResponse} have been detected from {observerLocation}. The expected response time is {expectedResponseTime}.`,

	defaultMessage: `Alert for abnormal ({severity} level) response time detected on monitor {monitor} with url {monitorUrl} from location {observerLocation} at {anomalyStartTimestamp} has recovered`,
	defaultMessage: `The alert for abnormal ({severity} level) response time detected at {anomalyStartTimestamp} on the monitor {monitor} checking {monitorUrl} from {observerLocation} is no longer active.`,

[Synthetics] add default email recovery message #154862

[Synthetics] add default email recovery message #154862

Conversation

dominiqueclarke commented Apr 12, 2023 • edited by kibanamachine Loading

Summary

Testing

apmmachine commented Apr 12, 2023

🤖 GitHub comments

elasticmachine commented Apr 18, 2023

kibana-ci commented Apr 19, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

shahzad31 left a comment

Choose a reason for hiding this comment

florent-leborgne 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

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

florent-leborgne Apr 19, 2023 • edited Loading

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

dominiqueclarke commented Apr 12, 2023 •

edited by kibanamachine

Loading

florent-leborgne Apr 19, 2023 •

edited

Loading