[APM] Client new platform migration #64046

ogupte · 2020-04-21T09:07:44Z

Closes #32894.

migrates all files in the legacy plugin path for apm to the new plugin path at x-pack/plugins/apm.
replaces the legacy client setup with the new kibana platform setup.
implements a new API at /api/apm/index-pattern/title which was necessary since this value was no longer available as an injected meta data in legacy. I was unable to expose it from the plugin config, since it's really a config value from a dependency apm_oss, so it made sense to just pass it to the client via API response.
adds missing plugin dependencies features and triggers_actions_ui.
moves all scripts and cypress io code in apm/scripts and apm/e2e.
migrates to the new saved objects mappings API and conventions in apm/server/saved_objects

This PR does NOT:

change file/dir names to snake_case, instead it excludes it from the case checking task.
change the routing history scheme. At the top level, it will route into and out of APM using the platform's history object, and then under that, it still uses the hash history object for APM-specific routes. This should be address to take advantage of the global, platform history object in another PR.

For the reviewer's convenience, all substantive changes were limited to this commit: 9e72500

elasticmachine · 2020-04-22T00:13:24Z

Pinging @elastic/uptime (Team:uptime)

…kibana

…ims and legacy adapters.

ogupte · 2020-04-28T06:33:20Z

retest

dgieselaar · 2020-04-28T07:09:51Z

x-pack/plugins/apm/kibana.json

+    "data",
+    "home",
+    "licensing",
+    "triggers_actions_ui"


this should probably be in optional, with the other plugins necessary for alerting (alerting and actions).

actionTypeRegistry and alertTypeRegistry are required for <AlertsContextProvider>, so it will have to be required unless we do some kind of conditional rendering of context, but i don't think that will be good for downstream consumers of that context.

dgieselaar

No specific concerns from me, just a few qs and suggestions - great work @ogupte!

dgieselaar · 2020-04-28T07:10:19Z

x-pack/plugins/apm/public/application/index.tsx

+};
+
+/**
+ * This module will be loaded asynchronously to reduce the bundle size of your plugin's main bundle.


should we reword this comment?

dgieselaar · 2020-04-28T08:02:34Z

x-pack/plugins/apm/server/lib/helpers/es_client.ts

@@ -20,7 +20,8 @@ import {
  ESSearchResponse
 } from '../../../typings/elasticsearch';
 import { OBSERVER_VERSION_MAJOR } from '../../../common/elasticsearch_fieldnames';
-import { pickKeys } from '../../../../../legacy/plugins/apm/public/utils/pickKeys';
+// eslint-disable-next-line @kbn/eslint/no-restricted-paths
+import { pickKeys } from '../../../public/utils/pickKeys';


should we move this to common?

dgieselaar · 2020-04-28T08:03:31Z

x-pack/plugins/apm/server/lib/errors/queries.test.ts

@@ -9,7 +9,7 @@ import { getErrorGroups } from './get_error_groups';
 import {
  SearchParamsMock,
  inspectSearchParams
-} from '../../../../../legacy/plugins/apm/public/utils/testHelpers';
+} from '../../../public/utils/testHelpers';


not related to this PR, but feels like this file should not be in public anyway. there's a bunch of node imports there that will break when it's used in storybook for instance.

Pretty sure it does, in fact, break in storybook.

dgieselaar · 2020-04-28T08:09:51Z

x-pack/plugins/infra/public/components/logging/log_entry_flyout/log_entry_actions_menu.tsx

@@ -9,7 +9,8 @@ import { EuiButtonEmpty, EuiContextMenuItem, EuiContextMenuPanel, EuiPopover } f
 import { FormattedMessage } from '@kbn/i18n/react';
 import React, { useMemo } from 'react';
 import { useVisibilityState } from '../../../utils/use_visibility_state';
-import { getTraceUrl } from '../../../../../../legacy/plugins/apm/public/components/shared/Links/apm/ExternalLinks';
+// eslint-disable-next-line @kbn/eslint/no-restricted-paths
+import { getTraceUrl } from '../../../../../apm/public/components/shared/Links/apm/ExternalLinks';


this should be exported from /apm/public to prevent the warning no?

dgieselaar · 2020-04-28T08:11:43Z

x-pack/plugins/apm/typings/common.d.ts

 // .svg
-import '../../../legacy/plugins/canvas/types/webpack';
+// import '../../../legacy/plugins/canvas/types/webpack';


can we remove this then?

dgieselaar · 2020-04-28T08:15:46Z

x-pack/plugins/apm/server/lib/index_pattern/get_apm_index_pattern_titles.ts

+ */
+import { APMRequestHandlerContext } from '../../routes/typings';
+
+export async function getApmIndexPatternTitle(


why is this async?

dgieselaar · 2020-04-28T08:22:07Z

x-pack/plugins/apm/public/plugin.ts

+  PluginStartContract as AlertingPluginPublicStart
+} from '../../alerting/public';
+// eslint-disable-next-line @kbn/eslint/no-restricted-paths
+import { PluginSetupContract as FeaturesPluginSetup } from '../../features/server';


