-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[ECO][Inventory] Redirect ECS k8s entities to dashboards #197222
[ECO][Inventory] Redirect ECS k8s entities to dashboards #197222
Conversation
f18d706
to
6cb0fe8
Compare
0a09527
to
e5929db
Compare
import { unflattenObject } from '@kbn/observability-utils/object/unflatten_object'; | ||
import type { Entity, InventoryEntityLatest } from '../entities'; | ||
|
||
export function unflattenEntity(entity: Entity) { |
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.
Temp util Entity
is not replaced by InventoryEntityLatest
/ci |
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
…-ecs-k8s-entities
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.
will continue the review
x-pack/packages/observability/observability_utils/es/utils/esql_result_to_plain_objects.ts
Show resolved
Hide resolved
x-pack/plugins/observability_solution/inventory/public/components/alerts_badge/alerts_badge.tsx
Show resolved
Hide resolved
import { unflattenEntity } from '../../common/utils/unflatten_entity'; | ||
import { useKibana } from './use_kibana'; | ||
|
||
const KUBERNETES_DASHBOARDS_IDS: Record<string, string> = { |
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.
This approach won't work for other spaces. As the integration package installs the dashboards for the default one (if I'm not mistaken).
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.
Good point. It won't indeed. I think users will have to do this . @roshan-elastic , since we're delegating to the dashboards page the responsibility for checking the existence of a dashboard, should we check with them if they can create better messaging for troubleshooting?
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.
Hey @crespocarlos @kpatticha good thought.
We can go ahead with our part and I'll mention to @teresaalvarezsoler (Dashboards) and @daniela-elastic (Integrations) about whether there might be a more helpful empty state.
To confirm, the problem is:
- We'll send a user to a dashboard by ID (e.g. https://edge-oblt.kb.us-west2.gcp.elastic-cloud.com/app/dashboards#/view/adsad)
- The dashboard may or may not exist depending on which space a user is in (because integrations install in the default space - not all spaces)
- If a user clicks through from a link in our UI to a dashboard and it doesn't exist - they'll hit this page:
We'd like to propose the dashboard empty state should handle 'missing dashboards' potentially a bit more gracefully (e.g. a link to common reasons why it may not show) or maybe there's something better we could do (e.g. pass through a query-string explaining we're expecting the dashboard to exist and have a toast pop up to suggest a more helpful call to action to fix it).
Is that right @crespocarlos?
cc @teresaalvarezsoler @daniela-elastic - we want to send users through to dashboards for some entities within Inventory (we'd start with K8s entities - for which we have dashboards which are packaged in the K8s integration). The problem we believe is that the dashboards only exist for the space you installed the integration in:
FYI - All of this is 'tech preview' so we're not too worried right now
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.
We'd like to propose the dashboard empty state should handle 'missing dashboards' potentially a bit more gracefully (e.g. a link to common reasons why it may not show) or maybe there's something better we could do (e.g. pass through a query-string explaining we're expecting the dashboard to exist and have a toast pop up to suggest a more helpful call to action to fix it).
@roshan-elastic yeah, something like that. If a dashboard id is found on any space, the empty state message could point users to a possible solution to the problem.
cc: @kpatticha
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.
Cool - @teresaalvarezsoler @daniela-elastic - I can mention in our next 1-2-1...more of an FYI for now
@elasticmachine merge upstream |
return serviceOverviewLocator?.getRedirectUrl({ | ||
serviceName: identityValue, | ||
// service.environemnt is not part of entity.identityFields | ||
// we need to manually get its value |
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.
IMO, we don't have to pass the environment if it's not part of the identityFields.
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.
The Service Overview expects it though. It's used there to filter the data by environment. If not passed I'm not sure if there will be consequences.
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.
environment
is optional it will probably select "ALL" environments.
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.
yes. and if we don't pass it at all, it will always select ALL. Would that be correct?
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.
Yes. We do the same for the service inventory. If a service has more than one environments and the top right selector has All we don't pass the environment
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.
right. just for context, we were already passing the service.environment
before this PR. @kpatticha , is the suggestion here to change that and not pass it anymore?
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.
Yes. I suspect we were passing the service.environment
cause initially it was part of the identityFields
. When we removed it from the entity defintion we didn't update the ui.
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.
LGTM
@elasticmachine merge upstream |
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.
LGTM, some nits.
id: '1', | ||
display_name: 'entity_name', | ||
definition_id: 'entity_definition_id', | ||
} as EntityLatest['entity'], |
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.
Do you need this typing?
} as EntityLatest['entity'], | |
}, |
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.
it is if I omit some properties. This object is there to provide a basic mock for the tests
</EuiFlexItem> | ||
<EuiFlexItem className="eui-textTruncate"> | ||
<span className="eui-textTruncate" data-test-subj="entityNameDisplayName"> | ||
{entity[ENTITY_DISPLAY_NAME]} |
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.
Can you do?
{entity[ENTITY_DISPLAY_NAME]} | |
{entity.display_name} |
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.
Currently, no.
I'm finalizing this #198758 and will open a PR as soon as this one is merged.
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.
Ah you created the new type but haven't replaced the old one, right?
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.
Not yet. I created that type to facilitate the call to Entity Client's helper functions. I'll replace the old type on the other PR.
return serviceOverviewLocator?.getRedirectUrl({ | ||
serviceName: identityValue, | ||
// service.environemnt is not part of entity.identityFields | ||
// we need to manually get its value |
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.
environment
is optional it will probably select "ALL" environments.
KUBERNETES: { | ||
CLUSTER: createKubernetesEntity('cluster'), | ||
CONTAINER: createKubernetesEntity('container'), | ||
CRONJOB: createKubernetesEntity('cron_job'), | ||
DAEMONSET: createKubernetesEntity('daemon_set'), | ||
DEPLOYMENT: createKubernetesEntity('deployment'), | ||
JOB: createKubernetesEntity('job'), | ||
NAMESPACE: createKubernetesEntity('namespace'), | ||
NODE: createKubernetesEntity('node'), | ||
POD: createKubernetesEntity('pod'), | ||
STATEFULSET: createKubernetesEntity('stateful_set'), | ||
}, |
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.
IMHO we should not type these.
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.
This is mostly needed to properly map an entity type with its detail view.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
⏳ Build in-progress
History
|
Starting backport for target branches: 8.x |
) closes [elastic#196142](elastic#196142) ## Summary Links kubernetes ECS entities to their corresponding dashboards > [!IMPORTANT] > ECS `replicaset` doesn't have a dedicated dashboard. `container` will be handled in a separate ticket > Semconv won't link to any dashboard/page <img width="800" alt="image" src="https://github.com/user-attachments/assets/711dbd28-f0ef-4af0-a658-afe7f1595697"> ![redirect](https://github.com/user-attachments/assets/77d5d2e1-7ec4-40cd-b7d8-419e07e6b760) ### How to test - While elastic#196916 is not merged, change `ENTITIES_LATEST_ALIAS` constant to `'.entities.v1.latest*'` - Start a local kibana and es instances - Run ` node scripts/synthtrace k8s_entities.ts --live --clean ` - Run `PUT kbn:/internal/entities/managed/enablement` on the devtools - Install the kubernetes integration package to have the dashboards installed. - Navigate to `Inventory` and click through the k8s entities --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 8145cb7)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…) (#198815) # Backport This will backport the following commits from `main` to `8.x`: - [[ECO][Inventory] Redirect ECS k8s entities to dashboards (#197222)](#197222) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Carlos Crespo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-04T16:01:05Z","message":"[ECO][Inventory] Redirect ECS k8s entities to dashboards (#197222)\n\ncloses [#196142](https://github.com/elastic/kibana/issues/196142)\r\n\r\n## Summary\r\n\r\nLinks kubernetes ECS entities to their corresponding dashboards\r\n\r\n> [!IMPORTANT]\r\n> ECS `replicaset` doesn't have a dedicated dashboard. `container` will\r\nbe handled in a separate ticket\r\n> Semconv won't link to any dashboard/page\r\n\r\n<img width=\"800\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/711dbd28-f0ef-4af0-a658-afe7f1595697\">\r\n\r\n\r\n![redirect](https://github.com/user-attachments/assets/77d5d2e1-7ec4-40cd-b7d8-419e07e6b760)\r\n\r\n\r\n### How to test\r\n- While #196916 is not merged,\r\nchange `ENTITIES_LATEST_ALIAS` constant to `'.entities.v1.latest*'`\r\n- Start a local kibana and es instances \r\n- Run ` node scripts/synthtrace k8s_entities.ts --live --clean `\r\n- Run `PUT kbn:/internal/entities/managed/enablement` on the devtools\r\n- Install the kubernetes integration package to have the dashboards\r\ninstalled.\r\n- Navigate to `Inventory` and click through the k8s entities\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"8145cb7c6f483c3a8aa561b492fa098e3ce52027","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services"],"title":"[ECO][Inventory] Redirect ECS k8s entities to dashboards","number":197222,"url":"https://github.com/elastic/kibana/pull/197222","mergeCommit":{"message":"[ECO][Inventory] Redirect ECS k8s entities to dashboards (#197222)\n\ncloses [#196142](https://github.com/elastic/kibana/issues/196142)\r\n\r\n## Summary\r\n\r\nLinks kubernetes ECS entities to their corresponding dashboards\r\n\r\n> [!IMPORTANT]\r\n> ECS `replicaset` doesn't have a dedicated dashboard. `container` will\r\nbe handled in a separate ticket\r\n> Semconv won't link to any dashboard/page\r\n\r\n<img width=\"800\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/711dbd28-f0ef-4af0-a658-afe7f1595697\">\r\n\r\n\r\n![redirect](https://github.com/user-attachments/assets/77d5d2e1-7ec4-40cd-b7d8-419e07e6b760)\r\n\r\n\r\n### How to test\r\n- While #196916 is not merged,\r\nchange `ENTITIES_LATEST_ALIAS` constant to `'.entities.v1.latest*'`\r\n- Start a local kibana and es instances \r\n- Run ` node scripts/synthtrace k8s_entities.ts --live --clean `\r\n- Run `PUT kbn:/internal/entities/managed/enablement` on the devtools\r\n- Install the kubernetes integration package to have the dashboards\r\ninstalled.\r\n- Navigate to `Inventory` and click through the k8s entities\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"8145cb7c6f483c3a8aa561b492fa098e3ce52027"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197222","number":197222,"mergeCommit":{"message":"[ECO][Inventory] Redirect ECS k8s entities to dashboards (#197222)\n\ncloses [#196142](https://github.com/elastic/kibana/issues/196142)\r\n\r\n## Summary\r\n\r\nLinks kubernetes ECS entities to their corresponding dashboards\r\n\r\n> [!IMPORTANT]\r\n> ECS `replicaset` doesn't have a dedicated dashboard. `container` will\r\nbe handled in a separate ticket\r\n> Semconv won't link to any dashboard/page\r\n\r\n<img width=\"800\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/711dbd28-f0ef-4af0-a658-afe7f1595697\">\r\n\r\n\r\n![redirect](https://github.com/user-attachments/assets/77d5d2e1-7ec4-40cd-b7d8-419e07e6b760)\r\n\r\n\r\n### How to test\r\n- While #196916 is not merged,\r\nchange `ENTITIES_LATEST_ALIAS` constant to `'.entities.v1.latest*'`\r\n- Start a local kibana and es instances \r\n- Run ` node scripts/synthtrace k8s_entities.ts --live --clean `\r\n- Run `PUT kbn:/internal/entities/managed/enablement` on the devtools\r\n- Install the kubernetes integration package to have the dashboards\r\ninstalled.\r\n- Navigate to `Inventory` and click through the k8s entities\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"8145cb7c6f483c3a8aa561b492fa098e3ce52027"}}]}] BACKPORT--> Co-authored-by: Carlos Crespo <[email protected]>
closes #196142
Summary
Links kubernetes ECS entities to their corresponding dashboards
Important
ECS
replicaset
doesn't have a dedicated dashboard.container
will be handled in a separate ticketSemconv won't link to any dashboard/page
How to test
ENTITIES_LATEST_ALIAS
constant to'.entities.v1.latest*'
node scripts/synthtrace k8s_entities.ts --live --clean
PUT kbn:/internal/entities/managed/enablement
on the devtoolsInventory
and click through the k8s entities