[Monitoring] Metricbeat migration status logic #34871

chrisronline · 2019-04-10T15:06:08Z

Relates to #39010

This is step 1 from the ticket linked above. It contains the server logic and endpoint necessary to determine the migration state of each product based on the existence of certain indices.

We perform a single query across all monitoring indices, using a terms agg on the _index field. Then, we perform multiple terms subaggs for each stack product, determining from which product instance uuid the index exists for. Then, we collect the response and determine the state for each product: fully migrated, partially migrated, or internally collected.

As a way to provide support for a net new user experience (where the user has no monitoring setup), we are also attempting to detect the existence of certain products (mainly beats and logstash) based on the presence of other indices. This is a bit limiting as it will only look at the dedicated monitoring cluster for these indices (maybe we should do a CCS search here?) but it should provide some help.

This PR contains both mocha server tests and api integration tests (which is the majority of this diff).

Testing

Testing is limited to just Kibana and ES for the main part, but Logstash and Beats should be used for detection testing.

Main Testing - Part 1

Setup internal collection monitoring for all stack products (Kibana, ES, Logstash, Beats, APM)
POST the API and verify the response makes sense (/api/monitoring/v1/setup/collection)
- We should see all as internal collectors

Main Testing - Part 2

Setup internal collection monitoring for Logstash and Beats
Use Metricbeat for collection of ES and Kibana but do not disable internal collection.
POST the API and verify the response makes sense (/api/monitoring/v1/setup/collection)
- We should see Logstash and Beats as internal collectors, and Kibana and ES are partially migrated

Main Testing - Part 3

Setup internal collection monitoring for Logstash and Beats
Use Metricbeat for collection of ES and Kibana and disable internal collection.
POST the API and verify the response makes sense (/api/monitoring/v1/setup/collection)
- We should see Logstash and Beats as internal collectors, and Kibana and ES are fully migrated

Detect Testing

Configure a beat to send data to the monitoring cluster, but do not enable monitoring
POST the API and verify the response makes sense (/api/monitoring/v1/setup/collection)
- We should see that beats might be present in the api response

Multi-Cluster Testing

Configure multiple clusters in the above setups (probably just one as the behavior should be the same).
POST the API and verify the response makes sense (/api/monitoring/v1/setup/collection)
POST the cluster-specific API and verify the response makes sense (/api/monitoring/v1/setup/collection/{clusterUuid})

APM Testing

Only run Kibana from your local computer
Clone and setup this repo: https://github.com/elastic/apm-integration-testing
Start the apm testing environment: ./scripts/compose.py start --with-opbeans-node --no-kibana master
POST the API and verify the response makes sense (/api/monitoring/v1/setup/collection)
- We should see that apm is showing up

elasticmachine · 2019-04-10T15:52:55Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-10T16:36:36Z

Pinging @elastic/stack-monitoring

ycombinator · 2019-04-10T17:01:57Z

POST the API...

Any reason these APIs are POSTs and not GETs? AFAICT none of them take or need to take request bodies.

chrisronline · 2019-04-10T17:03:16Z

Any reason these APIs are POSTs and not GETs? AFAICT none of them take or need to take request bodies.