are we importing the right file here? we don't need the one from features/public?

you're right, i was importing the wrong type. Also we don't even use the features plugin in the client app, so we could just omit it

dgieselaar · 2020-04-28T08:32:23Z

x-pack/plugins/apm/public/index.ts

+} from '../../../../src/core/public';
+import { ApmPlugin, ApmPluginSetup, ApmPluginStart } from './plugin';
+
+export interface ConfigSchema {


It's not entirely clear to me when reading this what this type is based on. It looks like they are the properties we want to expose from the server config to the browser, but I'm not seeing the connection to runtime code. Can we tie them together in some way?

rylnd

Did not exercise the APM migration itself, but SIEM changes look good. Thank you for keeping those references up to date 👍

elasticmachine · 2020-04-28T17:15:09Z

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

smith

Great work! 👍 from me once tests pass and conflicts fixed.

…ation

spalger

Operations: path updates in config LGTM

- exposes getTraceUrl as a plugin static export of apm/public and updates import in infra - fixes FeaturesPluginSetup import in apm/public app - renames get_apm_index_pattern_titles -> get_apm_index_pattern_title - getApmIndexPatternTitle is now a synchronous getter function - removes unused comments and xpack.apm.apmForESDescription i18n translations

ogupte · 2020-04-29T08:25:49Z

retest

…migration

plugin start to when the Home screen first renders

FrankHassanabad

For SIEM changes this is 👍

jasonrhodes

Nice! Congrats 👏

snide

I don't think you need these files anymore. You might also want to look into the css files you moved over. A few of them reference selectors that no longer exist like kuiLocalNav. I've only done a code scan of these changes.

x-pack/plugins/apm/public/application/index.tsx

x-pack/plugins/apm/public/style/index.scss

snide

LGTM. Thanks for the changes

sorenlouv · 2020-04-30T09:44:48Z

src/dev/precommit_hook/casing_check_config.js

+  // TODO fix file names in APM to remove these
+  'x-pack/plugins/apm/public/**/*',
+  'x-pack/plugins/apm/scripts/**/*',
+  'x-pack/plugins/apm/e2e/**/*',


Is there any automated way (eslint --fix) to update the filenames?

not for scripts or e2e. for public, i started working on a code mod to update all the imports with the new paths, but then it was suggested by the platform team to ignore them for now and address it in another PR

sorenlouv · 2020-04-30T10:06:39Z

x-pack/plugins/apm/public/services/rest/index_pattern.ts

+  return await callApmApi({
+    pathname: '/api/apm/index_pattern/title'
+  });
+};


I don't think we want an endpoint for each config value the UI needs. Could we make this more generic so it returns all APM config values (supposing there is nothing confidential in them).

There's some opportunity for that since we also get the various apm_oss indices for index settings default values. But Let's address it in another tech debt issue. I'd like to keep this PR as minimal as possible.

Agree, I should have made it clear that this was for follow-up. We don't have to do it until the need arises (eg. we need to pass another config value to the UI)

paul-tavares

LGTM

Does not seem to have touched any Endpoint files.

- moving readOnly badge, and help extension setup to occure only when apm app is mounted - registering saved object types - also moved createStaticIndexPattern from a react useEffect on the APM home screen to when the app is mounted

kibanamachine · 2020-04-30T20:08:39Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c47f3b3

