-
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
[ObsUx] [Infra] Change container details view with asset details view #180436
[ObsUx] [Infra] Change container details view with asset details view #180436
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
026620f
to
6f1973f
Compare
365ced1
to
07eadf0
Compare
/oblt-deploy-serverless |
62b894b
to
7753742
Compare
b5f7604
to
a69a97b
Compare
ba3d65f
to
8c12e90
Compare
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! Thanks for this PR. Left a couple of nits, but it's ok to merge.
...y_solution/infra/public/components/asset_details/tabs/overview/metrics/container_metrics.tsx
Outdated
Show resolved
Hide resolved
dataViewId: dataView?.id, | ||
options: { overview }, | ||
}); | ||
export const KubernetesNodeCharts = React.forwardRef<HTMLDivElement, MetricsChartsFields>( |
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.
Thanks for moving this component into this file.
await infraSynthtraceKibanaClient.installSystemPackage(version); | ||
synthEsClient = await getInfraSynthtraceEsClient(esClient); | ||
await synthEsClient.index( | ||
generateDockerContainersData({ |
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.
💯 !
|
||
await pageObjects.assetDetails.clickOverviewTab(); | ||
}); | ||
// Add this test after implementing the creation of metadata for containers in sythtrace |
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 we need to merge these commented-out lines?
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.
No, we don't need to merge them, was just a reminder to add those test when we can see the metrics data
a89627f
to
8fc3332
Compare
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
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.
Changes in the UI settings collector LGTM.
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.
UI settings constant LGTM 👍
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.
changes to synthtrace code co-owned by obs-ux-logs
LGTM 👍
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Canvas Sharable Runtime
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled in files
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Part of #179844
In this PR
How to test
Container view