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] Added Dataset Quality Locator #177000

Conversation

mohamedhamed-ahmed
Copy link
Contributor

@mohamedhamed-ahmed mohamedhamed-ahmed commented Feb 15, 2024

closes #170611

📝 Summary

This PR adds the infrastructure work for the locators needed to create the navigation link from the Logs Explorer to the Dataset Quality Page, but the links themselves are to be added with a later ticket.

💡For Reviewers

To be abled to test this PR you can add the below code here to make the link visible in the Logs Explorer Page.

<ConnectedDatasetQualityLink /> <VerticalRule />

🎥 Demo

datasetqualitynavigation.mov

@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!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the types here will be removed as part of another ticket to make use of the types from the data plugin
https://github.com/elastic/kibana/blob/9e04d2c5c7b2139b7e23743b133ea79bf93f5ba3/src/plugins/data/common/query/timefilter/types.ts

const qualityCounts = mapPercentagesToQualityCounts(datasetsQuality.percentages);
const datasetsWithoutIgnoredField =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not part of the PR scope but a bug that is found and decided to fix it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this file will later on be moved to the dataset quality plugin as we are planning to create our own app

): DatasetQualityLocatorParams => {
const { time, refreshInterval } = logsExplorerState;
const locatorParams: DatasetQualityLocatorParams = {
filters: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this construction will be refactored when we use the types from the data plugin

@mohamedhamed-ahmed mohamedhamed-ahmed marked this pull request as ready for review February 20, 2024 11:49
@mohamedhamed-ahmed mohamedhamed-ahmed requested a review from a team as a code owner February 20, 2024 11:49
@mohamedhamed-ahmed mohamedhamed-ahmed added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.14.0 labels Feb 20, 2024
@@ -95,6 +96,10 @@ export class ObservabilityLogsExplorerPlugin
useHash,
})
);
const datasetQualityLocator = share.url.locators.create(
new DatasetQualityLocatorDefinition({})
Copy link
Contributor

@yngrdyn yngrdyn Feb 21, 2024

Choose a reason for hiding this comment

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

Why are we not using useHash here? What's the difference?

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 correct will add it here we should read it from state:storeInSessionStorage

@mohamedhamed-ahmed mohamedhamed-ahmed changed the title [Dataset Quality] Added link from Logs Explorer to Dataset Quality [Dataset Quality] Added Dataset Quality Locators Feb 21, 2024
@mohamedhamed-ahmed mohamedhamed-ahmed changed the title [Dataset Quality] Added Dataset Quality Locators [Dataset Quality] Added Dataset Quality Locator Feb 21, 2024
const pageState = datasetQualityUrlSchemaV1.urlSchemaRT.encode(
deepCompactObject({
v: 1,
filters,
Copy link
Contributor

@yngrdyn yngrdyn Feb 21, 2024

Choose a reason for hiding this comment

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

Here we will be not only taking into account timeRange or the other filters as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

at the moment I am only passing the timerange, its the only filter added to the locator params.
We can later add more filters to the interface

Copy link
Contributor

@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 🚀

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1620 1621 +1
datasetQuality 167 168 +1
discover 724 725 +1
logsExplorer 676 677 +1
logsShared 223 224 +1
observability 599 600 +1
observabilityLogsExplorer 203 206 +3
observabilityOnboarding 196 197 +1
total +10

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/deeplinks-observability 26 29 +3
observabilityLogsExplorer 20 21 +1
total +4

Async chunks

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

id before after diff
datasetQuality 136.3KB 136.4KB +104.0B
observabilityLogsExplorer 149.5KB 149.5KB +3.0B
total +107.0B

Page load bundle

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

id before after diff
observabilityLogsExplorer 13.5KB 14.3KB +814.0B
Unknown metric groups

API count

id before after diff
@kbn/deeplinks-observability 37 40 +3
observabilityLogsExplorer 20 21 +1
total +4

ESLint disabled line counts

id before after diff
@kbn/deeplinks-observability 5 8 +3

Total ESLint disabled count

id before after diff
@kbn/deeplinks-observability 5 8 +3

History

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

@yngrdyn yngrdyn merged commit 9a47387 into elastic:main Feb 22, 2024
16 checks passed
yngrdyn added a commit that referenced this pull request Feb 23, 2024
This is a [follow
up](#177000 (comment))
PR of #177000.

## Changes
- Replace custom types for `timeRangeConfig` in dataset quality with
common types coming from data plugin.
semd pushed a commit to semd/kibana that referenced this pull request Mar 4, 2024
This is a [follow
up](elastic#177000 (comment))
PR of elastic#177000.

## Changes
- Replace custom types for `timeRangeConfig` in dataset quality with
common types coming from data plugin.
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
closes elastic#170611

## 📝  Summary

This PR adds the infrastructure work for the locators needed to create
the navigation link from the Logs Explorer to the Dataset Quality Page,
but the links themselves are to be added with a later ticket.

## 💡For Reviewers

To be abled to test this PR you can add the below code
[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/observability_solution/observability_logs_explorer/public/components/logs_explorer_top_nav_menu.tsx#L150)
to make the link visible in the Logs Explorer Page.

`<ConnectedDatasetQualityLink />
  <VerticalRule />`

## 🎥 Demo



https://github.com/elastic/kibana/assets/11225826/1f3ce10a-3b8c-4027-b72d-1ed71b782fa5
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
This is a [follow
up](elastic#177000 (comment))
PR of elastic#177000.

## Changes
- Replace custom types for `timeRangeConfig` in dataset quality with
common types coming from data plugin.
@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.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dataset quality] Link to dataset health page from Observability Logs Explorer
5 participants