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
[Profiling] adding KQL bar to embeddables #171016
[Profiling] adding KQL bar to embeddables #171016
Conversation
Pinging @elastic/apm-ui (Team:APM) |
🤖 GitHub commentsExpand to view the GitHub comments
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.
Looks good ✨ Only a couple of minor comments.
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.
LGMT!
In the description, could we add the steps to test the PR? I believe it would help reviewers get up to speed with working on Profiling in general.
x-pack/plugins/apm/public/components/app/profiling_overview/index.tsx
Outdated
Show resolved
Hide resolved
…condes/kibana into profiling-embeddables-kql-filter
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.
x-pack/plugins/apm/public/components/app/profiling_overview/index.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/profiling/public/embeddables/profiling_embeddable_search_bar.tsx
Outdated
Show resolved
Hide resolved
Do you see the Profiling kuery bar being used independently in any plugin? I think the kuery bar will always be used together with another Profiling component to filter its result down. That is the reason I did not create a new embeddable for the kuery bar itself. |
Not really, I expect that the profiling search bar will always be used with another Profiling component. However, here is an example of Alerts exporting the search bar https://github.com/elastic/kibana/blob/2a8a68c1f8d14074d23ca9feef118aa172191bfe/x-pack/plugins/apm/public/components/app/alerts_overview/index.tsx#L85C14-L101 (it doesn't have to be the same for profiling, I was just curious) |
The advantage I see is the client will be able to position the kuery bar wherever they want. We could place it on top of the tabs for example. |
Ok, after talking to @iogbole we agreed that would be better to display the Profiling kuery bar on top of the Tabs, also to keep it consistent with the other pages. Thanks for raising it up @kpatticha. |
@elasticmachine run elasticsearch-ci/docs |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Adding KQL bar to the
Flamegraph
andTop 10 functions
embeddable.kql.mov
New version: