-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Platform Onboarding] Elastic cloud migration assistant page #145523
[Platform Onboarding] Elastic cloud migration assistant page #145523
Conversation
eb1453a
to
8243bc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @claracruz! I took a quick look through the code and left some initial comments. Did not test locally, but screenshots look great so far 👍
src/plugins/home/public/application/components/add_data/add_data.tsx
Outdated
Show resolved
Hide resolved
src/plugins/home/public/application/components/add_data/add_data.tsx
Outdated
Show resolved
Hide resolved
src/plugins/home/public/application/components/add_data/add_data.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/kibana.json
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/plugin.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/common/index.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/plugin.ts
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,3 @@ | |||
# Cloud Data Migration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have much context on the cloud_integrations
directory in Kibana. Do you know if there are any guidelines on what should live under here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell it seems to be all cloud related implementations. e.g. Gainsight, DriftChat, FullStory, Cloud links e.t.c
@claracruz Don't want to do this on your PR myself, but looks like we might need a merge from upstream? I'll be happy to dive in via my local once again when the errors are gone. |
6147189
to
74f0a51
Compare
No worries on the sidebar... that can stay. Could just move the layout to a subdued I'll need to get the writers to do a pass on the content. And, I'll send you a new image, that SVG looks like it got corrupt when I compressed it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@claracruz hope it's ok I'm adding my changes this way. There are a couple that I'd like to get input on still.
src/plugins/home/public/application/components/add_data/add_data.tsx
Outdated
Show resolved
Hide resolved
src/plugins/home/public/application/components/add_data/add_data.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
6085a46
to
cf1602f
Compare
2795c8c
to
9cdf0ae
Compare
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @claracruz! This looks really great. I tested locally, and left a couple more comments in the code.
I also updated your branch and reviewed the CI errors. A couple things:
- I think you need to add a
.i18nrc.json
file to your plugin for translations. It should look something like this:
{
"prefix": "cloudDataMigration",
"paths": {
"cloudDataMigration": "."
},
"translations": []
}
Then, you will also need to update the ids
of any i18n strings. You can run node scripts/i18n_check.js
to verify translations when you're done.
- You need to update the
limits.yml
file since this is a new plugin. To do so, you can run:
node scripts/build_kibana_platform_plugins.js --update-limits
-
You need to update some jest snapshots in the
home
plugin. It looks like the failing tests are in:src/plugins/home/public/application/components/add_data/add_data.test.tsx
-
The
plugin.test.ts
tests are also failing. You can run jest tests in kibana via:npm run test:jest <path_to_test_file>
-
There appears to be a handful of functional tests that rely on the side nav in Stack Management to be in a particular order. Now that the "Migrate" link has been added, those are failing. There's quite a few here, so let me know if you need help going through them all. More about Kibana testing here - https://www.elastic.co/guide/en/kibana/current/development-tests.html.
-
There's another CI error that appears related to the
cloud_integrations
directory. I'm not sure about this one yet. Let's resolve all the other issues first, and if it's still an issue, I can help revisit.
ERROR Error: ENOENT: no such file or directory, stat '/dev/shm/builds/kb-n2-16-spot-721a7f5a24dec03f/elastic/kibana-pull-request/kibana/x-pack/plugins/cloud_integrations/features/tsconfig.json'
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
src/plugins/home/public/application/components/add_data/add_data.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_integrations/cloud_data_migration/public/application/components/app.tsx
Outdated
Show resolved
Hide resolved
@claracruz let's also not forget about the telemetry requirements. We can discuss more offline, but just leaving a comment here so I don't forget 😄 |
b3e7f6c
to
3d4818a
Compare
@claracruz Can you please post screenshots with the updated text? I'll do a final copy review. |
Build migration info page Update home page navigation Implemment breadcrumbs Remove unneeded typings Remove unneeded code and comments Fix breadcrumb Remove unnecessary component Fix newline at the end of files Update breadcrumb label
Update tests, styles and images Update copy
6b67efd
to
c7ebadc
Compare
f16871f
to
c63cb0f
Compare
Pinging @elastic/platform-onboarding (Team:Journey/Onboarding) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work @claracruz! I opened #147474 and #147462 to track some follow-up work.
We should also update the CODEOWNERS
file and assign our team as owner of this new plugin. I can open up a separate PR to address that.
<EuiSpacer size="l" /> | ||
|
||
<div> | ||
<EuiButton fill={true} target="_blank" href="https://ela.st/cloud-migration"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll want to track telemetry here; opened #147474 to follow up.
management: { | ||
data: [PLUGIN_ID], | ||
}, | ||
catalogue: [PLUGIN_ID], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is required. I'll open up a follow-up PR to fix.
Hi @gchaps, here are the latest screenshots. Let us know if there are any further copy changes that need to be made. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ATM all plugins under /x-pack/plugins/cloud_integrations/
are identified as being owned by Core (which is also what triggered the review from our team):
Line 313 in 3e49992
/x-pack/plugins/cloud_integrations/ @elastic/kibana-core |
I think you may need to change that, as I assume Core isn't supposed to be owning the new plugin you're introducing in this PR, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GX code changes LGTM.
@pgayvallet Ah, I didn't realize that there was already an existing owner for that directory. Thanks for pointing that out. I made a comment in #145523 (review) that the CODEOWNERS file needs to be updated. Would it be OK if I follow up with this change in a separate PR? The engineer who opened this is out until January and I would like to get this merged if possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
Fixes #141124
This PR adds a new static migration page in self-managed Kibana instances. This page provides self-managed users with a link to migration instructions for migrating their self-managed cluster over to Elastic Cloud.
A new landing page is added to the Management plugin with a page that links to the migration assistant instructions. The url for the new page is
/app/management/data/migrate_data
.Release Note
Self-managed Kibana instances now have a link to migration instructions for migrating their self-managed cluster over to Elastic Cloud.
Screenshots
Homepage
Landing page
Checklist
Delete any items that are not applicable to this PR.
Risk Matrix