Query cluster UUID on apm-server startup #6591

stuartnelson3 · 2021-11-11T11:52:55Z

Motivation/summary

stack monitoring ui in cloud depends on the beat module of metricbeat, which requires apm-server to know its cluster_uuid in order for the documents to be indexed. The apm-server only gets its cluster_uuid as a callback to making an initial connection with elasticsearch.

When using datastreams, the initial connection isn't made until it attempts to index a document, which means that stack monitoring ui is broken for apm-server until a document is indexed, even though nothing may be wrong with the apm-server.

Copy the request made in libbeat and execute it as a precondition to get the cluster_uuid as soon as the server is ready.

Checklist

Update CHANGELOG.asciidoc
~~Documentation has been updated~~

How to test these changes

start apm-integration with elasticsearch + kibana
install the apm-integration
start apm-server: apm-server -e -E apm-server.data_streams.enabled=true -E http.enabled=true
curl the /state internal metrics endpoint: curl -s http://localhost:5066/state | jq '.outputs.elasticsearch'
you should see non-empty output for the cluster_uuid

mergify · 2021-11-11T11:53:25Z

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

backport-7.x is the label to automatically backport to the 7.x branch.
backport-7./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.

beater/beater.go

stuartnelson3 · 2021-11-11T11:54:35Z

beater/beater.go

+	if elasticsearchRegistry.Get(clusterUUID) != nil {
+		elasticsearchRegistry.Remove(clusterUUID)
+	}
+	clusterUUIDRegVar := monitoring.NewString(elasticsearchRegistry, clusterUUID)


I looked through the api but it wasn't immediately apparent to me how to update a value already existing in a registry.

There's a Get method on the monitoring.Registry which I think you can use?

Would the libbeat code have been called yet at this point? I'm wondering though if there's any risk that the libbeat code could kick in concurrently and panic.

There's a Get method on the monitoring.Registry which I think you can use?

roger, I have to cast to *monitoring.String, but it appears to work (the return value is Var interface type, it seems, which only exposes a single Visit method).

Would the libbeat code have been called yet at this point? I'm wondering though if there's any risk that the libbeat code could kick in concurrently and panic.

The registry code is called synchronously, before *beater.Run, so it should always be set up. The Set method on the *monitoring.String could be called at the same time, but it has an internal mutex, so access should be fine.

Cool, thanks. I see you removed the test guards, looks much safer to me now.

apmmachine · 2021-11-11T11:59:56Z

💚 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: 2021-11-15T15:21:32.765+0000
Duration: 44 min 54 sec
Commit: 498c662

Test stats 🧪

Test	Results
Failed	0
Passed	6022
Skipped	19
Total	6041

🤖 GitHub comments

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

/test : Re-trigger the build.
/hey-apm : Run the hey-apm benchmark.
/package : Generate and publish the docker images.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

axw

Generally LGTM, but I'm a bit worried about the timing of the var registration. Is there any possibility that the libbeat code would be called after ours, and cause a panic?

beater/beater.go

axw · 2021-11-11T12:17:51Z

beater/beater.go

+	if elasticsearchRegistry.Get(clusterUUID) != nil {
+		elasticsearchRegistry.Remove(clusterUUID)
+	}
+	clusterUUIDRegVar := monitoring.NewString(elasticsearchRegistry, clusterUUID)


There's a Get method on the monitoring.Registry which I think you can use?

Would the libbeat code have been called yet at this point? I'm wondering though if there's any risk that the libbeat code could kick in concurrently and panic.

axw

LGTM!

stuartnelson3 · 2021-11-11T18:12:03Z

/test

stuartnelson3 · 2021-11-12T12:17:21Z

/test

stuartnelson3 · 2021-11-12T16:49:23Z

/test

mergify · 2021-11-15T08:24:48Z

This pull request is now in conflicts. Could you fix it @stuartnelson3? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b query-cluster-uuid upstream/query-cluster-uuid
git merge upstream/master
git push upstream query-cluster-uuid

don't return a nil error when there was an http status issue

registries are created when the cluster_uuid callback is registered, which doesn't happen under elastic-agent because of the config being delivered later.

(cherry picked from commit ecf0df2)

(cherry picked from commit ecf0df2) # Conflicts: # changelogs/head.asciidoc

(cherry picked from commit ecf0df2) Co-authored-by: stuart nelson <[email protected]>

marclop · 2021-12-21T06:29:49Z

Verified with 9bb5e4c:

./apm-server -E output.elasticsearch.username=admin -E output.elasticsearch.password=changeme -E apm-server.kibana.host=http://localhost:5601 -E apm-server.kibana.username=admin -E apm-server.kibana.password=changeme -E apm-server.kibana.enabled=true -E apm-server.expvar.enabled=true -E http.enabled=true
...
$ curl -s http://localhost:5066/state | jq '.outputs.elasticsearch'
{
  "cluster_uuid": "nE_NVzmNRAWVnk5BzWVSJw"
}

stuartnelson3 added 3 commits November 11, 2021 12:11

query cluster_uuid as precondition

39253dd

add test for querying cluster_uuid

