[UX Dashboard] Migrate service list query out of APM #132548

justinkambic · 2022-05-19T18:56:30Z

Summary

Related to #454.

Migrates the service list query and route out of APM to the UX plugin.

Changes:

deleted the service list route
deleted the service list query
migrated the unit test to UX (snapshot remains unchanged, minus the removal of the apm key)
utilized the Data plugin to fetch the data
deleted some APM-specific code from the query generation process, output of the query generator seemed to remain unchanged
leveraging data view hook to get the pattern to use for the useEsSearch call
copied some server-specific helper functions from APM server to UX for use in query generation

Testing this PR

You should test the dashboard against a rich set of UX data, like that available on the o11y edge cluster.

Ensure that the expected services show up in the dropdown. Select each value and see that the view updates as expected.

Apply any kind of filtering the app is capable of to ensure that the list behaves according to your expectations.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2022-05-19T18:56:35Z

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

justinkambic · 2022-05-19T18:57:08Z

x-pack/plugins/ux/common/utils/merge_projection.ts

+    : U
+  : U;
+
+export function mergeProjection<T extends any, U extends SourceProjection>(


Ended up moving this over because there's logic about projection in the query generator for this component. LMK if this can be somehow skipped and we can delete this file.

APM doesn't use projections anymore. If you also want to get rid of projections for the UX related code you are welcome to do it as part of this PR or a follow-up. If you do that we can delete the merge_projections.ts in both APM and UX.

@sqren I think there are one or two other components in APM relying on that code. When I take them out I'll make sure nothing else references the projections and delete then.

@shahzad31 I'd appreciate your thoughts on whether we want this projection code to move into UX or if we can remove it here and now.

i think if it makes sense we can move these. If you think the util apm is using in place of projections, it's also worth exploring those.

justinkambic · 2022-05-19T18:57:38Z

x-pack/plugins/ux/public/services/data/getEsFilter.ts

+import { UxUIFilters } from '../../../typings/ui_filters';
+import { environmentQuery } from '../../components/app/rum_dashboard/local_uifilters/queries';
+
+export function getEsFilter(uiFilters: UxUIFilters, exclude?: boolean) {


Likewise copied this from the server because the component's query generator depends on it.

Copying is probably fine. Fewer dependencies <3

sorenlouv · 2022-05-20T07:14:17Z

x-pack/plugins/apm/server/routes/rum_client/route.ts

-    return { rumServices };
-  },
-});
-
 const rumVisitorsBreakdownRoute = createApmServerRoute({


I assume the remaining routes will have to be moved out in a future PR?

@sqren yes we are going to be doing this one component at a time, or grouped if there are some that it makes sense to move at once. Some are going to be ported to using data plugin queries, others will become embeddables, with the goal of avoiding adding a server component to the UX plugin. We're planning to have them all removed soon!

sorenlouv · 2022-05-20T07:15:28Z

x-pack/plugins/ux/common/processor_event.ts

+  metric = 'metric',
+  span = 'span',
+  profile = 'profile',
+}


This is fine for now. But we should find a way to share these to avoid duplication. Eg. this could reside in plugins/observability/common/apm/processor_event.ts (or something like it).

++ I am happy to move them there now and reference from APM if you want. I can make that as a follow-up so it's easier to review without these tangential changes.

sorenlouv · 2022-05-20T07:18:47Z

x-pack/plugins/ux/common/utils/merge_projection.ts

+    : U
+  : U;
+
+export function mergeProjection<T extends any, U extends SourceProjection>(


APM doesn't use projections anymore. If you also want to get rid of projections for the UX related code you are welcome to do it as part of this PR or a follow-up. If you do that we can delete the merge_projections.ts in both APM and UX.

sorenlouv · 2022-05-20T07:20:03Z

x-pack/plugins/ux/public/services/data/getEsFilter.ts

+import { UxUIFilters } from '../../../typings/ui_filters';
+import { environmentQuery } from '../../components/app/rum_dashboard/local_uifilters/queries';
+
+export function getEsFilter(uiFilters: UxUIFilters, exclude?: boolean) {


Copying is probably fine. Fewer dependencies <3

shahzad31 · 2022-05-23T16:19:13Z

@elasticmachine merge upstream

x-pack/plugins/ux/public/services/data/get_es_filter.ts

shahzad31

LGTM !!

Thanks for picking it up.

…kibana into 454/service-list-migration

justinkambic · 2022-05-23T23:04:45Z

@elasticmachine merge upstream

kibana-ci · 2022-05-24T00:04:55Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ux`	133	138	+5

Async chunks

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

id	before	after	diff
`ux`	154.0KB	155.3KB	+1.3KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`ux`	13	12	-1

Total ESLint disabled count

id	before	after	diff
`ux`	14	13	-1

History

💔 Build #47037 failed 98724ab
💛 Build #47004 was flaky 335bbd5
💚 Build #46601 succeeded 5c2f9bd
💔 Build #46569 failed 0d280fa
💔 Build #46239 failed 917c12d592fac85f41c9e6e173962df8bac90384

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

cc @justinkambic

kibanamachine · 2022-05-24T15:18:47Z

💔 All backports failed

Status	Branch	Result
❌	8.4	The branch "8.4" is invalid or doesn't exist

Manual backport

To create the backport manually run:

node scripts/backport --pr 132548

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2022-05-25T15:16:18Z

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

kibanamachine · 2022-05-26T15:26:18Z

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

kibanamachine · 2022-05-27T15:36:17Z

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

kibanamachine · 2022-05-30T15:46:18Z

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

justinkambic added chore technical debt Improvement of the software architecture and operational architecture labels May 19, 2022

justinkambic requested review from a team as code owners May 19, 2022 18:56

justinkambic self-assigned this May 19, 2022

botelastic bot added the Team:APM All issues that need APM UI Team support label May 19, 2022

justinkambic commented May 19, 2022

View reviewed changes

justinkambic mentioned this pull request May 19, 2022

Move UX server side code out of APM UI elastic/uptime#454

Closed

19 tasks

justinkambic added release_note:skip Skip the PR/issue when compiling release notes v8.3.0 v8.4.0 labels May 19, 2022

justinkambic force-pushed the 454/service-list-migration branch from 2713c1a to 917c12d Compare May 19, 2022 19:24

sorenlouv approved these changes May 20, 2022

View reviewed changes

justinkambic added 2 commits May 20, 2022 12:22

Migrate service list query out of APM.

60cba17

Delete obsolete e2e test.

0d280fa

justinkambic force-pushed the 454/service-list-migration branch from 7d07c26 to 0d280fa Compare May 20, 2022 16:22

Rename non-snakecase files.

5c2f9bd

justinkambic mentioned this pull request May 20, 2022

[UX Dashboard] Remove search url query from APM server #132654

Merged

justinkambic removed the v8.3.0 label May 23, 2022

Merge branch 'main' into 454/service-list-migration

335bbd5

shahzad31 reviewed May 23, 2022

View reviewed changes

x-pack/plugins/ux/public/services/data/get_es_filter.ts Show resolved Hide resolved

shahzad31 approved these changes May 23, 2022

View reviewed changes

justinkambic added 3 commits May 23, 2022 14:42

Merge branch 'main' into 454/service-list-migration

6b1689d

Add tests for new filter generator.

a0477c5

Merge branch '454/service-list-migration' of github.com:justinkambic/…

98724ab

…kibana into 454/service-list-migration

justinkambic enabled auto-merge (squash) May 23, 2022 19:01

Merge branch 'main' into 454/service-list-migration

f219fef

justinkambic merged commit 1886f39 into elastic:main May 24, 2022

justinkambic changed the title ~~[Synthetics] Migrate service list query out of APM~~ [UX Dashboard] Migrate service list query out of APM May 24, 2022

justinkambic added the auto-backport Deprecated - use backport:version if exact versions are needed label May 24, 2022

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 25, 2022

shahzad31 removed backport missing Added to PRs automatically when the are determined to be missing a backport. auto-backport Deprecated - use backport:version if exact versions are needed labels May 30, 2022

kibanamachine added the backport:skip This commit does not require backporting label May 30, 2022

shahzad31 deleted the 454/service-list-migration branch May 30, 2022 16:27

sorenlouv added v8.3.0 and removed v8.4.0 labels Jun 1, 2022

justinkambic mentioned this pull request Jun 28, 2022

[UX Dashboard] Unify processor event #135367

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UX Dashboard] Migrate service list query out of APM #132548

[UX Dashboard] Migrate service list query out of APM #132548

justinkambic commented May 19, 2022

elasticmachine commented May 19, 2022

justinkambic May 19, 2022

sorenlouv May 20, 2022

justinkambic May 20, 2022

shahzad31 May 20, 2022

justinkambic May 19, 2022

sorenlouv May 20, 2022

sorenlouv May 20, 2022

justinkambic May 20, 2022

sorenlouv May 20, 2022

justinkambic May 20, 2022

sorenlouv May 20, 2022

sorenlouv May 20, 2022

shahzad31 commented May 23, 2022

shahzad31 left a comment

justinkambic commented May 23, 2022

kibana-ci commented May 24, 2022

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented May 24, 2022

kibanamachine commented May 25, 2022

kibanamachine commented May 26, 2022

kibanamachine commented May 27, 2022

kibanamachine commented May 30, 2022

[UX Dashboard] Migrate service list query out of APM #132548

[UX Dashboard] Migrate service list query out of APM #132548

Conversation

justinkambic commented May 19, 2022

Summary

Testing this PR

Checklist

Risk Matrix

For maintainers

elasticmachine commented May 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahzad31 commented May 23, 2022

shahzad31 left a comment

Choose a reason for hiding this comment

justinkambic commented May 23, 2022

kibana-ci commented May 24, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented May 24, 2022

💔 All backports failed

Manual backport

Questions ?

kibanamachine commented May 25, 2022

kibanamachine commented May 26, 2022

kibanamachine commented May 27, 2022

kibanamachine commented May 30, 2022