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

[Monitoring] Ensure Stack Monitoring UI works with metricbeat-* indices #96205

Merged
merged 103 commits into from
Apr 23, 2021

Conversation

chrisronline
Copy link
Contributor

Relates to #73864
Relates to elastic/beats#24427

This PR is meant to update all existing Stack Monitoring UI code to correctly work off metricbeat-* and .monitoring-* indices for Elasticsearch-related areas of code. It is an extension of #88212 as it identifies the fields missed by actually using a working Metricbeat module (previously, we had to mock the output of the module which was error-prone).

This effort is hard to guarantee 100% completion because we need to ensure every part of the front-end works properly with both data sets. We have two main ways to test this:

  1. Spot-check manually that all charts/data values are properly presented in the UI
  2. Leverage our existing functional test suites

The main issue with 2 is that our existing functional test suites use archive data which is in .monitoring-* indices so they aren't very helpful in ensuring the metricbeat-* data path works (but are helpful in determining the .monitoring-* data path still does work) so I did some work to build archive data using metricbeat-* indices.

This work was done in several parts:

Part 1: Write a script that identifies all known aliases that currently exist in the metricbeat-* indices and write it to a file: https://gist.github.com/chrisronline/d24ec862efdb0cfa331c7ef785241116

Part 2: Modify the rebuild all function of the es_archiver script to build out metricbeat-* indices from the existing .monitoring-* data archives, using the known aliases as well as some custom logic: https://github.com/elastic/kibana/pull/90838/files#diff-b0251a00b48f25e70b98d6741ae88948b22f9220a5ba5914aaaab465038da659 (This script is ran like: node ../scripts/es_archiver.js rebuild-all)

Part 3: Duplicate existing test suites and use the newly created archives based off metricbeat-* indices: https://github.com/elastic/kibana/pull/90838/files#diff-36d8e173c4e5a530dd9d5d68faca3cfd3f69f3803f80efda74c1f8bc562f033b

Note: elastic/elasticsearch#71233 needs to be merged before all functional test will consistently pass

@jasonrhodes jasonrhodes enabled auto-merge (squash) April 21, 2021 21:19
@jasonrhodes
Copy link
Member

@elasticmachine merge upstream

kibanamachine and others added 4 commits April 22, 2021 16:47
The functional test here: x-pack/test/api_integration/apis/monitoring/apm/instance_mb.js [it('should get apm instance data'] was failing due to these two values being wrong. I investigated the queries and couldn't see any reason our code would have broken this, so I assume something in the archive just resulted in slightly different numbers when we made the changes.

I'm adding this long note in the hopes that if this does turn into some kind of GC related bug, we can track it back to this.
@chrisronline chrisronline requested review from a team as code owners April 23, 2021 00:37
@jasonrhodes jasonrhodes force-pushed the monitoring/ecs_final_es branch from 38bfef6 to 15d50e7 Compare April 23, 2021 02:27
@jasonrhodes
Copy link
Member

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
infra 1.9MB 1.9MB +187.0B
monitoring 726.5KB 728.8KB +2.4KB
total +2.5KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@jasonrhodes jasonrhodes merged commit c9ce295 into master Apr 23, 2021
@mshustov mshustov deleted the monitoring/ecs_final_es branch April 26, 2021 09:09
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 27, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 96205 or prevent reminders by adding the backport:skip label.

1 similar comment
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 96205 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 29, 2021
jasonrhodes added a commit that referenced this pull request Apr 29, 2021
…*` indices (#96205) (#98672)

* [Monitoring] Ensure Stack Monitoring UI works with `metricbeat-*` indices (#96205)

* WIP

* WIP

* Remove unnecessary fields

* Work on node detail page

* Cluster overview looking good

* Index page

* Fix types

* ML jobs

* CCR

* CCR

* We just need total here

* Standalone cluster fix

* Re-enable logstash

* FIx jest test

* Fix tests

* Fix tests

* Fix unused import

* Add new MB-based archives

* Add actual archives

* Fix types

* Add this file back in

* Fix tests and add more

* Update whitelist

* Renames

* Renames

* Only do ccs if enabled

* Updates

* Comment out

* More tests passing

* Another passing test

* More passing, yay

* Forgot to add the actual tests, wow

* CCR

* Fix CI issues

* Missed a field here

* Kibana tests passing

* Fix type issues

* Fix type

* Fix types

* Good chunk of logstash work

* Fix types

* Fix jest test

* Fix issue with get usage in logstash code

* Fix small bug here

* Update archives with proper mappings

* Handle both legacy and mb fields properly

* Fixes

* Fix jest test

* Fix issue

* Getting setup tests in a better state

* Only beats is failing now, which is good

* Progress on cluster listing

* Functional tests passing!

* More progress on cluster tests

* More cluster work

* Fix test

* Last recovery working

* Fix types

* Fix tests

* More tweaks

* Fix snapshot

* Use stats instead of kibana_stats

* Use node and node_stats for logstash

* Beats tests passing

* APM tests passing

* API tests passing!

* Fix types

* Fix tests

* Renames beats-with-restarted-instance archive dirs

Kebab case is disallowed for all newly added files.

* Renames logstash-pipeline dirs

Kebab case disallowed for all new files

* Renames multi-basic dirs

Kebab case disallowed for all new files

* Renames singlecluster-* dirs

Kebab case disallowed for all new files

* Fixes inaccurate path change for archive setup

* Reverts changes to rebuild_all script

Co-authored-by: Jason Rhodes <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	x-pack/plugins/monitoring/server/lib/apm/_apm_stats.js
#	x-pack/test/api_integration/apis/monitoring/apm/fixtures/cluster.json

* Fix merge conflict error, 0 vs null

* Temporarily disabling _mb func tests for monitoring

We did this for master but now need to do it for this backport as well, see #98238 for more info

* Fixes incorrect archive path for new test after merge

Co-authored-by: Chris Roberson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Monitoring Stack Monitoring team v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants