[APM] Disable popover for grouped edges in Service Map #186597
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
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:
|
mykolaharmash
added
release_note:skip
|
@elasticmachine merge upstream |
…king-service-maps-arrow
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
jennypavlova
approved these changes
Jun 24, 2024
There was a problem hiding this comment.
LGTM 💯
I don't see an error now. I saw some issues with showing "undefined" in the tooltip but it's not related to the PR. I double-checked on main and I can reproduce it there as well so it's out of the scope of the PR but decided to share it so we can investigate in the future - I used oblt-lite data to reproduce this:
|
Thank you for checking! I've created a separate issue in our backlog for the |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jun 24, 2024
Closes elastic/sdh-apm#1378 Closes elastic#186101 ## Summary Looking at the code, it seems like use-case for showing popover on an edge that leads into a grouped resources node was never supported, and it's unclear what kind of content should be displayed in the popover in that case. So this change just prevents the crashing reported in SDH by disabling popover on grouped edged. In case we want to support this use-case, there should a product decision about the content to show in the popover. https://github.com/elastic/kibana/assets/793851/9a353843-fd70-4543-a0b3-2fb8293d497f Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 17b010c)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Jun 24, 2024
… (#186813) # Backport This will backport the following commits from `main` to `8.14`: - [[APM] Disable popover for grouped edges in Service Map (#186597)](#186597) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Mykola Harmash","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-06-24T12:20:58Z","message":"[APM] Disable popover for grouped edges in Service Map (#186597)\n\nCloses https://github.com/elastic/sdh-apm/issues/1378\r\nCloses https://github.com/elastic/kibana/issues/186101\r\n\r\n## Summary\r\n\r\nLooking at the code, it seems like use-case for showing popover on an\r\nedge that leads into a grouped resources node was never supported, and\r\nit's unclear what kind of content should be displayed in the popover in\r\nthat case.\r\n\r\nSo this change just prevents the crashing reported in SDH by disabling\r\npopover on grouped edged. In case we want to support this use-case,\r\nthere should a product decision about the content to show in the\r\npopover.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/793851/9a353843-fd70-4543-a0b3-2fb8293d497f\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"17b010c603d3e364a75cba1472a7179dea647aa3","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services","v8.15.0"],"title":"[APM] Disable popover for grouped edges in Service Map","number":186597,"url":"https://github.com/elastic/kibana/pull/186597","mergeCommit":{"message":"[APM] Disable popover for grouped edges in Service Map (#186597)\n\nCloses https://github.com/elastic/sdh-apm/issues/1378\r\nCloses https://github.com/elastic/kibana/issues/186101\r\n\r\n## Summary\r\n\r\nLooking at the code, it seems like use-case for showing popover on an\r\nedge that leads into a grouped resources node was never supported, and\r\nit's unclear what kind of content should be displayed in the popover in\r\nthat case.\r\n\r\nSo this change just prevents the crashing reported in SDH by disabling\r\npopover on grouped edged. In case we want to support this use-case,\r\nthere should a product decision about the content to show in the\r\npopover.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/793851/9a353843-fd70-4543-a0b3-2fb8293d497f\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"17b010c603d3e364a75cba1472a7179dea647aa3"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/186597","number":186597,"mergeCommit":{"message":"[APM] Disable popover for grouped edges in Service Map (#186597)\n\nCloses https://github.com/elastic/sdh-apm/issues/1378\r\nCloses https://github.com/elastic/kibana/issues/186101\r\n\r\n## Summary\r\n\r\nLooking at the code, it seems like use-case for showing popover on an\r\nedge that leads into a grouped resources node was never supported, and\r\nit's unclear what kind of content should be displayed in the popover in\r\nthat case.\r\n\r\nSo this change just prevents the crashing reported in SDH by disabling\r\npopover on grouped edged. In case we want to support this use-case,\r\nthere should a product decision about the content to show in the\r\npopover.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/793851/9a353843-fd70-4543-a0b3-2fb8293d497f\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"17b010c603d3e364a75cba1472a7179dea647aa3"}}]}] BACKPORT--> Co-authored-by: Mykola Harmash <[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.
Closes https://github.com/elastic/sdh-apm/issues/1378
Closes #186101
Summary
Looking at the code, it seems like use-case for showing popover on an edge that leads into a grouped resources node was never supported, and it's unclear what kind of content should be displayed in the popover in that case.
So this change just prevents the crashing reported in SDH by disabling popover on grouped edged. In case we want to support this use-case, there should a product decision about the content to show in the popover.
CleanShot.2024-06-21.at.11.35.00.mp4