[Logs Shared] Move LogStream and LogView into new shared plugin#161151
Merged
tonyghiani merged 87 commits intoelastic:mainfrom Jul 5, 2023
Merged
Conversation
…ghiani/kibana into 159128-split-log-stream-into-plugin
…ghiani/kibana into 159128-split-log-stream-into-plugin
…ghiani/kibana into 159128-split-log-stream-into-plugin
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
tonyghiani
added
Team:Infra Monitoring UI - DEPRECATED
Contributor
|
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
sorenlouv
approved these changes
Jul 4, 2023
maryam-saeidi
approved these changes
Jul 4, 2023
juliaElastic
approved these changes
Jul 4, 2023
added 2 commits
July 4, 2023 10:26
…ub.com:tonyghiani/kibana into tonyghiani-159128-split-log-stream-into-plugin
sphilipse
approved these changes
Jul 4, 2023
sabarasaba
approved these changes
Jul 4, 2023
Kerry350
approved these changes
Jul 4, 2023
Ikuni17
approved these changes
Jul 4, 2023
💚 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
References to deprecated APIs
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Jul 5, 2023
* main: (354 commits) [Synthetics] Overview page fix last refresh value display (elastic#161086) [Synthetics] Remove TLS alert option for ICMP monitor (elastic#161173) fixing the path of manifets for hints autodiscover (elastic#161075) [Fleet] Fix permissions in integrations Assets page (elastic#161233) Update publicBaseUrl warning id (elastic#161204) [ML] Fix Anomaly Explorer URL for alerting context with non-default space (elastic#160899) [Enterprise Search]Add 404 error handling for mappings and documents endpoints (elastic#161203) [Logs Shared] Move LogStream and LogView into new shared plugin (elastic#161151) [Security Solutions] Fix CellActions component should hide ShowTopN action for nested fields (elastic#159645) [SecuritySolutions] Remove filter actions from Cases alerts table and fix show_top_n action (elastic#161150) [Infrastructure UI] Add strict payload validation to inventory_views endpoint (elastic#160852) [api-docs] 2023-07-05 Daily api_docs build (elastic#161225) Fix errors in custom metric payload in SLO dev docs (elastic#161141) [data views] Fix overwrite param for create (elastic#160953) [Synthetics] Perform params API HTTP migration (elastic#160575) [Cloud Security][FTR]Refactor API FTR to use .to.eql instead of .to.be (elastic#160694) Have SLO routes return a 403 instead of a 400 when user has an insufficient license (elastic#161193) [Discover] Fix shared links flaky test (elastic#161172) [ftr] Improve FTR error handling for NoSuchSessionError (elastic#161025) skip flaky suite (elastic#151981) ...
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Jul 5, 2023
* main: (354 commits) [Synthetics] Overview page fix last refresh value display (elastic#161086) [Synthetics] Remove TLS alert option for ICMP monitor (elastic#161173) fixing the path of manifets for hints autodiscover (elastic#161075) [Fleet] Fix permissions in integrations Assets page (elastic#161233) Update publicBaseUrl warning id (elastic#161204) [ML] Fix Anomaly Explorer URL for alerting context with non-default space (elastic#160899) [Enterprise Search]Add 404 error handling for mappings and documents endpoints (elastic#161203) [Logs Shared] Move LogStream and LogView into new shared plugin (elastic#161151) [Security Solutions] Fix CellActions component should hide ShowTopN action for nested fields (elastic#159645) [SecuritySolutions] Remove filter actions from Cases alerts table and fix show_top_n action (elastic#161150) [Infrastructure UI] Add strict payload validation to inventory_views endpoint (elastic#160852) [api-docs] 2023-07-05 Daily api_docs build (elastic#161225) Fix errors in custom metric payload in SLO dev docs (elastic#161141) [data views] Fix overwrite param for create (elastic#160953) [Synthetics] Perform params API HTTP migration (elastic#160575) [Cloud Security][FTR]Refactor API FTR to use .to.eql instead of .to.be (elastic#160694) Have SLO routes return a 403 instead of a 400 when user has an insufficient license (elastic#161193) [Discover] Fix shared links flaky test (elastic#161172) [ftr] Improve FTR error handling for NoSuchSessionError (elastic#161025) skip flaky suite (elastic#151981) ...
Kerry350
added a commit
that referenced
this pull request
Aug 17, 2023
## Summary As part of #161151 a [selection of component imports were made lazy](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/public/index.ts#L52) and wrapped with a [`dynamic` wrapper component](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/common/dynamic.tsx#L22). Unfortunately some of these imports did not adhere to the rules of React's `lazy` imports (needing a `default` export, no named imports etc), and the `dynamic` wrapper seems to have suppressed error information that would have been available via using `lazy` directly. Only the anomaly and categories log entry examples (in the expanded rows) were affected by this, as the stream and embeddable import from locations that were backed by a `default` export (and those top level components don't import from that particular index file lower in the hierarchy). For imports that weren't backed by a `default` I've added them, and where necessary moved components to new files if needed (since it's one `default` per file). Also open to suggestions of ways we can alter the `<dynamic />` component and maintain the error safety 🤔 ## Examples Without these changes:  Warning using `lazy` directly without the `dynamic` wrapper:  ## Testing - Check all instances render correctly (stream, embeddable uses, and ML page log entry examples).
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Aug 17, 2023
## Summary As part of elastic#161151 a [selection of component imports were made lazy](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/public/index.ts#L52) and wrapped with a [`dynamic` wrapper component](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/common/dynamic.tsx#L22). Unfortunately some of these imports did not adhere to the rules of React's `lazy` imports (needing a `default` export, no named imports etc), and the `dynamic` wrapper seems to have suppressed error information that would have been available via using `lazy` directly. Only the anomaly and categories log entry examples (in the expanded rows) were affected by this, as the stream and embeddable import from locations that were backed by a `default` export (and those top level components don't import from that particular index file lower in the hierarchy). For imports that weren't backed by a `default` I've added them, and where necessary moved components to new files if needed (since it's one `default` per file). Also open to suggestions of ways we can alter the `<dynamic />` component and maintain the error safety 🤔 ## Examples Without these changes:  Warning using `lazy` directly without the `dynamic` wrapper:  ## Testing - Check all instances render correctly (stream, embeddable uses, and ML page log entry examples). (cherry picked from commit a96785c)
kibanamachine
added a commit
that referenced
this pull request
Aug 17, 2023
…4180) # Backport This will backport the following commits from `main` to `8.10`: - [[Logs] Amend lazy imports in logs_shared plugin (#164102)](#164102) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kerry Gallagher","email":"kerry.gallagher@elastic.co"},"sourceCommit":{"committedDate":"2023-08-17T13:44:57Z","message":"[Logs] Amend lazy imports in logs_shared plugin (#164102)\n\n## Summary\r\n\r\nAs part of #161151 a [selection of\r\ncomponent imports were made\r\nlazy](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/public/index.ts#L52)\r\nand wrapped with a [`dynamic` wrapper\r\ncomponent](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/common/dynamic.tsx#L22).\r\nUnfortunately some of these imports did not adhere to the rules of\r\nReact's `lazy` imports (needing a `default` export, no named imports\r\netc), and the `dynamic` wrapper seems to have suppressed error\r\ninformation that would have been available via using `lazy` directly.\r\n\r\nOnly the anomaly and categories log entry examples (in the expanded\r\nrows) were affected by this, as the stream and embeddable import from\r\nlocations that were backed by a `default` export (and those top level\r\ncomponents don't import from that particular index file lower in the\r\nhierarchy). For imports that weren't backed by a `default` I've added\r\nthem, and where necessary moved components to new files if needed (since\r\nit's one `default` per file).\r\n\r\nAlso open to suggestions of ways we can alter the `<dynamic />`\r\ncomponent and maintain the error safety 🤔\r\n\r\n## Examples\r\n\r\nWithout these changes:\r\n\r\n\r\n\r\nWarning using `lazy` directly without the `dynamic` wrapper:\r\n\r\n\r\n\r\n## Testing\r\n\r\n- Check all instances render correctly (stream, embeddable uses, and ML\r\npage log entry examples).","sha":"a96785cd2cf6dc28b6d786423e4604a7aee10c97","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Feature:Logs UI","Team:Infra Monitoring UI","v8.10.0","v8.11.0"],"number":164102,"url":"https://github.com/elastic/kibana/pull/164102","mergeCommit":{"message":"[Logs] Amend lazy imports in logs_shared plugin (#164102)\n\n## Summary\r\n\r\nAs part of #161151 a [selection of\r\ncomponent imports were made\r\nlazy](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/public/index.ts#L52)\r\nand wrapped with a [`dynamic` wrapper\r\ncomponent](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/common/dynamic.tsx#L22).\r\nUnfortunately some of these imports did not adhere to the rules of\r\nReact's `lazy` imports (needing a `default` export, no named imports\r\netc), and the `dynamic` wrapper seems to have suppressed error\r\ninformation that would have been available via using `lazy` directly.\r\n\r\nOnly the anomaly and categories log entry examples (in the expanded\r\nrows) were affected by this, as the stream and embeddable import from\r\nlocations that were backed by a `default` export (and those top level\r\ncomponents don't import from that particular index file lower in the\r\nhierarchy). For imports that weren't backed by a `default` I've added\r\nthem, and where necessary moved components to new files if needed (since\r\nit's one `default` per file).\r\n\r\nAlso open to suggestions of ways we can alter the `<dynamic />`\r\ncomponent and maintain the error safety 🤔\r\n\r\n## Examples\r\n\r\nWithout these changes:\r\n\r\n\r\n\r\nWarning using `lazy` directly without the `dynamic` wrapper:\r\n\r\n\r\n\r\n## Testing\r\n\r\n- Check all instances render correctly (stream, embeddable uses, and ML\r\npage log entry examples).","sha":"a96785cd2cf6dc28b6d786423e4604a7aee10c97"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/164102","number":164102,"mergeCommit":{"message":"[Logs] Amend lazy imports in logs_shared plugin (#164102)\n\n## Summary\r\n\r\nAs part of #161151 a [selection of\r\ncomponent imports were made\r\nlazy](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/public/index.ts#L52)\r\nand wrapped with a [`dynamic` wrapper\r\ncomponent](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/common/dynamic.tsx#L22).\r\nUnfortunately some of these imports did not adhere to the rules of\r\nReact's `lazy` imports (needing a `default` export, no named imports\r\netc), and the `dynamic` wrapper seems to have suppressed error\r\ninformation that would have been available via using `lazy` directly.\r\n\r\nOnly the anomaly and categories log entry examples (in the expanded\r\nrows) were affected by this, as the stream and embeddable import from\r\nlocations that were backed by a `default` export (and those top level\r\ncomponents don't import from that particular index file lower in the\r\nhierarchy). For imports that weren't backed by a `default` I've added\r\nthem, and where necessary moved components to new files if needed (since\r\nit's one `default` per file).\r\n\r\nAlso open to suggestions of ways we can alter the `<dynamic />`\r\ncomponent and maintain the error safety 🤔\r\n\r\n## Examples\r\n\r\nWithout these changes:\r\n\r\n\r\n\r\nWarning using `lazy` directly without the `dynamic` wrapper:\r\n\r\n\r\n\r\n## Testing\r\n\r\n- Check all instances render correctly (stream, embeddable uses, and ML\r\npage log entry examples).","sha":"a96785cd2cf6dc28b6d786423e4604a7aee10c97"}},{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Kerry Gallagher <kerry.gallagher@elastic.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📓 Summary
Closes #159128
Due to a dependencies issue when disabling a plugin in serverless mode, the LogStream feature and related logic were disabled for every consumer.
We decided to split this shared component and endpoint into their own plugin of shared logs utilities, reducing to the minimum the required dependency that could disable the plugin.
What we moved can be summarized with:
infrastructure-monitoring-log-viewsaved object definition and registration<LogStream />component<ScrollableLogTextStreamView />component and related logicinfraplugin.🤓 Review hints
Most of the changes are just renaming and moving stuff into the new plugin, but for some operations was required to implement new logic, which may deserve a more critical review:
plugin.tsfiles for theinfraandlogs_sharedplugins. The new plugin now registers the fallback actions to retrieve a source configuration if there's no stored log view. It also set the configuration for the message field and registers the log view saved object.logEntriesDomainhas also been moved inside the new plugin, but is also used by the logs-analysis endpoints, so it is exposed by the logs_shared plugin and consumed byinfra.👣 Following steps
We currently are still using the
observabilityplugin for consuming the CoPilot feature on our LogsStream flyout.The plugin dependency is marked as optional, so disabling the
observabilityplugin in a serverless environment won't disable also the exposed features in this new plugin, but it'll affect only the CoPilot feature, which won't be loaded.In future, would be nice to extract the CoPilot feature into its own package/plugin, so that also serverless projects can consume it without depending on `observability.