-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] stream results back for project monitors #138069
[Synthetics] stream results back for project monitors #138069
Conversation
src/plugins/bfetch/server/plugin.ts
Outdated
} | ||
); | ||
}; | ||
const routeHandler: RequestHandler<any, any, any> = async (context, request, response) => { |
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'd like to get these types correct.
const routeDefinition = { | ||
path: `/${removeLeadingSlash(path)}`, | ||
validate: { | ||
body: schema.any(), |
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 suppose I need to remove this for get
failedMonitors: pushMonitorFormatter.failedMonitors, | ||
failedStaleMonitors: pushMonitorFormatter.failedStaleMonitors, | ||
}); | ||
subject?.complete(); |
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 suppose I need a try catch around everything, to ensure that complete
is always called in the event of errors and prevent the connection from hanging.
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.
Think in the case of error, you would need .error
from the observer. Thats how Observer pattern works IIRC.
b81c4e9
to
c1d6913
Compare
8fc2377
to
8d60180
Compare
44da1ee
to
76f229a
Compare
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.
bfetch code changes LGTM.
Pinging @elastic/uptime (Team:uptime) |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
History
To update your PR or re-run it, just comment with: |
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.
LGTM
End to end tested the PR following the instructions but using a cloud 8.5 snapshot and everything works.
}); | ||
} catch (error) { | ||
subject?.error(error); | ||
} finally { |
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 thought I added this? I guess I didn't push my last commit. :(
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.
No worries, we got it in there 😄
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.
LGTM! 🚢
* stream results back for private locations * update types * consolidate apis * adjust tests * Fix API tests. * Define types more clearly. * Reintroduce default method. * Add error handling to observable. Co-authored-by: Justin Kambic <jk@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 4990478)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
elastic#138848) * stream results back for private locations * update types * consolidate apis * adjust tests * Fix API tests. * Define types more clearly. * Reintroduce default method. * Add error handling to observable. Co-authored-by: Justin Kambic <jk@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 4990478) Co-authored-by: Dominique Clarke <dominique.clarke@elastic.co>
* stream results back for private locations * update types * consolidate apis * adjust tests * Fix API tests. * Define types more clearly. * Reintroduce default method. * Add error handling to observable. Co-authored-by: Justin Kambic <jk@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
After elastic/uptime#475, the project monitors api became much slower, often resulting in timeouts on the synthetic-agent side for even a small number of monitors, sometimes as low as 5. This PR accounts for that by streaming updates back to the synthetics agent. The final update in the stream remains the same, and consists of a full summary of the created, updated, deleted, and failed monitors.
EDIT: Added by @justinkambic:
Testing this PR
It is possible to test the
push
command from Synthetics with only a Kibana/ES cluster. It's also possible to test this E2E, with some additional configuration.Testing
push
onlyYou can configure Kibana to do this in a few minutes.
4. Click `Private Locations`, and follow the flow to make an Agent Policy
5. Navigate back to Monitor Management, follow the flow to add a Private Location
6. Create an [API key](https://www.elastic.co/guide/en/kibana/master/api-keys.html) for your admin user in Kibana. 7. Checkout the [synthetics branch that supports push streaming](https://github.com/elastic/synthetics/pull/576). 8. `npm run build && npm link`. Then, make a new directory for the project you will push. `mkdir {PROJECT_NAME}`. For the sake of clarity, make this project folder the same as the private location name you specified. 9. Use the build Synthetics CLI to create your project. `node dist/cli init {PROJECT_PATH}`. Follow the prompts to configure your project. You should see your private location on the list of choices in the CLI, select that one. 10. Enter your project dir and run `npm link @elastic/synthetics`. 11. After Synthetics builds your project, run `push` to see if your project's monitors get loaded to your Kibana instance. Example: `env SYNTHETICS_API_KEY={API_KEY} npm run push`. You should see output like the example below:
Testing this patch e2e
To do this, you will largely need to replicate the previous testing example, but with some additional configuration.
Firstly, set up the
elastic-package
utility.elastic-package stack up -d -v --version 8.5.0-SNAPSHOT
to spin up a deployment on 8.5 that we can use for testing purposes.server.port: 5602
.elastic-package
also useshttps
by default now, so you'll need to modify your host config to account for this if you were usinghttp
previously. Example:elasticsearch.hosts: https://localhost:9200
.kibana_system
password for it.4. Define an API key for your user. 5. Follow the instructions in the previous steps to push monitors. 6. Navigate to Uptime, you should be able to observer your monitors running:
Additional testing