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.

Sign up for GitHub

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

Jump to bottom

[Synthetics] Rename uptime plugin to synthetics #130037

Merged
merged 18 commits into from
Apr 20, 2022
Merged

[Synthetics] Rename uptime plugin to synthetics #130037

merged 18 commits into from
Apr 20, 2022

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Apr 12, 2022 •
edited
Loading

Summary

In preparation for new synthetics app

Rename uptime plugin to synthetics

Updated relevant code paths !

i will recommend reviewing by scrolling through changed files via pull request file tree
image

@shahzad31 shahzad31 marked this pull request as ready for review April 18, 2022 15:52
@shahzad31 shahzad31 requested review from a team as code owners April 18, 2022 15:52
@shahzad31 shahzad31 self-assigned this Apr 18, 2022
spalger
spalger approved these changes Apr 18, 2022
View reviewed changes
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

Operations owned changes LGTM

dominiqueclarke
dominiqueclarke reviewed Apr 18, 2022
View reviewed changes
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Looks great to me. Minimal comments.

x-pack/plugins/synthetics/common/constants/plugin.ts
@@ -10,13 +10,13 @@ import { i18n } from '@kbn/i18n';
export const PLUGIN = {
APP_ROOT_ID: 'react-uptime-root',
DESCRIPTION: i18n.translate('xpack.uptime.pluginDescription', {
defaultMessage: 'Uptime monitoring',
defaultMessage: 'Synthetics monitoring',
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are these two values displayed? Are they displayed to the end user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't think so.

Though the title goes here
image

x-pack/plugins/synthetics/kibana.json
@@ -1,6 +1,6 @@
{
"configPath": ["xpack", "uptime"],
"id": "uptime",
"id": "synthetics",
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we agreed not to change the id as of yet. I'm not against it, but wondering why you decided to go ahead and change it now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

feature id is what matters most, so i decided to go ahead with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering how that will impact alerting RBAC, here: https://github.com/elastic/kibana/blob/main/packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts#L24

Copy link
Contributor

Choose a reason for hiding this comment

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

@elastic/actionable-observability can you remind me if the values above are meant to be the plugin id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most likely feature id, since there are two from infra plugin logs and infrastructure

x-pack/plugins/translations/translations/ja-JP.json Outdated
@@ -28510,7 +28510,6 @@
"xpack.uptime.emptyStateError.notFoundPage": "ページが見つかりません",
"xpack.uptime.emptyStateError.title": "エラー",
"xpack.uptime.enableAlert.editAlert": "アラートを編集",
"xpack.uptime.featureRegistry.uptimeFeatureName": "アップタイム",
Copy link
Contributor

Choose a reason for hiding this comment

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

See this removed, but don't see a corresponding synthetics replacement added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

new translation will be generated for new keys auto.

jbudz
jbudz reviewed Apr 18, 2022
View reviewed changes
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

There's xpack.uptime.index in src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker, does this need to be updated too?

@shahzad31 shahzad31 added the release_note:skip Skip the PR/issue when compiling release notes label Apr 18, 2022
@shahzad31
Copy link
Contributor Author

There's xpack.uptime.index in src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker, does this need to be updated too?

we don't want to update that, we will keep uptime app/feature for a while. And potentially add new keys for the new app.

@shahzad31 shahzad31 enabled auto-merge (squash) April 20, 2022 16:29
dominiqueclarke
dominiqueclarke approved these changes Apr 20, 2022
View reviewed changes
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

@shahzad31 shahzad31 merged commit a4a0822 into elastic:main Apr 20, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 20, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
synthetics - 752 +752
uptime 752 - -752
total -0

Async chunks

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

id before after diff
synthetics - 783.7KB ⚠️ +783.7KB
uptime 783.5KB - -783.5KB
total +133.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
synthetics - 25.1KB +25.1KB
uptime 25.1KB - -25.1KB
total +43.0B
Unknown metric groups

async chunk count

id before after diff
synthetics - 15 +15
uptime 15 - -15
total -0

ESLint disabled in files

id before after diff
synthetics - 7 +7
uptime 7 - -7
total -0

ESLint disabled line counts

id before after diff
synthetics - 53 +53
uptime 53 - -53
total -0

References to deprecated APIs

id before after diff
synthetics - 34 +34
uptime 34 - -34
total -0

Total ESLint disabled count

id before after diff
synthetics - 60 +60
uptime 60 - -60
total -0

History

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

cc @shahzad31

kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
@shahzad31 @kibanamachine @kertal
[Synthetics] Rename uptime plugin to synthetics (elastic#130037)
c548b6b 
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@shahzad31 shahzad31 deleted the rename-synthetics branch July 12, 2022 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbudz jbudz jbudz left review comments

@dominiqueclarke dominiqueclarke dominiqueclarke approved these changes

@spalger spalger spalger approved these changes

Assignees

@shahzad31 shahzad31

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Synthetics v8.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@shahzad31 @kibana-ci @spalger @jbudz @dominiqueclarke @rashmivkulkarni @kibanamachine