Skip to content
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

[Heartbeat] One shot mode #25972

Merged
merged 17 commits into from
Oct 14, 2021
Merged

[Heartbeat] One shot mode #25972

merged 17 commits into from
Oct 14, 2021

Conversation

andrewvc
Copy link
Contributor

@andrewvc andrewvc commented May 27, 2021

In this mode heartbeat runs a single monitor once, writes all events to its output, then exits. See the included docs for examples and testing it. This is useful internally mostly as an experiment for now.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • Check the sample config

Related issues

Use cases

Screenshots

Logs

In this mode heartbeat runs a single monitor once, writes all events to
its output, then exits.
@andrewvc andrewvc added enhancement Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels May 27, 2021
@andrewvc andrewvc self-assigned this May 27, 2021
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 27, 2021
@botelastic
Copy link

botelastic bot commented May 27, 2021

This pull request doesn't have a Team:<team> label.

@elasticmachine
Copy link
Collaborator

elasticmachine commented May 27, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-10-14T01:51:54.809+0000

  • Duration: 67 min 25 sec

  • Commit: 79251a9

Test stats 🧪

Test Results
Failed 0
Passed 3543
Skipped 80
Total 3623

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

@spalger
Copy link

spalger commented Jun 9, 2021

While using this I plan to do setup logic outside of the process before telling heartbeat to execute a synthetic suite in one-shot mode. I'm planning on informing the "services" I am starting up in my journeys where ES and Kibana are via the environment, and it seems like https://github.com/elastic/beats/blob/master/x-pack/heartbeat/monitors/browser/synthexec/synthexec.go#L112 suggests this should work well, do you think it's safe to rely on this behavior?

@andrewvc
Copy link
Contributor Author

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Aug 17, 2021

Command update: failure

Base branch update has failed
merge conflict between base and head
err-code: 209C6

@BenB196
Copy link

BenB196 commented Sep 21, 2021

I don't mean to pester, but are there any plans to move this forward? This would be a really good feature for things like automated CI/CD tests.

@andrewvc
Copy link
Contributor Author

@BenB196 we have some internal use cases for this, but didn't think any external users would have cause to use it. Can you let us know more about what sort of testing you'd use this for? Normally you want tests run on a schedule, which is what heartbeat does.

@BenB196
Copy link

BenB196 commented Sep 21, 2021

@andrewvc sure. The main use case I see from this feature is the ability to abstract some "infrastructure" stuff from people writing test cases, while also giving them access to built in Elastic features; like the browser Observability features in Kibana, and machine learning for things like anomalously long response times on loading specific tests to detect regressions pre-production. All without having to do any custom integrations between playwright/synethics/elastic.

A bit more detail on the "infrastructure" part. It's far easier to provide someone a docker-compose file or a Kubernetes manifest with all of the base settings already set for Heartbeat, and then just tell them to insert their inline playwright script, or tell them to replace their zip_url information, than it is to have to essentially reinvent what Heartbeat does minus the scheduled run part.

@andrewvc
Copy link
Contributor Author

That's really helpful @BenB196 , so, as I understand it, you're missing the Kibana side of what synthetics does, and being able to run one-offs would be helpful.

IMHO that's a good reason to finish off this PR (which is 80% done really, just needs docs, tests, cleanup etc.).

I would say, however, that Kibana/Uptime itself is really oriented around scheduled runs, monitors that occasionally run will be.... weird. How do you handle this? Do you ask your users to pop into Kibana and wait for the monitor to appear? Is this roughness OK?

@BenB196
Copy link

BenB196 commented Sep 21, 2021

@andrewvc yeah, you've pretty much got the gist of it. I think the Kibana roughness is fine honestly. It just requires a small amount of education on the user end about either having to wait a bit for the results to show up, or to expand the timeframe back further to see old/past runs for a monitor id/environment.

Also, a bit more of a technical/implementation side, from how I would see this being used. Is it would be more or less part of a jenkins (or similar) pipeline, where this would just be one part of a testing process. So most likely the pipeline would run, and by the time it is complete, the data will already be in Kibana/Elasticsearch for the user to review if needed. It was also allow for the user to use the Heartbeat/synthetic debug logs to track any kind of issues that may arise during the pipeline.

