[Heartbeat] Fix run from location in states loader #35336

andrewvc · 2023-05-04T20:08:36Z

The states loader currently does not correctly read the location from the heartbeat.run_from.id config option. It's currently only applied to the observer.* fields. This patch fixes that. It also makes the logs more clear.

Note that we now log on each initial state for a monitor. This means one more log line, but only on the first exec of that monitor.

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

below test has been run

How to test this PR locally

Enable a monitor, either lightweight or browser, and set in your hearbeat config:

heartbeat.run_once: true
heartbeat.run_from:
  id: minneapolis

Then run with

mage build && ./heartbeat -e -E cloud.id=$CLOUD_ID -E cloud.auth=$CLOUD_AUTH -E output.elasticsearch.allow_older_versions=
true 2>&1 | jq .message

Look for the output:

"no previous state found for monitor my-monitor-aoeuncoaeuc in Elasticsearch (loc=minneapolis)"
"initializing new state for monitor my-monitor-aoeuncoaeuc: <monitorstate:id=minneapolis-187e85ec8e9-0,started=2023-05-04 15:05:38.665323551 -0500 CDT m=+1.134478775,up=0,down=1>"

for your monitor, if it does not have previous state in ES

And if it does have previous state (which it should on the next run):

"loaded previous state for monitor my-monitor-aoeuncoaeuc: <monitorstate:id=minneapolis-187e85ec8e9-0,started=2023-05-04 15:05:38.665323551 -0500 CDT,up=0,down=1>"

Note that the state ID should look like: minneapolis-187e85ec8e9-0 where the first string is the run_from.id value.

The states loader currently does not correctly read the location from the `heartbeat.run_from.id` config option. It's currently only applied to the `observer.*` fields. This patch fixes that.

mergify · 2023-05-04T20:09:10Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @andrewvc? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2023-05-04T20:34:38Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-05-05T21:43:32.363+0000
Duration: 47 min 24 sec

Test stats 🧪

Test	Results
Failed	0
Passed	1927
Skipped	25
Total	1952

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the 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.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2023-05-04T21:26:39Z

Pinging @elastic/uptime (Team:Uptime)

heartbeat/beater/heartbeat.go

heartbeat/monitors/factory.go

vigneshshanmugam · 2023-05-04T22:31:55Z

heartbeat/monitors/wrappers/monitorstate/esloader.go

 	return func(sf stdfields.StdMonitorFields) (*State, error) {
+		var runFromID string
+		if sf.RunFrom != nil {


Shouldnt we default to using beatLocation otherwise, passed as 3rd param in func?

The purpose of this is that we want the fleet integration to set the location correctly, however the fleet integration can only set monitor fields, not root fields. Hence the precedence

vigneshshanmugam · 2023-05-05T21:19:46Z

heartbeat/beater/heartbeat.go

@@ -131,6 +131,11 @@ func New(b *beat.Beat, rawConfig *conf.C) (beat.Beater, error) {
 		}),
 		trace: trace,
 	}
+	runFromID := "<unknown location>"


nit: we have default value as runFrom Id in state and unknown here, can we follow a common value.

vigneshshanmugam · 2023-05-05T21:26:32Z

heartbeat/monitors/wrappers/monitorstate/esloader.go

@@ -54,9 +59,9 @@ func MakeESLoader(esc *eslegclient.Connection, indexPattern string, beatLocation
 			},
 		}