They both accept optional parameters (which aren't actually documented in the route defintion, so I need to do that) which are currently only used for testing. Maybe this is a bad idea? I needed someway to make the api integration tests work since the data for those tests isn't live

x-pack/plugins/monitoring/server/routes/api/v1/setup/cluster_setup_status.js

ycombinator · 2019-04-10T17:17:04Z

With the cleanup you'll do for #34871 (comment), there's a non-test-only reason for the APIs to be POSTs instead of GETs. So I'm +1 to keep them as POSTs.

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js

ycombinator · 2019-04-10T18:03:48Z

I've gotten through all 3 Main Testing items so far and it's looking great! I'll finish off the remaining two test cases shortly.

elasticmachine · 2019-04-10T18:45:57Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ycombinator · 2019-04-10T23:12:18Z

I called the cluster-specific API with a non-existent cluster UUID like so:

curl -H 'kbn-xsrf: foobar' -X POST -v 'http://localhost:5601/api/monitoring/v1/setup/collection/chris_sucks' | jq .

I still got a 200 OK response. I think we should return a 404?

ycombinator · 2019-04-10T23:27:42Z

I've gone through all test cases. There are a couple of unresolved comments, but otherwise this is looking very good!

chrisronline · 2019-04-11T20:29:47Z

I updated the description to include testing details for APM which was just recently fixed

elasticmachine · 2019-04-11T21:17:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-12T02:57:33Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

ycombinator · 2019-04-13T09:35:01Z

@chrisronline I see you added a change for APM and addressed my mightExists comment. But it doesn't look like you've addressed my 404 comment yet. Let me know when you've addressed all the feedback from the previous round and I'll re-review this PR.

chrisronline · 2019-04-15T12:40:22Z

@ycombinator Thanks for the reminder. Our current queries (at least I don't think so) make it possible to detect a cluster existing or not, so assuming that's true, we'd have to add a query to just check that. Does that seem accurate to you?

ycombinator · 2019-04-15T12:47:51Z

Our current queries (at least I don't think so) make it possible to detect a cluster existing or not, so assuming that's true, we'd have to add a query to just check that. Does that seem accurate to you?

I'm not sure, I'll have to dig into the code to figure this out. I'll do that now and get back to you with what I find.

elasticmachine · 2019-04-15T14:25:29Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-15T18:51:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js

ycombinator · 2019-04-16T14:38:19Z

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js

+    return false;
+  }
+
+  return get(bucket, 'beat_type.buckets[0].key') === 'apm-server';


Given that you're using _.get here, do you need the guard for bucket.beat_type above?

Sorry, I wasn't clear (or maybe what I'm proposing won't work) but I was thinking this function could just become:

function isBeatFromAPM(bucket) { return get(bucket, 'beat_type.buckets[0].key') === 'apm-server'; }

If bucket does not have a beat_type key, _.get should return a falsy value. Comparing that with apm-server would return false. So you wouldn't need the guard check.

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js

ycombinator · 2019-04-16T14:41:26Z

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js

+ * detected: {
+ *   doesExist: boolean, // This product definitely exists but does not have any monitoring documents (kibana and ES)
+ *   mightExist: boolean, // This product might exist based on the presence of some other indices
+ * }


❤️ this comment.

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js

ycombinator · 2019-04-16T14:48:13Z

x-pack/test/api_integration/apis/monitoring/setup/collection/index.js

+    loadTestFile(require.resolve('./kibana_exclusive_mb'));
+    loadTestFile(require.resolve('./es_and_kibana_mb'));
+    loadTestFile(require.resolve('./es_and_kibana_exclusive_mb'));
+    loadTestFile(require.resolve('./detect_beats'));


You've got ES, Kibana, and Beats covered here. Do you think we should add coverage for APM and Logstash too?

Yup definitely should. Thanks for the reminder

To be clear, this would just cover the detection logic. I'm not sure we can test between -mb- indices and non -mb- indices for beats, logstash or APM? Am I understanding that correctly? Because beats and APM won't be generating those indices and logstash doesn't support that yet

Your understanding is accurate.

At some point (hopefully soon) we'll have -mb- indices for Logstash. At that time we'd want to come back and add coverage for those here. For a future PR, of course!

ycombinator

Reviewed code and left comments.

chrisronline · 2019-04-16T15:28:27Z

Thanks @ycombinator. I addressed most of your comments. Will work on adding those additional tests

elasticmachine · 2019-04-16T16:42:55Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-16T20:31:04Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-16T21:31:41Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

chrisronline · 2019-04-17T13:06:10Z

@ycombinator I added those tests and this is ready for another round, thanks!

ycombinator

LGTM. Nice work! 👏👏👏

Looking forward to the the rest of the steps!

* Initial logic and tests for this * Improve test * Some cleanup and adding api integration tests * Add comments * Account for no documents returned * Add detection logic and tests * Ensure these fields are optional * Update detection logic * Beats, logstash and apm can only possibly exist * Fix some issues with APM * Fix tests * Small updates based on needing to also include the filebeat index * Fix issue with the reduce * PR feedback * More PR feedback * Add additional tests * Add additional tests

chrisronline · 2019-04-17T14:21:17Z

Backport:

7.x: bf9cf47

elasticmachine · 2019-06-14T18:50:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

chrisronline requested review from igoristic and ycombinator April 10, 2019 16:36

chrisronline self-assigned this Apr 10, 2019

chrisronline added Team:Monitoring Stack Monitoring team review v7.2.0 v8.0.0 labels Apr 10, 2019

chrisronline marked this pull request as ready for review April 10, 2019 16:36

ycombinator reviewed Apr 10, 2019

View reviewed changes

x-pack/plugins/monitoring/server/routes/api/v1/setup/cluster_setup_status.js Outdated Show resolved Hide resolved

ycombinator reviewed Apr 10, 2019

View reviewed changes

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js Show resolved Hide resolved

ycombinator reviewed Apr 10, 2019

View reviewed changes

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js Show resolved Hide resolved

ycombinator reviewed Apr 10, 2019

View reviewed changes

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js Show resolved Hide resolved

chrisronline added 3 commits April 15, 2019 08:58

Initial logic and tests for this

769dca3

Improve test

b0b19a5

Some cleanup and adding api integration tests

15c49ef

Merge remote-tracking branch 'elastic/master' into monitoring/mb_ux_2

6474c7c

ycombinator reviewed Apr 16, 2019

View reviewed changes

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js Outdated Show resolved Hide resolved

ycombinator reviewed Apr 16, 2019

View reviewed changes

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js Outdated Show resolved Hide resolved

ycombinator reviewed Apr 16, 2019

View reviewed changes

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js Show resolved Hide resolved

ycombinator reviewed Apr 16, 2019

View reviewed changes

x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.js Outdated Show resolved Hide resolved

ycombinator reviewed Apr 16, 2019

View reviewed changes

PR feedback

c33814b

More PR feedback

64df859

Add additional tests

c5ef2a5

Add additional tests

8b2f412

ycombinator approved these changes Apr 17, 2019

View reviewed changes

chrisronline merged commit 33f1123 into elastic:master Apr 17, 2019

chrisronline deleted the monitoring/mb_ux_2 branch April 17, 2019 13:17

chrisronline mentioned this pull request Apr 17, 2019

[7.x] [Monitoring] Metricbeat migration status logic (#34871) #35226

Merged

ycombinator mentioned this pull request Jun 14, 2019

Implement a UI within Stack Monitoring to help users migrate to MB shipping/collecting #39010

Closed

5 tasks

chrisronline mentioned this pull request Aug 6, 2019

[Monitoring] Metricbeat migration improvements #42600

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Monitoring] Metricbeat migration status logic #34871

[Monitoring] Metricbeat migration status logic #34871

chrisronline commented Apr 10, 2019 •

edited by ycombinator

Loading

elasticmachine commented Apr 10, 2019

elasticmachine commented Apr 10, 2019

ycombinator commented Apr 10, 2019

chrisronline commented Apr 10, 2019

ycombinator commented Apr 10, 2019

ycombinator commented Apr 10, 2019

elasticmachine commented Apr 10, 2019

ycombinator commented Apr 10, 2019 •

edited

Loading

ycombinator commented Apr 10, 2019

chrisronline commented Apr 11, 2019

elasticmachine commented Apr 11, 2019

elasticmachine commented Apr 12, 2019

ycombinator commented Apr 13, 2019

chrisronline commented Apr 15, 2019

ycombinator commented Apr 15, 2019

elasticmachine commented Apr 15, 2019

elasticmachine commented Apr 15, 2019

ycombinator Apr 16, 2019

ycombinator Apr 16, 2019

ycombinator Apr 16, 2019

ycombinator Apr 16, 2019

chrisronline Apr 16, 2019

chrisronline Apr 16, 2019

ycombinator Apr 16, 2019

ycombinator left a comment

chrisronline commented Apr 16, 2019

elasticmachine commented Apr 16, 2019

elasticmachine commented Apr 16, 2019

elasticmachine commented Apr 16, 2019

chrisronline commented Apr 17, 2019

ycombinator left a comment

chrisronline commented Apr 17, 2019

elasticmachine commented Jun 14, 2019

[Monitoring] Metricbeat migration status logic #34871

[Monitoring] Metricbeat migration status logic #34871

Conversation

chrisronline commented Apr 10, 2019 • edited by ycombinator Loading

Testing

Main Testing - Part 1

Main Testing - Part 2

Main Testing - Part 3

Detect Testing

Multi-Cluster Testing

APM Testing

elasticmachine commented Apr 10, 2019

💚 Build Succeeded

elasticmachine commented Apr 10, 2019

ycombinator commented Apr 10, 2019

chrisronline commented Apr 10, 2019

ycombinator commented Apr 10, 2019

ycombinator commented Apr 10, 2019

elasticmachine commented Apr 10, 2019

💔 Build Failed

ycombinator commented Apr 10, 2019 • edited Loading

ycombinator commented Apr 10, 2019

chrisronline commented Apr 11, 2019

elasticmachine commented Apr 11, 2019

💔 Build Failed

elasticmachine commented Apr 12, 2019

💚 Build Succeeded

ycombinator commented Apr 13, 2019

chrisronline commented Apr 15, 2019

ycombinator commented Apr 15, 2019

elasticmachine commented Apr 15, 2019

💔 Build Failed

elasticmachine commented Apr 15, 2019

💚 Build Succeeded

ycombinator Apr 16, 2019

Choose a reason for hiding this comment

ycombinator Apr 16, 2019

Choose a reason for hiding this comment

ycombinator Apr 16, 2019

Choose a reason for hiding this comment

ycombinator Apr 16, 2019

Choose a reason for hiding this comment

chrisronline Apr 16, 2019

Choose a reason for hiding this comment

chrisronline Apr 16, 2019

Choose a reason for hiding this comment

ycombinator Apr 16, 2019

Choose a reason for hiding this comment

ycombinator left a comment

Choose a reason for hiding this comment

chrisronline commented Apr 16, 2019

elasticmachine commented Apr 16, 2019

💚 Build Succeeded

elasticmachine commented Apr 16, 2019

💔 Build Failed

elasticmachine commented Apr 16, 2019

💚 Build Succeeded

chrisronline commented Apr 17, 2019

ycombinator left a comment

Choose a reason for hiding this comment

chrisronline commented Apr 17, 2019

elasticmachine commented Jun 14, 2019

💔 Build Failed

chrisronline commented Apr 10, 2019 •

edited by ycombinator

Loading

ycombinator commented Apr 10, 2019 •

edited

Loading