[APM] Improve user journey from APM UI to Discover while preserving context #232361
[APM] Improve user journey from APM UI to Discover while preserving context #232361rmyz merged 52 commits intoelastic:mainfrom
Conversation
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
...bility/plugins/apm/public/components/shared/links/discover_links/open_in_discover_button.tsx
Outdated
Show resolved
Hide resolved
...bility/plugins/apm/public/components/shared/links/discover_links/open_in_discover_button.tsx
Outdated
Show resolved
Hide resolved
# Conflicts: # x-pack/solutions/observability/plugins/apm/tsconfig.json
iblancof
left a comment
There was a problem hiding this comment.
Checked the code changes after the suggested updates, and with "Open in Discover" now everywhere, LGTM
...bility/plugins/apm/public/components/shared/links/discover_links/open_in_discover_button.tsx
Show resolved
Hide resolved
.../plugins/apm/public/components/shared/links/discover_links/open_error_in_discover_button.tsx
Outdated
Show resolved
Hide resolved
.../plugins/apm/public/components/shared/links/discover_links/open_error_in_discover_button.tsx
Outdated
Show resolved
Hide resolved
| serviceName, | ||
| }; | ||
|
|
||
| const esqlQuery = getESQLQuery({ |
There was a problem hiding this comment.
Lines 86–115 are duplicated across all three components. Can we refactor it to avoid repetition?
There was a problem hiding this comment.
There was an exact comment asking the opposite, I will keep it in their own components since this PR has been open for too long
There was a problem hiding this comment.
I'm not suggesting to do the opposite, It makes sense to keep separate files to handle each case, but in the end we’re always rendering a button that opens Discover with some params. That button logic could be shared.
With the current approach, if we ever need to change how the button works, we’ll have to update it in 3 different components, here as an example
This also makes it easy to introduce inconsistencies, one component is already using a link instead of a button (not sure why).
There was a problem hiding this comment.
created base discover button to share the button logic in 84020d1 (#232361)
...ity/plugins/apm/public/components/shared/links/discover_links/open_span_in_discover_link.tsx
Outdated
Show resolved
Hide resolved
.../plugins/apm/public/components/shared/links/discover_links/open_error_in_discover_button.tsx
Outdated
Show resolved
Hide resolved
💔 Build Failed
Failed CI StepsHistory
cc @rmyz |
| href={discoverHref} | ||
| isDisabled={!esqlQuery || indexSettingsStatus !== FETCH_STATUS.SUCCESS} | ||
| > | ||
| {label} |
There was a problem hiding this comment.
nit: since the label it the same for all you can consider making it the default
kpatticha
left a comment
There was a problem hiding this comment.
Nice work, thanks for addressing the comments.
…ontext (elastic#232361) Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
…ontext (elastic#232361) Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
…ontext (elastic#232361) Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Summary
Closes #232178
This PR improves the navigation from APM UI to Discover regarding the traces flow.
When navigating to Discover, the user will be able to see an ES|QL query preserving the context they had in APM UI.
Example of the new logic working:
Kapture.2025-08-27.at.15.19.13.mp4
Modified pages:
Changes made:
How to test
Important
Please, test both APM and OTel scenarios, so we ensure everything works fine.
Thank you!
APM
OTel