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

[Observability Solution][Maintenance] Move to Emotion CSS, enable Telemetry and i18n ESLint rules for all Obs plugins #177785

Merged

Conversation

CoenWarmer
Copy link
Contributor

@CoenWarmer CoenWarmer commented Feb 26, 2024

Summary

This does two things:

  • Moves to Emotion CSS from .scss files
  • Enables Telemetry and i18n ESLint rules for all Observability apps

Why?

Move to Emotion CSS
There were four .scss files total in the 17 Observability plugins. Two of them were empty. The remaining two had one class each. By removing the two empty files and moving to Emotion, we can remove the lines pertaining to .scss files in the CODEOWNERS file.

Enabling Telemetry and i18n ESLint rules for all Observability apps
One of the reasons for consolidating Obs apps into one folder was to create a more consistent development experience across apps in the Observability org.

By changing the eslint rule config to enable the Telemetry and i18n ESLint rules on all .ts and tsx files in observability_solution, we enable the rule for 5 apps that did not have them enabled before and we ensure that the rule will immediately be enabled on new Observability applications at the moment of creation.

Related PRs:

@CoenWarmer CoenWarmer requested review from a team as code owners February 26, 2024 09:09
@botelastic botelastic bot added the Team:obs-ux-management Observability Management User Experience Team label Feb 26, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@CoenWarmer CoenWarmer added the release_note:skip Skip the PR/issue when compiling release notes label Feb 26, 2024
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@CoenWarmer CoenWarmer requested review from a team as code owners February 26, 2024 09:14
@CoenWarmer CoenWarmer changed the title [Observability Solution][Maintenance] Move to Emotion CSS, enable i18n ESLint rules for all Obs plugins [Observability Solution][Maintenance] Move to Emotion CSS, enable Telemetry and i18n ESLint rules for all Obs plugins Feb 26, 2024
@CoenWarmer CoenWarmer requested a review from a team as a code owner February 26, 2024 11:04
…Warmer/kibana into feat/observability-solution-cleanup
@@ -60,7 +60,11 @@ export const ListStatus = ({
}}
/>
}
actions={[<EuiButton onClick={onRetry}>{noDataRetryLabel}</EuiButton>]}
actions={[
<EuiButton data-test-subj="logsExplorerListStatusButton" onClick={onRetry}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Telemetry ESLint rule is configured to autofix, so the CI has added these.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<EuiButton data-test-subj="logsExplorerListStatusButton" onClick={onRetry}>
<EuiButton data-test-subj="logsExplorerListStatusRetryButton" onClick={onRetry}>

@CoenWarmer CoenWarmer force-pushed the feat/observability-solution-cleanup branch from 2369e2a to 50a0272 Compare February 26, 2024 11:10
@@ -110,7 +110,12 @@ export const AlertsPopover = () => {
{state.isAddRuleFlyoutOpen && addRuleFlyout}
<EuiPopover
button={
<EuiButtonEmpty onClick={togglePopover} iconType="arrowDown" iconSide="right">
<EuiButtonEmpty
data-test-subj="observabilityLogsExplorerAlertsPopoverAlertsButton"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Telemetry ESLint rule is configured to autofix, so the CI has added these.

@@ -90,7 +90,7 @@ pageLoadAssetSize:
licensing: 29004
links: 44490
lists: 22900
logsExplorer: 55000
logsExplorer: 57000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bundle was going 1.2kb over the limit

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

CSS change for obs-ux-management files (removing the news_feed.scss file and replacing with in-line emotion for the Obs overview news feed) LGTM

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for that

Copy link
Contributor

@formgeist formgeist left a comment

Choose a reason for hiding this comment

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

Obs design sign off

…ponents/flyout_detail/sub_components/highlight_section.tsx

Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani01@gmail.com>
Copy link
Contributor

@tonyghiani tonyghiani left a comment

Choose a reason for hiding this comment

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

logs_explorer changes LGTM, just left a couple of code suggestions for data-test-subj naming

@CoenWarmer CoenWarmer enabled auto-merge (squash) March 14, 2024 09:33
@CoenWarmer CoenWarmer requested a review from a team as a code owner March 14, 2024 10:15
@botelastic botelastic bot added the Team:obs-knowledge Observability Experience Knowledge team label Mar 14, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-knowledge-team (Team:obs-knowledge)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1432 1427 -5
logsExplorer 680 678 -2
observability 882 890 +8
total +1

Async chunks

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

id before after diff
infra 1.4MB 1.4MB -464.0B
logsExplorer 1.1MB 1.1MB +6.2KB
observability 896.2KB 901.9KB +5.7KB
observabilityAIAssistantApp 139.4KB 139.4KB +63.0B
observabilityLogsExplorer 153.0KB 153.1KB +70.0B
total +11.6KB

Page load bundle

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

id before after diff
logsExplorer 53.8KB 56.1KB +2.3KB
logsShared 221.9KB 221.9KB +54.0B
total +2.4KB

History

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

@CoenWarmer CoenWarmer merged commit fbc544d into elastic:main Mar 14, 2024
18 checks passed
@kibanamachine kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Mar 14, 2024
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:skip Skip the PR/issue when compiling release notes Team:obs-knowledge Observability Experience Knowledge team Team:obs-ux-management Observability Management User Experience Team v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet