Skip to content

Commit

Permalink
[Dataset Quality] Refactor integrations in DQ Flyout to not depend on…
Browse files Browse the repository at this point in the history
… main DQ Page page (#187450)

## Summary

This PR is a prerequisite to the Locator Implementation for Logs
Explorer - #186287

## Problem Statement

- Integrations were fetched when the main DQ page loads and stored in
the State Machine. This means when the Flyout Opens, it was referencing
already fetched data from the main page, updating the URL and then that
was used to render certain sections on the Flyout. This causes issues as
when a Locator is used to directly open the Flyout from some other page.
In that case everything happen asynchronously causing the data to be not
present when the flyout open thus those integration sections were not
present.

## Solution

- Now when the flyout is opened or is already open, it reads the basic
params from the URL like `DataStream`. With this information, it make
API call to fetch Integration information and thus making it
independent.
- Does this means you duplicated the Logic to fetch Integrations ? Yes
and No. Logic has to be duplicated as Flyout is moving to its own page
very soon. This means it would anyhow not be able to re-use that
Integration Information available. Secondly the duplication is not one
to one, its more catered towards Flyout logic
- Split the state machine to make Integration Calls only when the opened
Dataset is actually an integration. This is done by chaining the
respective states after the `DataStreamSettings` state confirms presence
of Integration.

## What else has been done

- Type cleaning: A lot of types has to be refactored to make this
change. Also simplified some duplicate types. We were using
  - Runtime types
  - Types Derived from Runtime Types
  - Inferred Types from API Responses
  We don't need the 3rd one. 1 and 2 and sufficient.
  • Loading branch information
achyutjhunjhunwala authored Jul 4, 2024
1 parent 358dece commit c483701
Show file tree
Hide file tree
Showing 30 changed files with 466 additions and 160 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Once the tests finish, the instances will be terminated.
node x-pack/plugins/observability_solution/dataset_quality/scripts/api --server

# run tests
node x-pack/plugins/observability_solution/dataset_quality/scripts/api --runner --grep-files=error_group_list
node x-pack/plugins/observability_solution/dataset_quality/scripts/api --runner --grep-files=data_stream_settings.spec.ts
```


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ const userPrivilegesRt = rt.type({
canMonitor: rt.boolean,
});

export type DataStreamUserPrivileges = rt.TypeOf<typeof userPrivilegesRt>;

const datasetUserPrivilegesRt = rt.intersection([
userPrivilegesRt,
rt.type({
Expand Down Expand Up @@ -44,14 +42,13 @@ export const dashboardRT = rt.type({
title: rt.string,
});

export type Dashboard = rt.TypeOf<typeof dashboardRT>;

export const integrationDashboardsRT = rt.type({
dashboards: rt.array(dashboardRT),
});

export type IntegrationDashboards = rt.TypeOf<typeof integrationDashboardsRT>;
export type Dashboard = rt.TypeOf<typeof dashboardRT>;

export const getIntegrationDashboardsResponseRt = rt.exact(integrationDashboardsRT);
export type IntegrationDashboardsResponse = rt.TypeOf<typeof integrationDashboardsRT>;

export const integrationIconRt = rt.intersection([
rt.type({
Expand All @@ -74,18 +71,19 @@ export const integrationRt = rt.intersection([
version: rt.string,
icons: rt.array(integrationIconRt),
datasets: rt.record(rt.string, rt.string),
dashboards: rt.array(dashboardRT),
}),
]);

export type Integration = rt.TypeOf<typeof integrationRt>;
export type IntegrationType = rt.TypeOf<typeof integrationRt>;

export const getIntegrationsResponseRt = rt.exact(
rt.type({
integrations: rt.array(integrationRt),
})
);

export type IntegrationResponse = rt.TypeOf<typeof getIntegrationsResponseRt>;

export const degradedDocsRt = rt.type({
dataset: rt.string,
count: rt.number,
Expand Down Expand Up @@ -117,6 +115,7 @@ export type DegradedFieldResponse = rt.TypeOf<typeof getDataStreamDegradedFields

export const dataStreamSettingsRt = rt.partial({
createdOn: rt.union([rt.null, rt.number]), // rt.null is needed because `createdOn` is not available on Serverless
integration: rt.string,
});

export type DataStreamSettings = rt.TypeOf<typeof dataStreamSettingsRt>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { DataStreamType } from '../types';

export interface GetDataStreamIntegrationParams {
type: DataStreamType;
integrationName: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,20 @@
* 2.0.
*/

import { DashboardType, IntegrationType } from './types';
import { IntegrationType } from '../api_types';

export class Integration {
name: IntegrationType['name'];
title: string;
version: string;
datasets: Record<string, string>;
icons?: IntegrationType['icons'];
dashboards?: DashboardType[];

private constructor(integration: Integration) {
this.name = integration.name;
this.title = integration.title || integration.name;
this.version = integration.version || '1.0.0';
this.icons = integration.icons;
this.dashboards = integration.dashboards || [];
this.datasets = integration.datasets || {};
}

Expand All @@ -29,7 +27,6 @@ export class Integration {
...integration,
title: integration.title || integration.name,
version: integration.version || '1.0.0',
dashboards: integration.dashboards || [],
datasets: integration.datasets || {},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ export type DataStreamStatServiceResponse = GetDataStreamsStatsResponse;

export type GetIntegrationsParams =
APIClientRequestParamsOf<`GET /internal/dataset_quality/integrations`>['params'];
export type GetIntegrationsResponse = APIReturnType<`GET /internal/dataset_quality/integrations`>;
export type IntegrationType = GetIntegrationsResponse['integrations'][0];
export type IntegrationsResponse = IntegrationType[];

export type GetDataStreamsDegradedDocsStatsParams =
APIClientRequestParamsOf<`GET /internal/dataset_quality/data_streams/degraded_docs`>['params'];
Expand Down Expand Up @@ -69,9 +66,6 @@ export type GetNonAggregatableDataStreamsResponse =

export type GetIntegrationDashboardsParams =
APIClientRequestParamsOf<`GET /internal/dataset_quality/integrations/{integration}/dashboards`>['params']['path'];
export type GetIntegrationDashboardsResponse =
APIReturnType<`GET /internal/dataset_quality/integrations/{integration}/dashboards`>;
export type DashboardType = GetIntegrationDashboardsResponse['dashboards'][0];

export type { DataStreamStat } from './data_stream_stat';
export type {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* 2.0.
*/

import { DataStreamStatType } from '../data_streams_stats';
import { Integration } from '../data_streams_stats/integration';

export type SortDirection = 'asc' | 'desc';

export type Maybe<T> = T | null | undefined;
Expand All @@ -13,3 +16,12 @@ export interface Coordinate {
x: number;
y: Maybe<number>;
}

export interface BasicDataStream {
type: string;
name: DataStreamStatType['name'];
rawName: string;
namespace: string;
title: string;
integration?: Integration;
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { Header } from './header';
import { IntegrationSummary } from './integration_summary';
import { FlyoutProps } from './types';
import { FlyoutSummary } from './flyout_summary/flyout_summary';
import { BasicDataStream } from '../../../common/types';

// Allow for lazy loading
// eslint-disable-next-line import/no-default-export
Expand All @@ -39,8 +40,18 @@ export default function Flyout({ dataset, closeFlyout }: FlyoutProps) {
timeRange,
loadingState,
flyoutLoading,
integration,
} = useDatasetQualityFlyout();

const titleAndLinkDetails: BasicDataStream = {
name: dataset.name,
rawName: dataset.rawName,
integration: integration?.integrationDetails,
type: dataset.type,
namespace: dataset.namespace,
title: integration?.integrationDetails?.datasets?.[dataset.name] ?? dataset.name,
};

const { startTracking } = useDatasetDetailsTelemetry();

useEffect(() => {
Expand All @@ -58,7 +69,10 @@ export default function Flyout({ dataset, closeFlyout }: FlyoutProps) {
<EuiSkeletonRectangle width="100%" height={80} />
) : (
<>
<Header dataStreamStat={dataset} />
<Header
titleAndLinkDetails={titleAndLinkDetails}
loading={!loadingState.datasetIntegrationDone}
/>
<EuiFlyoutBody css={flyoutBodyStyles} data-test-subj="datasetQualityFlyoutBody">
<EuiPanel hasBorder={false} hasShadow={false} paddingSize="l">
<FlyoutSummary
Expand Down Expand Up @@ -86,12 +100,13 @@ export default function Flyout({ dataset, closeFlyout }: FlyoutProps) {
fieldFormats={fieldFormats}
/>

{dataStreamStat.integration && (
{integration?.integrationDetails && (
<>
<EuiSpacer />
<IntegrationSummary
integration={dataStreamStat.integration}
dashboardsLoading={loadingState.datasetIntegrationsLoading}
integration={integration.integrationDetails}
dashboards={integration?.dashboards ?? []}
dashboardsLoading={loadingState.datasetIntegrationDashboardLoading}
/>
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
EuiFlexGroup,
EuiFlexItem,
EuiFlyoutHeader,
EuiSkeletonTitle,
EuiTitle,
useEuiShadow,
useEuiTheme,
Expand All @@ -22,15 +23,21 @@ import {
} from '../../../common/translations';
import { NavigationSource } from '../../services/telemetry';
import { useRedirectLink } from '../../hooks';
import { FlyoutDataset } from '../../state_machines/dataset_quality_controller';
import { IntegrationIcon } from '../common';
import { BasicDataStream } from '../../../common/types';

export function Header({ dataStreamStat }: { dataStreamStat: FlyoutDataset }) {
const { integration, title } = dataStreamStat;
export function Header({
titleAndLinkDetails,
loading,
}: {
titleAndLinkDetails: BasicDataStream;
loading: boolean;
}) {
const { integration, title } = titleAndLinkDetails;
const euiShadow = useEuiShadow('s');
const { euiTheme } = useEuiTheme();
const redirectLinkProps = useRedirectLink({
dataStreamStat,
dataStreamStat: titleAndLinkDetails,
telemetry: {
page: 'details',
navigationSource: NavigationSource.Header,
Expand All @@ -39,47 +46,55 @@ export function Header({ dataStreamStat }: { dataStreamStat: FlyoutDataset }) {

return (
<EuiFlyoutHeader hasBorder>
<EuiFlexGroup justifyContent="flexStart">
<EuiFlexItem grow>
<EuiFlexGroup gutterSize="m" justifyContent="flexStart" alignItems="center">
<EuiTitle data-test-subj="datasetQualityFlyoutTitle">
<h3>{title}</h3>
</EuiTitle>
<div
{loading ? (
<EuiSkeletonTitle
size="s"
data-test-subj="datasetQualityFlyoutIntegrationLoading"
className="datasetQualityFlyoutIntegrationLoading"
/>
) : (
<EuiFlexGroup justifyContent="flexStart">
<EuiFlexItem grow>
<EuiFlexGroup gutterSize="m" justifyContent="flexStart" alignItems="center">
<EuiTitle data-test-subj="datasetQualityFlyoutTitle">
<h3>{title}</h3>
</EuiTitle>
<div
css={css`
${euiShadow};
padding: ${euiTheme.size.xs};
border-radius: ${euiTheme.size.xxs};
`}
>
<IntegrationIcon integration={integration} />
</div>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup
css={css`
${euiShadow};
padding: ${euiTheme.size.xs};
border-radius: ${euiTheme.size.xxs};
margin-right: ${euiTheme.size.l};
`}
gutterSize="s"
justifyContent="flexEnd"
alignItems="center"
>
<IntegrationIcon integration={integration} />
</div>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup
css={css`
margin-right: ${euiTheme.size.l};
`}
gutterSize="s"
justifyContent="flexEnd"
alignItems="center"
>
<EuiButton
data-test-subj="datasetQualityHeaderButton"
size="s"
{...redirectLinkProps.linkProps}
iconType={
redirectLinkProps.isLogsExplorerAvailable ? 'logoObservability' : 'discoverApp'
}
>
{redirectLinkProps.isLogsExplorerAvailable
? flyoutOpenInLogsExplorerText
: flyoutOpenInDiscoverText}
</EuiButton>
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
<EuiButton
data-test-subj="datasetQualityHeaderButton"
size="s"
{...redirectLinkProps.linkProps}
iconType={
redirectLinkProps.isLogsExplorerAvailable ? 'logoObservability' : 'discoverApp'
}
>
{redirectLinkProps.isLogsExplorerAvailable
? flyoutOpenInLogsExplorerText
: flyoutOpenInDiscoverText}
</EuiButton>
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
)}
</EuiFlyoutHeader>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ import {
} from '@elastic/eui';
import { css } from '@emotion/react';
import { RouterLinkProps } from '@kbn/router-utils/src/get_router_link_props';
import { Integration } from '../../../common/data_streams_stats/integration';
import { useDatasetQualityFlyout } from '../../hooks';
import { useFlyoutIntegrationActions } from '../../hooks/use_flyout_integration_actions';
import { Integration } from '../../../common/data_streams_stats/integration';
import { Dashboard } from '../../../common/api_types';

const integrationActionsText = i18n.translate('xpack.datasetQuality.flyoutIntegrationActionsText', {
defaultMessage: 'Integration actions',
Expand All @@ -40,15 +41,17 @@ const viewDashboardsText = i18n.translate('xpack.datasetQuality.flyoutViewDashbo

export function IntegrationActionsMenu({
integration,
dashboards,
dashboardsLoading,
}: {
integration: Integration;
dashboards: Dashboard[];
dashboardsLoading: boolean;
}) {
const { dataStreamStat, canUserAccessDashboards, canUserViewIntegrations } =
useDatasetQualityFlyout();
const { version, name: integrationName } = integration;
const { type, name } = dataStreamStat!;
const { dashboards = [], version, name: integrationName } = integration;
const {
isOpen,
handleCloseMenu,
Expand Down
Loading

0 comments on commit c483701

Please sign in to comment.