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

[APM] Make UI indices space aware (support for spaces) #126176

Merged
merged 10 commits into from
Feb 24, 2022

Conversation

cauemarcondes
Copy link
Contributor

@cauemarcondes cauemarcondes commented Feb 22, 2022

closes #49647
closes #125964

To accomplish it, a new saved object type had to be created apm-indices-space (suggestion for a better name is welcome), since changing the current one to be space aware is not possible. So in order to migrate the current APM indices, a new migration process was added on server/plugin.start(). It first fetches the legacy APM indices, then fetches all available spaces, and then bulk creates the new APM indices for each available space after that the legacy one is deleted.

When default indices are NOT overwritten

Query:
Screen Shot 2022-02-22 at 1 30 32 PM

1. Default space:

Indices:
Screen Shot 2022-02-22 at 1 30 45 PM

Service inventory:
Screen Shot 2022-02-22 at 1 31 10 PM

2. Space A

Service inventory:
Screen Shot 2022-02-22 at 1 31 56 PM

When default indices are overwritten

1. Default space:

Indices:
Screen Shot 2022-02-22 at 1 11 06 PM

Service inventory:
Screen Shot 2022-02-22 at 1 11 39 PM

Query:
Screen Shot 2022-02-22 at 1 13 05 PM

2. Space A

Indices:
Screen Shot 2022-02-22 at 1 18 16 PM

Service inventory:
Screen Shot 2022-02-22 at 1 18 42 PM

Query:
Screen Shot 2022-02-22 at 1 17 38 PM

3. Space B

Indices:
Screen Shot 2022-02-22 at 1 20 22 PM

Service inventory:
Screen Shot 2022-02-22 at 1 20 49 PM

Query:
Screen Shot 2022-02-22 at 1 19 48 PM

@cauemarcondes cauemarcondes added release_note:enhancement backport:skip This commit does not require backporting v8.2.0 labels Feb 22, 2022
@cauemarcondes cauemarcondes force-pushed the apm-indices-space-aware branch from 1662c61 to 71ad988 Compare February 22, 2022 18:37
@cauemarcondes cauemarcondes marked this pull request as ready for review February 22, 2022 18:37
@cauemarcondes cauemarcondes requested a review from a team as a code owner February 22, 2022 18:37
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Feb 22, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

Copy link
Contributor

@jportner jportner left a comment

Choose a reason for hiding this comment

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

Code review only --

Thanks for the detailed PR description! In general the approach looks good, a couple nits below.

@formgeist
Copy link
Contributor

In terms of indication to the user that these index settings are now space-aware, we can utilize the current Advanced Settings callout, since there are no other examples of space-awareness indications in Kibana (afaik).

CleanShot 2022-02-22 at 15 01 22@2x

Frame 1

Feel free to move this to another issue/PR as this could be seen as an enhancement.

PS: The icon used in the callout is spacesApp, I just didn't have the right icon to use for my mock 👍

@cauemarcondes
Copy link
Contributor Author

Feel free to move this to another issue/PR as this could be seen as an enhancement.

@formgeist

Screen Shot 2022-02-23 at 4 59 57 PM

Screen Shot 2022-02-23 at 5 00 12 PM

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

lgtm!

@kibana-ci
Copy link
Collaborator

💚 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
apm 2.7MB 2.7MB +702.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
apm-indices 7 9 +2

History

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

@cauemarcondes cauemarcondes merged commit 4e238ec into elastic:main Feb 24, 2022
Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

lgtm

lucasfcosta pushed a commit to lucasfcosta/kibana that referenced this pull request Mar 2, 2022
* new apm indices saved object

* adding some comments

* fixing typo

* addressing PR comments

* fixing tests

* addressing PR comments

* fixing tests

* fixing test

* showing callout with space name

* addressing PR changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:enhancement Team:APM All issues that need APM UI Team support v8.2.0
Projects
None yet
6 participants