[Discover / Logs] Add new "Saved Search component" #199787
Merged
Conversation
This file contains 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
Kerry350
force-pushed
the
obs-saved-search-embeddable-poc
branch
2 times, most recently
from
45ae4a7 to
c1b0411
Compare
Kerry350
force-pushed
the
obs-saved-search-embeddable-poc
branch
from
2cd148a to
e6fd5f5
Compare
Kerry350
added
release_note:skip
|
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs) |
botelastic
bot
added
ci:project-deploy-observability
|
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
nreese
approved these changes
Nov 20, 2024
tonyghiani
reviewed
Nov 21, 2024
packages/kbn-saved-search-component/src/components/saved_search.tsx
Outdated
Show resolved
Hide resolved
packages/kbn-saved-search-component/src/components/saved_search.tsx
Outdated
Show resolved
Hide resolved
packages/kbn-saved-search-component/src/components/saved_search.tsx
Outdated
Show resolved
Hide resolved
packages/kbn-saved-search-component/src/components/saved_search.tsx
Outdated
Show resolved
Hide resolved
src/plugins/discover/public/embeddable/initialize_search_embeddable_api.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/observability_solution/apm/public/components/app/service_logs/index.tsx
Outdated
Show resolved
Hide resolved
Kerry350
force-pushed
the
obs-saved-search-embeddable-poc
branch
from
ab8f032 to
5b05acf
Compare
|
@tonyghiani Thanks for the review, I've responded to your feedback 👍 Regarding followups in this PR, there's nothing more to come. The mixing of columns will be handled by the Discover folks, and the top-right panel button was something I just wanted to point out. So this should be considered "done" 👍 |
tonyghiani
approved these changes
Nov 22, 2024
davismcphee
reviewed
Nov 26, 2024
src/plugins/discover/public/embeddable/components/saved_search_grid.tsx
Outdated
Show resolved
Hide resolved
src/plugins/discover/public/embeddable/initialize_search_embeddable_api.tsx
Show resolved
Hide resolved
…/kibana into obs-saved-search-embeddable-poc
|
@davismcphee Thanks for the review 🙏 I've responded to all the feedback, this is ready for another look. |
rudolf
approved these changes
Nov 27, 2024
| @@ -37,7 +37,8 @@ | |||
| "lens", | |||
| "maps", | |||
| "uiActions", | |||
| "logsDataAccess" | |||
| "logsDataAccess", | |||
| "savedSearch", | |||
There was a problem hiding this comment.
savedSearch is platform shared so does not impact solution independence ✅
davismcphee
approved these changes
Nov 27, 2024
There was a problem hiding this comment.
Thanks for the updates, and with that this LGTM on the Data Discovery side 👍 Great work, @Kerry350! I'm excited we've gotten to a point where we can rely on Discover directly for these things 🙂
src/plugins/discover/public/embeddable/initialize_search_embeddable_api.tsx
Show resolved
Hide resolved
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
History
cc @Kerry350 |
|
Manually removing |
|
Starting backport for target branches: 8.x |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
1 task
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
This was referenced Dec 3, 2024
Merged
Kerry350
added a commit
that referenced
this pull request
Dec 3, 2024
…202588) # Backport This will backport the following commits from `main` to `8.x`: - [[Discover / Logs] Add new "Saved Search component" (#199787)](#199787) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kerry Gallagher","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-29T23:09:24Z","message":"[Discover / Logs] Add new \"Saved Search component\" (#199787)\n\n## Summary\r\n\r\nImplements\r\nhttps://github.com/elastic/logs-dev/issues/111#issuecomment-2446470635.\r\n\r\nThis adds a new \"Saved Search component\". The component is a wrapper\r\naround the current Saved Search Embeddable, but uses\r\n`ReactEmbeddableRenderer` directly to render the embeddable outside of\r\nDashboard contexts. It monitors changes to things like `index`,\r\n`filters` etc and communicates these changes through the embeddable API.\r\n\r\nFor this PoC two locations were changed to use this component 1) Logs\r\nOverview flyout 2) APM Logs tab (when the Logs Overview isn't enabled\r\nvia advanced settings).\r\n\r\nThe component itself is technically beyond a PoC, and resides in it's\r\nown package. ~I'd like to get eyes from the Discover folks etc on the\r\napproach, and if we're happy I can fix the remaining known issues (apart\r\nfrom the mixing of columns point as I believe this exists on the roadmap\r\nanyway) and we can merge this for the initial two replacement points.~\r\n[Thanks Davis\r\n👌](https://github.com/elastic/logs-dev/issues/111#issuecomment-2475350199).\r\n\r\n`nonPersistedDisplayOptions` is added to facilitate some configurable\r\noptions via runtime state, but without the complexity of altering the\r\nactual saved search saved object.\r\n\r\nOn the whole I've tried to keep this as clean as possible whilst working\r\nwithin the embeddable framework, outside of a dashboard context.\r\n\r\n## Known issues\r\n\r\n- ~\"Flyout on flyout\" in the logs overview flyout (e.g. triggering the\r\ntable's flyout in this context).~ Fixed with `enableFlyout` option.\r\n- ~Filter buttons should be disabled via pills (e.g. in Summary\r\ncolumn).~ Fixed with `enableFilters` option.\r\n- Summary (`_source`) column cannot be used alongside other columns,\r\ne.g. log level, so column customisation isn't currently enabled. You'll\r\njust get timestamp and summary. This requires changes in the Unified\r\nData Table. **Won't be fixed in this PR**\r\n\r\n- We are left with this panel button that technically doesn't do\r\nanything outside of a dashboard. I don't *think* there's an easy way to\r\ndisable this. **Won't be fixed in this PR**\r\n\r\n\r\n\r\n## Followups\r\n\r\n- ~The Logs Overview details state machine can be cleaned up (it doesn't\r\nneed to fetch documents etc anymore).~ The state machine no longer\r\nfetches it's own documents. Some scaffolding is left in place as it'll\r\nbe needed for showing category details anyway.\r\n\r\n## Example\r\n\r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"b0122f547dc916ee5ccaad369968738d92596eaf","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-logs","Team:obs-ux-infra_services","backport:version","v8.18.0"],"number":199787,"url":"https://github.com/elastic/kibana/pull/199787","mergeCommit":{"message":"[Discover / Logs] Add new \"Saved Search component\" (#199787)\n\n## Summary\r\n\r\nImplements\r\nhttps://github.com/elastic/logs-dev/issues/111#issuecomment-2446470635.\r\n\r\nThis adds a new \"Saved Search component\". The component is a wrapper\r\naround the current Saved Search Embeddable, but uses\r\n`ReactEmbeddableRenderer` directly to render the embeddable outside of\r\nDashboard contexts. It monitors changes to things like `index`,\r\n`filters` etc and communicates these changes through the embeddable API.\r\n\r\nFor this PoC two locations were changed to use this component 1) Logs\r\nOverview flyout 2) APM Logs tab (when the Logs Overview isn't enabled\r\nvia advanced settings).\r\n\r\nThe component itself is technically beyond a PoC, and resides in it's\r\nown package. ~I'd like to get eyes from the Discover folks etc on the\r\napproach, and if we're happy I can fix the remaining known issues (apart\r\nfrom the mixing of columns point as I believe this exists on the roadmap\r\nanyway) and we can merge this for the initial two replacement points.~\r\n[Thanks Davis\r\n👌](https://github.com/elastic/logs-dev/issues/111#issuecomment-2475350199).\r\n\r\n`nonPersistedDisplayOptions` is added to facilitate some configurable\r\noptions via runtime state, but without the complexity of altering the\r\nactual saved search saved object.\r\n\r\nOn the whole I've tried to keep this as clean as possible whilst working\r\nwithin the embeddable framework, outside of a dashboard context.\r\n\r\n## Known issues\r\n\r\n- ~\"Flyout on flyout\" in the logs overview flyout (e.g. triggering the\r\ntable's flyout in this context).~ Fixed with `enableFlyout` option.\r\n- ~Filter buttons should be disabled via pills (e.g. in Summary\r\ncolumn).~ Fixed with `enableFilters` option.\r\n- Summary (`_source`) column cannot be used alongside other columns,\r\ne.g. log level, so column customisation isn't currently enabled. You'll\r\njust get timestamp and summary. This requires changes in the Unified\r\nData Table. **Won't be fixed in this PR**\r\n\r\n- We are left with this panel button that technically doesn't do\r\nanything outside of a dashboard. I don't *think* there's an easy way to\r\ndisable this. **Won't be fixed in this PR**\r\n\r\n\r\n\r\n## Followups\r\n\r\n- ~The Logs Overview details state machine can be cleaned up (it doesn't\r\nneed to fetch documents etc anymore).~ The state machine no longer\r\nfetches it's own documents. Some scaffolding is left in place as it'll\r\nbe needed for showing category details anyway.\r\n\r\n## Example\r\n\r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"b0122f547dc916ee5ccaad369968738d92596eaf"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199787","number":199787,"mergeCommit":{"message":"[Discover / Logs] Add new \"Saved Search component\" (#199787)\n\n## Summary\r\n\r\nImplements\r\nhttps://github.com/elastic/logs-dev/issues/111#issuecomment-2446470635.\r\n\r\nThis adds a new \"Saved Search component\". The component is a wrapper\r\naround the current Saved Search Embeddable, but uses\r\n`ReactEmbeddableRenderer` directly to render the embeddable outside of\r\nDashboard contexts. It monitors changes to things like `index`,\r\n`filters` etc and communicates these changes through the embeddable API.\r\n\r\nFor this PoC two locations were changed to use this component 1) Logs\r\nOverview flyout 2) APM Logs tab (when the Logs Overview isn't enabled\r\nvia advanced settings).\r\n\r\nThe component itself is technically beyond a PoC, and resides in it's\r\nown package. ~I'd like to get eyes from the Discover folks etc on the\r\napproach, and if we're happy I can fix the remaining known issues (apart\r\nfrom the mixing of columns point as I believe this exists on the roadmap\r\nanyway) and we can merge this for the initial two replacement points.~\r\n[Thanks Davis\r\n👌](https://github.com/elastic/logs-dev/issues/111#issuecomment-2475350199).\r\n\r\n`nonPersistedDisplayOptions` is added to facilitate some configurable\r\noptions via runtime state, but without the complexity of altering the\r\nactual saved search saved object.\r\n\r\nOn the whole I've tried to keep this as clean as possible whilst working\r\nwithin the embeddable framework, outside of a dashboard context.\r\n\r\n## Known issues\r\n\r\n- ~\"Flyout on flyout\" in the logs overview flyout (e.g. triggering the\r\ntable's flyout in this context).~ Fixed with `enableFlyout` option.\r\n- ~Filter buttons should be disabled via pills (e.g. in Summary\r\ncolumn).~ Fixed with `enableFilters` option.\r\n- Summary (`_source`) column cannot be used alongside other columns,\r\ne.g. log level, so column customisation isn't currently enabled. You'll\r\njust get timestamp and summary. This requires changes in the Unified\r\nData Table. **Won't be fixed in this PR**\r\n\r\n- We are left with this panel button that technically doesn't do\r\nanything outside of a dashboard. I don't *think* there's an easy way to\r\ndisable this. **Won't be fixed in this PR**\r\n\r\n\r\n\r\n## Followups\r\n\r\n- ~The Logs Overview details state machine can be cleaned up (it doesn't\r\nneed to fetch documents etc anymore).~ The state machine no longer\r\nfetches it's own documents. Some scaffolding is left in place as it'll\r\nbe needed for showing category details anyway.\r\n\r\n## Example\r\n\r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"b0122f547dc916ee5ccaad369968738d92596eaf"}},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: kibanamachine <[email protected]>
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this pull request
Dec 9, 2024
## Summary Implements elastic/logs-dev#111 (comment). This adds a new "Saved Search component". The component is a wrapper around the current Saved Search Embeddable, but uses `ReactEmbeddableRenderer` directly to render the embeddable outside of Dashboard contexts. It monitors changes to things like `index`, `filters` etc and communicates these changes through the embeddable API. For this PoC two locations were changed to use this component 1) Logs Overview flyout 2) APM Logs tab (when the Logs Overview isn't enabled via advanced settings). The component itself is technically beyond a PoC, and resides in it's own package. ~I'd like to get eyes from the Discover folks etc on the approach, and if we're happy I can fix the remaining known issues (apart from the mixing of columns point as I believe this exists on the roadmap anyway) and we can merge this for the initial two replacement points.~ [Thanks Davis 👌](elastic/logs-dev#111 (comment)). `nonPersistedDisplayOptions` is added to facilitate some configurable options via runtime state, but without the complexity of altering the actual saved search saved object. On the whole I've tried to keep this as clean as possible whilst working within the embeddable framework, outside of a dashboard context. ## Known issues - ~"Flyout on flyout" in the logs overview flyout (e.g. triggering the table's flyout in this context).~ Fixed with `enableFlyout` option. - ~Filter buttons should be disabled via pills (e.g. in Summary column).~ Fixed with `enableFilters` option. - Summary (`_source`) column cannot be used alongside other columns, e.g. log level, so column customisation isn't currently enabled. You'll just get timestamp and summary. This requires changes in the Unified Data Table. **Won't be fixed in this PR** - We are left with this panel button that technically doesn't do anything outside of a dashboard. I don't *think* there's an easy way to disable this. **Won't be fixed in this PR**  ## Followups - ~The Logs Overview details state machine can be cleaned up (it doesn't need to fetch documents etc anymore).~ The state machine no longer fetches it's own documents. Some scaffolding is left in place as it'll be needed for showing category details anyway. ## Example   --------- Co-authored-by: kibanamachine <[email protected]>
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this pull request
Dec 12, 2024
## Summary Implements elastic/logs-dev#111 (comment). This adds a new "Saved Search component". The component is a wrapper around the current Saved Search Embeddable, but uses `ReactEmbeddableRenderer` directly to render the embeddable outside of Dashboard contexts. It monitors changes to things like `index`, `filters` etc and communicates these changes through the embeddable API. For this PoC two locations were changed to use this component 1) Logs Overview flyout 2) APM Logs tab (when the Logs Overview isn't enabled via advanced settings). The component itself is technically beyond a PoC, and resides in it's own package. ~I'd like to get eyes from the Discover folks etc on the approach, and if we're happy I can fix the remaining known issues (apart from the mixing of columns point as I believe this exists on the roadmap anyway) and we can merge this for the initial two replacement points.~ [Thanks Davis 👌](elastic/logs-dev#111 (comment)). `nonPersistedDisplayOptions` is added to facilitate some configurable options via runtime state, but without the complexity of altering the actual saved search saved object. On the whole I've tried to keep this as clean as possible whilst working within the embeddable framework, outside of a dashboard context. ## Known issues - ~"Flyout on flyout" in the logs overview flyout (e.g. triggering the table's flyout in this context).~ Fixed with `enableFlyout` option. - ~Filter buttons should be disabled via pills (e.g. in Summary column).~ Fixed with `enableFilters` option. - Summary (`_source`) column cannot be used alongside other columns, e.g. log level, so column customisation isn't currently enabled. You'll just get timestamp and summary. This requires changes in the Unified Data Table. **Won't be fixed in this PR** - We are left with this panel button that technically doesn't do anything outside of a dashboard. I don't *think* there's an easy way to disable this. **Won't be fixed in this PR**  ## Followups - ~The Logs Overview details state machine can be cleaned up (it doesn't need to fetch documents etc anymore).~ The state machine no longer fetches it's own documents. Some scaffolding is left in place as it'll be needed for showing category details anyway. ## Example   --------- Co-authored-by: kibanamachine <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Implements https://github.com/elastic/logs-dev/issues/111#issuecomment-2446470635.
This adds a new "Saved Search component". The component is a wrapper around the current Saved Search Embeddable, but uses
ReactEmbeddableRendererdirectly to render the embeddable outside of Dashboard contexts. It monitors changes to things likeindex,filtersetc and communicates these changes through the embeddable API.For this PoC two locations were changed to use this component 1) Logs Overview flyout 2) APM Logs tab (when the Logs Overview isn't enabled via advanced settings).
The component itself is technically beyond a PoC, and resides in it's own package.
I'd like to get eyes from the Discover folks etc on the approach, and if we're happy I can fix the remaining known issues (apart from the mixing of columns point as I believe this exists on the roadmap anyway) and we can merge this for the initial two replacement points.Thanks Davis 👌.nonPersistedDisplayOptionsis added to facilitate some configurable options via runtime state, but without the complexity of altering the actual saved search saved object.On the whole I've tried to keep this as clean as possible whilst working within the embeddable framework, outside of a dashboard context.
Known issues
"Flyout on flyout" in the logs overview flyout (e.g. triggering the table's flyout in this context).Fixed withenableFlyoutoption.Filter buttons should be disabled via pills (e.g. in Summary column).Fixed withenableFiltersoption.Summary (
_source) column cannot be used alongside other columns, e.g. log level, so column customisation isn't currently enabled. You'll just get timestamp and summary. This requires changes in the Unified Data Table. Won't be fixed in this PRWe are left with this panel button that technically doesn't do anything outside of a dashboard. I don't think there's an easy way to disable this. Won't be fixed in this PR
Followups
The Logs Overview details state machine can be cleaned up (it doesn't need to fetch documents etc anymore).The state machine no longer fetches it's own documents. Some scaffolding is left in place as it'll be needed for showing category details anyway.Example