-		if sf.RunFrom != nil {
+		if runFromID != "" {


Found a bug with this approach, As we are passing the state query only when runFrom is specified. It leads to a bug when then runFrom is set and removed later.

Set heartbeat.runfrom to a test-loc, state will created on index <monitorstate:id=test-loc-<blah>>

Remove the value of run_from, now instead of checking the default location or unknown if we change the naming. We will be look at the wrong index for the monitor.

vigneshshanmugam

LGTM

* [Heartbeat] Fix run from location in states loader The states loader currently does not correctly read the location from the `heartbeat.run_from.id` config option. It's currently only applied to the `observer.*` fields. This patch fixes that. * Fix errcheck * Add tests * Fixes * Add changelog * More nil checks --------- Co-authored-by: Vignesh Shanmugam <vignesh.shanmugam22@gmail.com> (cherry picked from commit 3550280)

* [Heartbeat] Fix run from location in states loader The states loader currently does not correctly read the location from the `heartbeat.run_from.id` config option. It's currently only applied to the `observer.*` fields. This patch fixes that. * Fix errcheck * Add tests * Fixes * Add changelog * More nil checks --------- Co-authored-by: Vignesh Shanmugam <vignesh.shanmugam22@gmail.com> (cherry picked from commit 3550280) Co-authored-by: Andrew Cholakian <andrewvc@elastic.co>

* [Heartbeat] Fix run from location in states loader The states loader currently does not correctly read the location from the `heartbeat.run_from.id` config option. It's currently only applied to the `observer.*` fields. This patch fixes that. * Fix errcheck * Add tests * Fixes * Add changelog * More nil checks --------- Co-authored-by: Vignesh Shanmugam <vignesh.shanmugam22@gmail.com>

[Heartbeat] Fix run from location in states loader

e021784

The states loader currently does not correctly read the location from the `heartbeat.run_from.id` config option. It's currently only applied to the `observer.*` fields. This patch fixes that.

andrewvc added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label May 4, 2023

andrewvc self-assigned this May 4, 2023

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 4, 2023

andrewvc added bug Heartbeat v8.8.0 labels May 4, 2023

andrewvc added 3 commits May 4, 2023 16:05

Fix errcheck

c8d6d8d

Add tests

2225937

Fixes

bd58130

andrewvc marked this pull request as ready for review May 4, 2023 21:26

andrewvc requested a review from a team as a code owner May 4, 2023 21:26

Add changelog

6477ba1

vigneshshanmugam requested changes May 4, 2023

View reviewed changes

andrewvc and others added 2 commits May 4, 2023 21:58

More nil checks

4241c48

Merge branch 'main' into fix-run-from-states

0f21c5f

vigneshshanmugam reviewed May 5, 2023

View reviewed changes

Merge branch 'main' into fix-run-from-states

6ff8c3b

vigneshshanmugam approved these changes May 5, 2023

View reviewed changes

vigneshshanmugam merged commit 3550280 into elastic:main May 5, 2023

vigneshshanmugam added the backport-v8.8.0 Automated backport with mergify label May 5, 2023

mergify bot mentioned this pull request May 5, 2023

[8.8](backport #35336) [Heartbeat] Fix run from location in states loader #35361

Merged

reakaleek mentioned this pull request Jul 19, 2023

Fix ironbank validation in 8.8 #36115

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Heartbeat] Fix run from location in states loader #35336

[Heartbeat] Fix run from location in states loader #35336

andrewvc commented May 4, 2023 •

edited by vigneshshanmugam

Loading

mergify bot commented May 4, 2023

elasticmachine commented May 4, 2023 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

elasticmachine commented May 4, 2023

vigneshshanmugam May 4, 2023 •

edited

Loading

andrewvc May 5, 2023

vigneshshanmugam May 5, 2023

vigneshshanmugam May 5, 2023

vigneshshanmugam left a comment

[Heartbeat] Fix run from location in states loader #35336

[Heartbeat] Fix run from location in states loader #35336

Conversation

andrewvc commented May 4, 2023 • edited by vigneshshanmugam Loading

Checklist

Author's Checklist

How to test this PR locally

mergify bot commented May 4, 2023

elasticmachine commented May 4, 2023 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented May 4, 2023

vigneshshanmugam May 4, 2023 • edited Loading

Choose a reason for hiding this comment

andrewvc May 5, 2023

Choose a reason for hiding this comment

vigneshshanmugam May 5, 2023

Choose a reason for hiding this comment

vigneshshanmugam May 5, 2023

Choose a reason for hiding this comment

vigneshshanmugam left a comment

Choose a reason for hiding this comment

andrewvc commented May 4, 2023 •

edited by vigneshshanmugam

Loading

elasticmachine commented May 4, 2023 •

edited by jenkins-beats-ci bot

Loading

vigneshshanmugam May 4, 2023 •

edited

Loading