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.

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

[Dataset quality] Introduce management fly out #173554

Merged

Conversation

yngrdyn
Copy link
Contributor

@yngrdyn yngrdyn commented Dec 18, 2023

This PR was originally opened by @mohamedhamed-ahmed, in a mistake was closed by me 😅 when trying to bring the most recent changes from main and resolve the conflicts.

related to #170441

📝 Summary

This PR introduces the dataset quality flyout and shows the first 2 sections in the flyout, dataset and integration details. The new Actions column is also added as part of this PR.

✅ Testing

  1. Navigate to /app/observability-log-explorer/dataset-quality
  2. The Actions column should be visible at the rightmost side of the table
  3. Dataset Name column should now be clickable and opens the flyout
  4. Clicking the Open in Logs Explorer or the Open Action column should navigate to the Log Explorer with having the correct dataset and namespace selected

@yngrdyn yngrdyn requested a review from a team as a code owner December 18, 2023 17:27
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

@yngrdyn yngrdyn added the release_note:skip Skip the PR/issue when compiling release notes label Dec 19, 2023
@yngrdyn yngrdyn force-pushed the 170441-introduce-management-fly-out branch from 1804ea1 to 795bf0a Compare December 19, 2023 09:35
@yngrdyn yngrdyn linked an issue Dec 19, 2023 that may be closed by this pull request
@yngrdyn
Copy link
Contributor Author

yngrdyn commented Dec 19, 2023

/oblt-deploy

@yngrdyn
Copy link
Contributor Author

yngrdyn commented Jan 3, 2024

@awahab07 I was checking the custom deployment and I got this behaviour (Opening the fly out clicking on dataset title)
image

is that the latest version? if it's the latest version in the code we might need to update it to the little arrow present in other parts of the products (even logs explorer)
image

NOTICE.txt Outdated
@@ -1,5 +1,5 @@
Kibana source code with Kibana X-Pack source code
Copyright 2012-2023 Elasticsearch B.V.
Copyright 2012-2024 Elasticsearch B.V.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are not doing anything custom within this constructor we can omit it and use directly DataStreamDetails type

Copy link
Contributor

Choose a reason for hiding this comment

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

I see the constructor expand as we add more information to the model in future.

@awahab07
Copy link
Contributor

/oblt-deploy

@yngrdyn
Copy link
Contributor Author

yngrdyn commented Jan 16, 2024

I noticed the space per row is big, probably because of the new way of opening the flyout
image

This is how a row looked like before the opening mechanism
image

could we reduce the space around? For example in Log explorer the space is minimum
image

const formattedLastActivity = dataFormatter.convert(dataStreamStat.lastActivity);
const formattedCreatedOn = dataStreamDetails.createdOn
? dataFormatter.convert(dataStreamDetails.createdOn)
: '';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@isaclfreire if we don't have the Created on info should we just remove the row in the table?

Choose a reason for hiding this comment

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

Makes sense 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't implement hiding the row yet. Will there be valid cases when Created on won't be available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, an index without a backing index will not return a Created on date. This will most likely be true for data streams without data

- Adjusted table layout.
- Added skeleton loader.
- Refactored route, route params and associated types.
@awahab07
Copy link
Contributor

Updated table layout

Screen.Recording.2024-01-22.at.11.41.25.mov

Copy link
Contributor Author

@yngrdyn yngrdyn left a comment

Choose a reason for hiding this comment

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

LGTM! Good job mate 🚀

Copy link
Contributor

@mohamedhamed-ahmed mohamedhamed-ahmed left a comment

Choose a reason for hiding this comment

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

Thanks for putting the effort into this!! LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
datasetQuality 68 79 +11

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
datasetQuality 33.6KB 40.2KB +6.6KB

History

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

cc @awahab07 @mohamedhamed-ahmed

@awahab07
Copy link
Contributor

/oblt-deploy

@awahab07
Copy link
Contributor

/oblt-deploy-serverless

@awahab07 awahab07 merged commit 22ff125 into elastic:main Jan 22, 2024
16 checks passed
@kibanamachine kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 22, 2024
lcawl pushed a commit to lcawl/kibana that referenced this pull request Jan 26, 2024
related to  elastic#170441 

## 📝  Summary

This PR introduces the dataset quality flyout and shows the first 2
sections in the flyout, dataset and integration details. The new Actions
column is also added as part of this PR.

---------

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
Co-authored-by: Abdul Zahid <awahab07@yahoo.com>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
related to  elastic#170441 

## 📝  Summary

This PR introduces the dataset quality flyout and shows the first 2
sections in the flyout, dataset and integration details. The new Actions
column is also added as part of this PR.

---------

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
Co-authored-by: Abdul Zahid <awahab07@yahoo.com>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
related to  elastic#170441 

## 📝  Summary

This PR introduces the dataset quality flyout and shows the first 2
sections in the flyout, dataset and integration details. The new Actions
column is also added as part of this PR.

---------

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
Co-authored-by: Abdul Zahid <awahab07@yahoo.com>
@yngrdyn yngrdyn added the Team:obs-ux-logs Observability Logs User Experience Team label Mar 21, 2024
@elasticmachine
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-logs Observability Logs User Experience Team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dataset quality] Introduce management Fly-out
8 participants