Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[One Discover] Custom Service Name Cell #192381

Merged
merged 16 commits into from
Sep 15, 2024
Merged

[One Discover] Custom Service Name Cell #192381

merged 16 commits into from
Sep 15, 2024

Conversation

mohamedhamed-ahmed
Copy link
Contributor

@mohamedhamed-ahmed mohamedhamed-ahmed commented Sep 9, 2024 •
edited by kibanamachine
Loading

closes #190456

📝 Summary

This PR adds the agent icon as a prefix to the service name if an agent name is available.

🎥 Demo

ServiceNameBadge.mov

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@mohamedhamed-ahmed
Copy link
Contributor Author

/ci

@mohamedhamed-ahmed
Copy link
Contributor Author

/ci

@mohamedhamed-ahmed mohamedhamed-ahmed added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:obs-ux-logs Observability Logs User Experience Team release_note:enhancement Project:OneDiscover Enrich Discover with contextual awareness / Merge with Logs Explorer and removed release_note:skip Skip the PR/issue when compiling release notes labels Sep 10, 2024
@mohamedhamed-ahmed mohamedhamed-ahmed marked this pull request as ready for review September 10, 2024 18:29
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Sep 10, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

tonyghiani
tonyghiani approved these changes Sep 11, 2024
View reviewed changes
Copy link
Contributor

@tonyghiani tonyghiani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

crespocarlos
crespocarlos approved these changes Sep 11, 2024
View reviewed changes
Copy link
Contributor

@crespocarlos crespocarlos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obs UX Infra Services changes LGMT.

davismcphee
davismcphee approved these changes Sep 13, 2024
View reviewed changes
Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good and it works well! Left a couple of small suggestions around styling that I think should be addressed, but approving now since it otherwise LGTM 👍

src/plugins/discover/public/components/data_types/logs/service_name_badge_cell.tsx Outdated Show resolved Hide resolved
src/plugins/discover/public/components/data_types/logs/service_name_badge_cell.tsx Outdated Show resolved Hide resolved
src/plugins/discover/public/components/data_types/logs/service_name_badge_cell.tsx Outdated Show resolved Hide resolved
src/plugins/discover/public/components/data_types/logs/service_name_badge_cell.tsx Outdated Show resolved Hide resolved
src/plugins/discover/public/components/data_types/logs/service_name_badge_cell.tsx Outdated Show resolved Hide resolved
@mohamedhamed-ahmed
Copy link
Contributor Author

@davismcphee Thanks a million for the great comments, I addressed them all.
I see you already approved the PR, but would appreciate if you may have 1 more look. I also added the tooltip as suggested but would love to know your feedback about it.

davismcphee
davismcphee approved these changes Sep 13, 2024
View reviewed changes
Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest changes look and work great, thanks for addressing my feedback!

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 15, 2024 •
edited
Loading

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: e653a4c
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-192381-e653a4ce867c

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #96 / Cloud Security Posture Test adding Cloud Security Posture Integrations KSPM K8S KSPM K8S KSPM K8S Workflow

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 993 994 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
discover 811.7KB 812.7KB +1.0KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mohamedhamed-ahmed mohamedhamed-ahmed merged commit 7b3fa3a into elastic:main Sep 15, 2024
23 checks passed
markov00 pushed a commit to markov00/kibana that referenced this pull request Sep 18, 2024
@mohamedhamed-ahmed @kibanamachine @markov00
[One Discover] Custom Service Name Cell (elastic#192381)
c2693af 
closes elastic#190456

## 📝  Summary

This PR adds the agent icon as a prefix to the service name if an agent
name is available.

## 🎥 Demo


https://github.com/user-attachments/assets/4fad743a-6806-4440-91eb-fdfa35785a19

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@jbudz
Copy link
Member

jbudz commented Sep 20, 2024

@mohamedhamed-ahmed can you check if this needs to be backported to 8.x?

@mohamedhamed-ahmed
Copy link
Contributor Author

@mohamedhamed-ahmed can you check if this needs to be backported to 8.x?

@davismcphee wdyt? we can backport this I guess to 8.16 should we go forward with this?

@davismcphee
Copy link
Contributor

@mohamedhamed-ahmed Yes we definitely should! Missed this in my review, thanks @jbudz 👍

@mohamedhamed-ahmed mohamedhamed-ahmed removed the backport:skip This commit does not require backporting label Sep 20, 2024
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 20, 2024
@mohamedhamed-ahmed mohamedhamed-ahmed added v8.16.0 backport:version Backport to applied version labels and removed backport:skip This commit does not require backporting labels Sep 20, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 20, 2024
@mohamedhamed-ahmed
[One Discover] Custom Service Name Cell (elastic#192381)
3a9012a 
closes elastic#190456

## 📝  Summary

This PR adds the agent icon as a prefix to the service name if an agent
name is available.

## 🎥 Demo

https://github.com/user-attachments/assets/4fad743a-6806-4440-91eb-fdfa35785a19

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 7b3fa3a)
@kibanamachine kibanamachine mentioned this pull request Sep 20, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

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 Sep 20, 2024
@kibanamachine @mohamedhamed-ahmed
[8.x] [One Discover] Custom Service Name Cell (#192381) (#193632)
bc6757a 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[One Discover] Custom Service Name Cell
(#192381)](#192381)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"mohamedhamed-ahmed","email":"mohamed.ahmed@elastic.co"},"sourceCommit":{"committedDate":"2024-09-15T17:28:04Z","message":"[One
Discover] Custom Service Name Cell (#192381)\n\ncloses
#190456 📝
Summary\r\n\r\nThis PR adds the agent icon as a prefix to the service
name if an agent\r\nname is available.\r\n\r\n## 🎥
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/4fad743a-6806-4440-91eb-fdfa35785a19\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"7b3fa3ab100ed438bbb8f087a8d651d23378e174","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","ci:project-deploy-observability","Team:obs-ux-logs","Team:obs-ux-infra_services","Project:OneDiscover","v8.16.0","backport:version"],"title":"[One
Discover] Custom Service Name
Cell","number":192381,"url":"#192381
Discover] Custom Service Name Cell (#192381)\n\ncloses
#190456 📝
Summary\r\n\r\nThis PR adds the agent icon as a prefix to the service
name if an agent\r\nname is available.\r\n\r\n## 🎥
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/4fad743a-6806-4440-91eb-fdfa35785a19\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"7b3fa3ab100ed438bbb8f087a8d651d23378e174"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"#192381
Discover] Custom Service Name Cell (#192381)\n\ncloses
#190456 📝
Summary\r\n\r\nThis PR adds the agent icon as a prefix to the service
name if an agent\r\nname is available.\r\n\r\n## 🎥
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/4fad743a-6806-4440-91eb-fdfa35785a19\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"7b3fa3ab100ed438bbb8f087a8d651d23378e174"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crespocarlos crespocarlos crespocarlos approved these changes

@davismcphee davismcphee davismcphee approved these changes

@tonyghiani tonyghiani tonyghiani approved these changes

Assignees
No one assigned
Labels
backport:version Backport to applied version labels ci:project-deploy-observability Create an Observability project Project:OneDiscover Enrich Discover with contextual awareness / Merge with Logs Explorer release_note:enhancement Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team Team:obs-ux-logs Observability Logs User Experience Team v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[One Discover] Add custom cell renderer for service name
9 participants
@mohamedhamed-ahmed @obltmachine @elasticmachine @kibana-ci @jbudz @davismcphee @kibanamachine @crespocarlos @tonyghiani