54ab3f6

make sure the es client is not nil

036231b

stuartnelson3 requested a review from a team November 11, 2021 11:52

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 11, 2021

stuartnelson3 commented Nov 11, 2021

View reviewed changes

beater/beater.go Show resolved Hide resolved

stuartnelson3 commented Nov 11, 2021

View reviewed changes

update changelog

e1479b9

stuartnelson3 added backport-7.16 Automated backport with mergify to the 7.16 branch backport-8.0 Automated backport with mergify labels Nov 11, 2021

mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Nov 11, 2021

axw reviewed Nov 11, 2021

View reviewed changes

remove test guards and re-use cluster_uuid var

4d0a0c2

axw approved these changes Nov 11, 2021

View reviewed changes

Merge branch 'master' into query-cluster-uuid

7205a61

stuartnelson3 enabled auto-merge (squash) November 11, 2021 13:17

stuartnelson3 and others added 3 commits November 14, 2021 17:20

Merge branch 'master' into query-cluster-uuid

12d27ad

Merge branch 'master' into query-cluster-uuid

6d0e98b

Merge branch 'master' into query-cluster-uuid

017a664

stuartnelson3 added 4 commits November 15, 2021 11:32

return error if there is an http error

5db771b

don't return a nil error when there was an http status issue

create registries if they don't exist

0781439

registries are created when the cluster_uuid callback is registered, which doesn't happen under elastic-agent because of the config being delivered later.

Merge branch 'master' into query-cluster-uuid

4907d19

fix bad merge

498c662

stuartnelson3 merged commit ecf0df2 into elastic:master Nov 15, 2021

mergify bot pushed a commit that referenced this pull request Nov 15, 2021

Query cluster UUID on apm-server startup (#6591)

be0d2c7

(cherry picked from commit ecf0df2)

mergify bot mentioned this pull request Nov 15, 2021

[8.0] Query cluster UUID on apm-server startup (backport #6591) #6620

Merged

mergify bot pushed a commit that referenced this pull request Nov 15, 2021

Query cluster UUID on apm-server startup (#6591)

76ee939

(cherry picked from commit ecf0df2) # Conflicts: # changelogs/head.asciidoc

mergify bot mentioned this pull request Nov 15, 2021

[7.16] Query cluster UUID on apm-server startup (backport #6591) #6621

Merged

stuartnelson3 deleted the query-cluster-uuid branch November 15, 2021 16:14

marclop pushed a commit that referenced this pull request Nov 16, 2021

[7.16] Query cluster UUID on apm-server startup (backport #6591) (#6621)

4685e4f

(cherry picked from commit ecf0df2) Co-authored-by: stuart nelson <[email protected]>

stuartnelson3 added a commit that referenced this pull request Nov 16, 2021

Query cluster UUID on apm-server startup (#6591) (#6620)

59a43b1

(cherry picked from commit ecf0df2) Co-authored-by: stuart nelson <[email protected]>

stuartnelson3 added test-plan v8.0.0 labels Dec 20, 2021

marclop self-assigned this Dec 21, 2021

marclop added the test-plan-ok label Dec 21, 2021

axw mentioned this pull request Sep 21, 2022

Remove dependency on libbeat/cmd #9149

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Query cluster UUID on apm-server startup #6591

Query cluster UUID on apm-server startup #6591

stuartnelson3 commented Nov 11, 2021 •

edited

Loading

mergify bot commented Nov 11, 2021

stuartnelson3 Nov 11, 2021

axw Nov 11, 2021

stuartnelson3 Nov 11, 2021

axw Nov 11, 2021

apmmachine commented Nov 11, 2021 •

edited

Loading

Build stats

Test stats 🧪

axw left a comment

axw Nov 11, 2021

axw left a comment

stuartnelson3 commented Nov 11, 2021

stuartnelson3 commented Nov 12, 2021

stuartnelson3 commented Nov 12, 2021

mergify bot commented Nov 15, 2021

marclop commented Dec 21, 2021

Query cluster UUID on apm-server startup #6591

Query cluster UUID on apm-server startup #6591

Conversation

stuartnelson3 commented Nov 11, 2021 • edited Loading

Motivation/summary

Checklist

How to test these changes

mergify bot commented Nov 11, 2021

stuartnelson3 Nov 11, 2021

Choose a reason for hiding this comment

axw Nov 11, 2021

Choose a reason for hiding this comment

stuartnelson3 Nov 11, 2021

Choose a reason for hiding this comment

axw Nov 11, 2021

Choose a reason for hiding this comment

apmmachine commented Nov 11, 2021 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

axw left a comment

Choose a reason for hiding this comment

axw Nov 11, 2021

Choose a reason for hiding this comment

axw left a comment

Choose a reason for hiding this comment

stuartnelson3 commented Nov 11, 2021

stuartnelson3 commented Nov 12, 2021

stuartnelson3 commented Nov 12, 2021

mergify bot commented Nov 15, 2021

marclop commented Dec 21, 2021

stuartnelson3 commented Nov 11, 2021 •

edited

Loading

apmmachine commented Nov 11, 2021 •

edited

Loading