History

💔 Build #44326 failed 269f089
💔 Build #44300 failed eec8279
💔 Build #43643 failed 9fa801a
💔 Build #43291 failed 51c86cd
💔 Build #43304 failed 7d86a57

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

* migrate files from legacy path to new plugin path * update file paths to reflect migrated files * move minimal legacy client files back to legacy path in order to run kibana * Completes the full cutover to the new kibana platform removing all shims and legacy adapters. * Adds APM to ignored list for casing check. * - moves public/utils/pickKeys.ts to common/utils/pick_keys.ts - exposes getTraceUrl as a plugin static export of apm/public and updates import in infra - fixes FeaturesPluginSetup import in apm/public app - renames get_apm_index_pattern_titles -> get_apm_index_pattern_title - getApmIndexPatternTitle is now a synchronous getter function - removes unused comments and xpack.apm.apmForESDescription i18n translations * Moves automatic index pattern saved object creation from plugin start to when the Home screen first renders * removed unnecessary legacy css imports * fixed ci issues by: - moving readOnly badge, and help extension setup to occure only when apm app is mounted - registering saved object types - also moved createStaticIndexPattern from a react useEffect on the APM home screen to when the app is mounted

* [APM] Client new platform migration (#64046) * migrate files from legacy path to new plugin path * update file paths to reflect migrated files * move minimal legacy client files back to legacy path in order to run kibana * Completes the full cutover to the new kibana platform removing all shims and legacy adapters. * Adds APM to ignored list for casing check. * - moves public/utils/pickKeys.ts to common/utils/pick_keys.ts - exposes getTraceUrl as a plugin static export of apm/public and updates import in infra - fixes FeaturesPluginSetup import in apm/public app - renames get_apm_index_pattern_titles -> get_apm_index_pattern_title - getApmIndexPatternTitle is now a synchronous getter function - removes unused comments and xpack.apm.apmForESDescription i18n translations * Moves automatic index pattern saved object creation from plugin start to when the Home screen first renders * removed unnecessary legacy css imports * fixed ci issues by: - moving readOnly badge, and help extension setup to occure only when apm app is mounted - registering saved object types - also moved createStaticIndexPattern from a react useEffect on the APM home screen to when the app is mounted * [APM] fixes type declaration for ApmRoute

* upstream/master: (315 commits) [APM] Fix failing `ApmIndices` test (elastic#64965) [APM] Fix paths for ts optimization script (elastic#65012) Use HDR for percentiles (elastic#64758) [EPM] fix updates available filter (elastic#64957) [Uptime] Certificates page (elastic#64059) load lens app lazily (elastic#64769) [legacy/server/config] remove unnecessary deps for simple helper (elastic#64954) Fixed alert Edit flyout shows the error message when one of this actions has a preconfigured action type (elastic#64742) [data.search.aggs] Remove legacy aggs APIs. (elastic#64719) Fixed `AddAlert` flyout does not immediately update state to reflect new props (elastic#64927) [Discover] Show doc viewer action buttons on focus (elastic#64912) [EPM] restrict package install endpoint from installing/updating to old packages (elastic#64932) [Metrics UI] Add inventory metric threshold alerts (elastic#64292) [Canvas] Adds edit menu (elastic#64738) [Canvas] Adds refresh and autoplay options to view menu (elastic#64375) [Lens] Trigger a filter action on click in datatable visualization (elastic#63840) [SIEM][CASE] Refactor Connectors - Jira Connector (elastic#63450) [APM] Client new platform migration (elastic#64046) [Monitoring] NP Migration complete client cutover (elastic#62908) Ingest Node Pipelines UI (elastic#62321) ...

…or-part-mvp-2 * 'master' of github.com:elastic/kibana: (51 commits) [APM] Fix failing `ApmIndices` test (elastic#64965) [APM] Fix paths for ts optimization script (elastic#65012) Use HDR for percentiles (elastic#64758) [EPM] fix updates available filter (elastic#64957) [Uptime] Certificates page (elastic#64059) load lens app lazily (elastic#64769) [legacy/server/config] remove unnecessary deps for simple helper (elastic#64954) Fixed alert Edit flyout shows the error message when one of this actions has a preconfigured action type (elastic#64742) [data.search.aggs] Remove legacy aggs APIs. (elastic#64719) Fixed `AddAlert` flyout does not immediately update state to reflect new props (elastic#64927) [Discover] Show doc viewer action buttons on focus (elastic#64912) [EPM] restrict package install endpoint from installing/updating to old packages (elastic#64932) [Metrics UI] Add inventory metric threshold alerts (elastic#64292) [Canvas] Adds edit menu (elastic#64738) [Canvas] Adds refresh and autoplay options to view menu (elastic#64375) [Lens] Trigger a filter action on click in datatable visualization (elastic#63840) [SIEM][CASE] Refactor Connectors - Jira Connector (elastic#63450) [APM] Client new platform migration (elastic#64046) [Monitoring] NP Migration complete client cutover (elastic#62908) Ingest Node Pipelines UI (elastic#62321) ... # Conflicts: # x-pack/plugins/ingest_pipelines/common/types.ts # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_fields.tsx # x-pack/plugins/ingest_pipelines/public/shared_imports.ts

* master: (44 commits) onEvent prop for expression component (elastic#64995) [APM] Fix failing `ApmIndices` test (elastic#64965) [APM] Fix paths for ts optimization script (elastic#65012) Use HDR for percentiles (elastic#64758) [EPM] fix updates available filter (elastic#64957) [Uptime] Certificates page (elastic#64059) load lens app lazily (elastic#64769) [legacy/server/config] remove unnecessary deps for simple helper (elastic#64954) Fixed alert Edit flyout shows the error message when one of this actions has a preconfigured action type (elastic#64742) [data.search.aggs] Remove legacy aggs APIs. (elastic#64719) Fixed `AddAlert` flyout does not immediately update state to reflect new props (elastic#64927) [Discover] Show doc viewer action buttons on focus (elastic#64912) [EPM] restrict package install endpoint from installing/updating to old packages (elastic#64932) [Metrics UI] Add inventory metric threshold alerts (elastic#64292) [Canvas] Adds edit menu (elastic#64738) [Canvas] Adds refresh and autoplay options to view menu (elastic#64375) [Lens] Trigger a filter action on click in datatable visualization (elastic#63840) [SIEM][CASE] Refactor Connectors - Jira Connector (elastic#63450) [APM] Client new platform migration (elastic#64046) [Monitoring] NP Migration complete client cutover (elastic#62908) ...

botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Apr 22, 2020

ogupte force-pushed the apm-32894-np-migration branch from 51c86cd to 7d86a57 Compare April 27, 2020 07:36

ogupte added 5 commits April 27, 2020 22:12

migrate files from legacy path to new plugin path

c7ec626

update file paths to reflect migrated files

cd7406f

move minimal legacy client files back to legacy path in order to run …

59b2033

…kibana

Completes the full cutover to the new kibana platform removing all sh…

9e72500

…ims and legacy adapters.

Adds APM to ignored list for casing check.

9fa801a

ogupte force-pushed the apm-32894-np-migration branch from 7d86a57 to 9fa801a Compare April 28, 2020 05:52

ogupte marked this pull request as ready for review April 28, 2020 06:25

ogupte requested review from a team as code owners April 28, 2020 06:25

ogupte added release_note:skip Skip the PR/issue when compiling release notes v7.8 labels Apr 28, 2020

dgieselaar reviewed Apr 28, 2020

View reviewed changes

dgieselaar approved these changes Apr 28, 2020

View reviewed changes

rylnd approved these changes Apr 28, 2020

View reviewed changes

andrewvc added Team:APM All issues that need APM UI Team support and removed Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Apr 28, 2020

smith approved these changes Apr 28, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into apm-32894-np-migr…

bc06a6d

…ation

spalger approved these changes Apr 29, 2020

View reviewed changes

ogupte added v7.8.0 and removed v7.8 labels Apr 29, 2020

ogupte added 3 commits April 29, 2020 08:11

Merge branch 'master' of github.com:elastic/kibana into apm-32894-np-…

72258bb

…migration

Merge branch 'master' of github.com:elastic/kibana into apm-32894-np-…

cf5d8d0

…migration

Moves automatic index pattern saved object creation from

eec8279

plugin start to when the Home screen first renders

ogupte requested a review from a team as a code owner April 30, 2020 00:10

FrankHassanabad approved these changes Apr 30, 2020

View reviewed changes

jasonrhodes approved these changes Apr 30, 2020

View reviewed changes

snide suggested changes Apr 30, 2020

View reviewed changes

x-pack/plugins/apm/public/application/index.tsx Outdated Show resolved Hide resolved

x-pack/plugins/apm/public/style/index.scss Outdated Show resolved Hide resolved

removed unnecessary legacy css imports

269f089

snide approved these changes Apr 30, 2020

View reviewed changes

sorenlouv approved these changes Apr 30, 2020

View reviewed changes

sorenlouv reviewed Apr 30, 2020

View reviewed changes

paul-tavares approved these changes Apr 30, 2020

View reviewed changes

smith mentioned this pull request Apr 30, 2020

Navigating away from APM using Actions menu causes a full page refresh #64903

Closed

fixed ci issues by:

c47f3b3

- moving readOnly badge, and help extension setup to occure only when apm app is mounted - registering saved object types - also moved createStaticIndexPattern from a react useEffect on the APM home screen to when the app is mounted

ogupte merged commit 16ba937 into elastic:master Apr 30, 2020

ogupte mentioned this pull request Apr 30, 2020

[7.x] [APM] Client new platform migration (#64046) #64958

Merged

ogupte mentioned this pull request Apr 30, 2020

[APM] Fix legacy routing to prevent full page refresh between Observability apps #64804

Closed

dgieselaar mentioned this pull request May 2, 2020

[APM] Fix paths for ts optimization script #65012

Merged

sorenlouv mentioned this pull request May 4, 2020

[APM] Update path to APM app after move to NP #65034

Merged

sorenlouv added apm:test-plan-7.8.0 and removed apm:test-plan-7.8.0 labels May 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Client new platform migration #64046

[APM] Client new platform migration #64046

ogupte commented Apr 21, 2020 •

edited

Loading

elasticmachine commented Apr 22, 2020

ogupte commented Apr 28, 2020

dgieselaar Apr 28, 2020

ogupte Apr 29, 2020

dgieselaar left a comment

dgieselaar Apr 28, 2020

dgieselaar Apr 28, 2020

dgieselaar Apr 28, 2020

smith Apr 28, 2020

dgieselaar Apr 28, 2020

dgieselaar Apr 28, 2020

dgieselaar Apr 28, 2020

dgieselaar Apr 28, 2020

ogupte Apr 29, 2020

dgieselaar Apr 28, 2020

rylnd left a comment

elasticmachine commented Apr 28, 2020

smith left a comment

spalger left a comment

ogupte commented Apr 29, 2020

FrankHassanabad left a comment

jasonrhodes left a comment

snide left a comment

snide left a comment

sorenlouv Apr 30, 2020

ogupte Apr 30, 2020 •

edited

Loading

sorenlouv Apr 30, 2020

ogupte Apr 30, 2020

sorenlouv Apr 30, 2020 •

edited

Loading

paul-tavares left a comment

kibanamachine commented Apr 30, 2020

[APM] Client new platform migration #64046

[APM] Client new platform migration #64046

Conversation

ogupte commented Apr 21, 2020 • edited Loading

elasticmachine commented Apr 22, 2020

ogupte commented Apr 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgieselaar left a 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

rylnd left a comment

Choose a reason for hiding this comment

elasticmachine commented Apr 28, 2020

smith left a comment

Choose a reason for hiding this comment

spalger left a comment

Choose a reason for hiding this comment

ogupte commented Apr 29, 2020

FrankHassanabad left a comment

Choose a reason for hiding this comment

jasonrhodes left a comment

Choose a reason for hiding this comment

snide left a comment

Choose a reason for hiding this comment

snide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogupte Apr 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorenlouv Apr 30, 2020 • edited Loading

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 30, 2020

💚 Build Succeeded

History

ogupte commented Apr 21, 2020 •

edited

Loading

ogupte Apr 30, 2020 •

edited

Loading

sorenlouv Apr 30, 2020 •

edited

Loading