-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Synthetics] Adds additional heartbeat configuration options to the Synthetics app #154390
[Synthetics] Adds additional heartbeat configuration options to the Synthetics app #154390
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
…-add-extra-heartbeat-configs
…-add-extra-heartbeat-configs
…-add-extra-heartbeat-configs
…/github.com/dominiqueclarke/kibana into feat/synthetics-add-extra-heartbeat-configs
Pinging @elastic/uptime (Team:uptime) |
@elasticmachine merge upstream |
…/github.com/dominiqueclarke/kibana into feat/synthetics-add-extra-heartbeat-configs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice content! I have mostly minor wording style suggestions. I hope I haven't missed anything :)
.../plugins/synthetics/public/apps/synthetics/components/monitor_add_edit/form/field_config.tsx
Outdated
Show resolved
Hide resolved
.../plugins/synthetics/public/apps/synthetics/components/monitor_add_edit/form/field_config.tsx
Outdated
Show resolved
Hide resolved
.../plugins/synthetics/public/apps/synthetics/components/monitor_add_edit/form/field_config.tsx
Outdated
Show resolved
Hide resolved
.../plugins/synthetics/public/apps/synthetics/components/monitor_add_edit/form/field_config.tsx
Outdated
Show resolved
Hide resolved
.../plugins/synthetics/public/apps/synthetics/components/monitor_add_edit/form/field_config.tsx
Outdated
Show resolved
Hide resolved
.../plugins/synthetics/public/apps/synthetics/components/monitor_add_edit/form/field_config.tsx
Outdated
Show resolved
Hide resolved
fieldKey: ConfigKey.RESPONSE_JSON_CHECK, | ||
component: KeyValuePairsField, | ||
label: i18n.translate('xpack.synthetics.monitorConfig.responseJSON.label', { | ||
defaultMessage: 'Check response body contains JSON', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that all labels for similar settings are like this. I'd suggest 2 improvements depending on what is possible, timely and realistic to do:
-
Have simple labels and use the operators as preset fields (a bit like what the JSON setting looks like), for example:
Response status
Equals to
input field...
Response body
Contains
input field...
some help text
Does not contain
input field...
some help text -
Quick label structure improvement: remove "Check". It's already explained in the section description and name "Response checks"
- Response body contains JSON
- Response status equals to
- Response headers contain
- etc.
fieldKey: ConfigKey.PROXY_HEADERS, | ||
component: HeaderField, | ||
label: i18n.translate('xpack.synthetics.monitorConfig.proxyHeaders.label', { | ||
defaultMessage: 'Proxy headers', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not very content related, but I'm wondering from the screenshots if it wouldn't be clearer to have the "+ Add expression", "+ Add header" and similar types of button below the setting descriptions and values already added. Just thinking in terms of layout. Maybe it'd look odd as well, not sure.
.../plugins/synthetics/public/apps/synthetics/components/monitor_add_edit/form/field_config.tsx
Outdated
Show resolved
Hide resolved
.../plugins/synthetics/public/apps/synthetics/components/monitor_add_edit/form/field_config.tsx
Outdated
Show resolved
Hide resolved
…nitor_add_edit/form/field_config.tsx Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
…nitor_add_edit/form/field_config.tsx Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
…nitor_add_edit/form/field_config.tsx Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
…nitor_add_edit/form/field_config.tsx Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
…nitor_add_edit/form/field_config.tsx Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
@@ -90,7 +90,7 @@ const getNearestSupportedSchedule = (currentSchedule: string): string => { | |||
|
|||
return closest; | |||
} catch { | |||
return ALLOWED_SCHEDULES_IN_MINUTES[0]; | |||
return '10'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is. Paul and I had a conversation about this, and I decided to just make this quick change in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested newly added configs !!
Works as expected !!
…/github.com/dominiqueclarke/kibana into feat/synthetics-add-extra-heartbeat-configs
…nitor_add_edit/form/field_config.tsx Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
…nitor_add_edit/form/field_config.tsx Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Summary
Closes #140864
Relates to https://github.com/elastic/synthetics-dev/issues/159
Relates to elastic/synthetics#655
Adds missing heartbeat configs to the Synthetics app. The additional configs are specified below:
HTTP (proxy_headers, mode, ipv4, ipv6, response.include_body_max_bytes, check.response.json)
TCP (mode, ipv4, ivp6)
ICMP (mode, ipv4, ipv6)
Testing