@BenB196
Copy link

BenB196 commented Sep 21, 2021

The one thing that I can't tell from the PR of whether it does this or not, but it would be nice for Heartbeat to exit with a code similar to how the monitor run exited. i.e. If browser run succeeds exit with 0, if it fails exit with 1, etc. That way from a jenkins (or similar) pipeline, it would be easy to tell right away if the test failed without having to do much looking around.

@mergify
Copy link
Contributor

mergify bot commented Sep 22, 2021

This pull request does not have a backport label. Could you fix it @andrewvc? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Sep 22, 2021
@andrewvc andrewvc marked this pull request as ready for review October 13, 2021 16:00
@andrewvc andrewvc requested a review from a team as a code owner October 13, 2021 16:00
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (Team:Uptime)

Copy link
Member

@vigneshshanmugam vigneshshanmugam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

heartbeat/_meta/config/beat.yml.tmpl Outdated Show resolved Hide resolved
}
return nil
}

groups, _ := syscall.Getgroups()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to care about the permission model in one shot mode? or we are skipping it for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, yes. FYI, that's just a log line, there's no impact on functionality, the actual perms are reset elsewhere. But I'll move the logline up higher.

@@ -27,7 +27,7 @@ import (

// Config defines the structure of heartbeat.yml.
type Config struct {
// Modules is a list of module specific configuration data.
OneShot []*common.Config `config:"one_shot"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont want to deviate this PR in to some other direction, but are we okay with this naming or change it to something like RunOnce mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that idea, I think it's more clear, will change to run_once

heartbeat/monitors/plugin/plugin.go Outdated Show resolved Hide resolved
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@@ -109,3 +109,37 @@ include::monitors/monitor-tcp.asciidoc[]
include::monitors/monitor-http.asciidoc[]

include::monitors/monitor-browser.asciidoc[]

[float]
[[one-shot-mode]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[[one-shot-mode]]
[[run-once-mode]

logp.Info("Starting run_once run. This is an experimental feature and may be changed or removed in the future!")
cfgs := bt.config.RunOnce

publishClient, err := core.NewSyncClient(logp.NewLogger("oneshot mode"), b.Publisher, beat.ClientConfig{})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
publishClient, err := core.NewSyncClient(logp.NewLogger("oneshot mode"), b.Publisher, beat.ClientConfig{})
publishClient, err := core.NewSyncClient(logp.NewLogger("runonce mode"), b.Publisher, beat.ClientConfig{})

@andrewvc andrewvc merged commit 6d09457 into elastic:master Oct 14, 2021
@andrewvc
Copy link
Contributor Author

@Mergifyio backport 7.x

mergify bot pushed a commit that referenced this pull request Oct 14, 2021
(cherry picked from commit 6d09457)
@mergify
Copy link
Contributor

mergify bot commented Oct 14, 2021

Command backport 7.x: success

Backports have been created

andrewvc pushed a commit that referenced this pull request Oct 14, 2021
@andrewvc andrewvc deleted the oneshotoneop branch October 14, 2021 12:29
andrewvc added a commit to andrewvc/beats that referenced this pull request Oct 19, 2021
Fixes elastic#28437. This is an improvement over
elastic#25972 which had a more complicated
config interface.
andrewvc added a commit that referenced this pull request Oct 20, 2021
* [Heartbeat] Make run_once syntax a boolean

Fixes #28437. This is an improvement over
#25972 which had a more complicated
config interface.

* Fix python tests

* Fix python
mergify bot pushed a commit that referenced this pull request Oct 20, 2021
* [Heartbeat] Make run_once syntax a boolean

Fixes #28437. This is an improvement over
#25972 which had a more complicated
config interface.

* Fix python tests

* Fix python

(cherry picked from commit d31cae9)
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
* [Heartbeat] Make run_once syntax a boolean

Fixes elastic#28437. This is an improvement over
elastic#25972 which had a more complicated
config interface.

* Fix python tests

* Fix python
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify enhancement Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants