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.

Sign up for GitHub

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

Jump to bottom

Update Observability Data Sources with Type and Redirection #8492

Merged
merged 16 commits into from
Oct 21, 2024
Merged

Update Observability Data Sources with Type and Redirection #8492

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5d242f9
add type to datasources description area
paulstn Sep 28, 2024
f5a7109
Merge branch 'main' of https://github.com/opensearch-project/OpenSear…
paulstn Oct 15, 2024
d439950
complete merge
paulstn Oct 15, 2024
e94032f
keep imports
paulstn Oct 15, 2024
b5b90af
update redirection to discover
paulstn Oct 15, 2024
833ebb1
let only datasource redirection to explorer use plain discover
paulstn Oct 15, 2024
76b02cd
Changeset file for PR #8492 created/updated
opensearch-changeset-bot[bot] Oct 15, 2024
f81c9dd
update for case where flint is within default cluster
paulstn Oct 16, 2024
e616392
rename helper functions
paulstn Oct 16, 2024
4af3fdf
fix tests
paulstn Oct 16, 2024
8be2f8e
disabled redir on query enhancements disabled
paulstn Oct 16, 2024
4bb86fd
shelter against rison encoding error
paulstn Oct 16, 2024
1d2e635
Merge branch 'main' into update-obs-datasources-redir-page
paulstn Oct 16, 2024
f9fb6a9
try catch getting ui settings
paulstn Oct 16, 2024
911f22d
update tests
paulstn Oct 16, 2024
d2197e8
Merge branch 'main' into update-obs-datasources-redir-page
paulstn Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/8492.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Updated DataSource Management to redirect to Discover as well as display the type of the DataSource ([#8492](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8492))
Copy link
Member

Choose a reason for hiding this comment

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

Just FYI, the release notes generator requires the CHANGELOG description to be under 100 characters. Not sure if it counts the PR number, but the description itself is at 99 characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah my original description was over 100 characters and when I shortened it the changelog was accepted so I'd assume that the changebot checking the character count would be consistent with the release notes generator. That's just an assumption though.

2 changes: 1 addition & 1 deletion ...ect_query_data_sources_components/acceleration_management/acceleration_details_flyout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@
return (
<EuiButtonEmpty
onClick={() => {
onDiscoverIconClick(acceleration, dataSourceName, application);
onDiscoverIconClick(acceleration, dataSourceName, dataSourceMDSId, application);

Check warning on line 235 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/acceleration_management/acceleration_details_flyout.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source_management/public/components/direct_query_data_sources_components/acceleration_management/acceleration_details_flyout.tsx#L235 

Added line #L235 was not covered by tests
resetFlyout();
}}
>
Expand Down
4 changes: 2 additions & 2 deletions ...nents/direct_query_data_sources_components/acceleration_management/acceleration_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,11 @@ export const AccelerationTable = ({
const tableActions = [
{
name: 'Query Data',
description: 'Query in Observability Logs',
description: 'Query in Discover',
paulstn marked this conversation as resolved.
Show resolved Hide resolved
icon: 'discoverApp',
type: 'icon',
onClick: (acc: CachedAcceleration) => {
onDiscoverIconClick(acc, dataSourceName, application);
onDiscoverIconClick(acc, dataSourceName, dataSourceMDSId, application);
paulstn marked this conversation as resolved.
Show resolved Hide resolved
},
},
{
Expand Down
11 changes: 6 additions & 5 deletions ...nents/direct_query_data_sources_components/acceleration_management/acceleration_utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import { DATA_SOURCE_TYPES } from '../../../../framework/constants';
import { CachedAcceleration, RenderAccelerationFlyoutParams } from '../../../../framework/types';
import {
redirectToExplorerOSIdx,
redirectToExplorerWithDataSrc,
redirectToDiscoverOSIdx,
redirectToDiscoverWithDataSrc,
} from '../associated_object_management/utils/associated_objects_tab_utils';
export const ACC_PANEL_TITLE = 'Accelerations';
export const ACC_PANEL_DESC =
Expand Down Expand Up @@ -168,19 +168,20 @@
export const onDiscoverIconClick = (
acceleration: CachedAcceleration,
dataSourceName: string,
dataSourceMDSId: string | undefined,
application: ApplicationStart
) => {
// boolean determining whether its a skipping index table or mv/ci
if (acceleration.type === undefined) return;
if (acceleration.type === 'skipping') {
redirectToExplorerWithDataSrc(
redirectToDiscoverWithDataSrc(

Check warning on line 177 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/acceleration_management/acceleration_utils.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source_management/public/components/direct_query_data_sources_components/acceleration_management/acceleration_utils.tsx#L177 

Added line #L177 was not covered by tests
dataSourceName,
DATA_SOURCE_TYPES.S3Glue,
dataSourceMDSId,
acceleration.database,
acceleration.table,
application
);
} else {
redirectToExplorerOSIdx(acceleration.flintIndexName, application);
redirectToDiscoverOSIdx(acceleration.flintIndexName, dataSourceMDSId, application);

Check warning on line 185 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/acceleration_management/acceleration_utils.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source_management/public/components/direct_query_data_sources_components/acceleration_management/acceleration_utils.tsx#L185 

Added line #L185 was not covered by tests
}
};
6 changes: 3 additions & 3 deletions ...ata_sources_components/associated_object_management/associated_objects_details_flyout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
} from '../acceleration_management/acceleration_utils';
import {
isCatalogCacheFetching,
redirectToExplorerWithDataSrc,
redirectToDiscoverWithDataSrc,
} from './utils/associated_objects_tab_utils';

export interface AssociatedObjectsFlyoutProps {
Expand Down Expand Up @@ -78,9 +78,9 @@
<EuiButtonEmpty
onClick={() => {
if (tableDetail.type !== 'table') return;
redirectToExplorerWithDataSrc(
redirectToDiscoverWithDataSrc(

Check warning on line 81 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/associated_objects_details_flyout.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/associated_objects_details_flyout.tsx#L81 

Added line #L81 was not covered by tests
tableDetail.datasource,
DATA_SOURCE_TYPES.S3Glue,
dataSourceMDSId,
tableDetail.database,
tableDetail.name,
application
Expand Down
12 changes: 6 additions & 6 deletions ...t_query_data_sources_components/associated_object_management/associated_objects_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import {
ASSC_OBJ_TABLE_ACC_COLUMN_NAME,
ASSC_OBJ_TABLE_SEARCH_HINT,
ASSC_OBJ_TABLE_SUBJ,
redirectToExplorerOSIdx,
redirectToExplorerWithDataSrc,
redirectToDiscoverOSIdx,
redirectToDiscoverWithDataSrc,
} from './utils/associated_objects_tab_utils';

interface AssociatedObjectsTableProps {
Expand Down Expand Up @@ -177,7 +177,7 @@ export const AssociatedObjectsTable = (props: AssociatedObjectsTableProps) => {
description: i18n.translate(
'dataSourcesManagement.associatedObjectsTab.action.discover.description',
{
defaultMessage: 'Query in Observability Logs',
defaultMessage: 'Query in Discover',
}
),
type: 'icon',
Expand All @@ -188,11 +188,11 @@ export const AssociatedObjectsTable = (props: AssociatedObjectsTableProps) => {
const acceleration = cachedAccelerations.find(
(acc) => getAccelerationName(acc) === asscObj.name
);
redirectToExplorerOSIdx(acceleration!.flintIndexName, application);
redirectToDiscoverOSIdx(acceleration!.flintIndexName, dataSourceMDSId, application);
} else if (asscObj.type === 'table' || asscObj.type === 'skipping') {
redirectToExplorerWithDataSrc(
redirectToDiscoverWithDataSrc(
asscObj.datasource,
DATA_SOURCE_TYPES.S3Glue,
dataSourceMDSId,
asscObj.database,
asscObj.tableName,
application
Expand Down
18 changes: 9 additions & 9 deletions ...urces_components/associated_object_management/utils/associated_objects_tab_utils.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ import { ApplicationStart } from 'opensearch-dashboards/public';
import { DirectQueryLoadingStatus } from '../../../../../framework/types';
import {
isCatalogCacheFetching,
redirectToExplorerWithDataSrc,
redirectToExplorerOSIdx,
redirectToExplorerS3,
redirectToDiscoverWithDataSrc,
redirectToDiscoverOSIdx,
} from './associated_objects_tab_utils';
import {
DEFAULT_DATA_SOURCE_NAME,
Expand Down Expand Up @@ -41,12 +40,12 @@ describe('AssociatedObjectsTab utils', () => {
});
});

describe('redirectToExplorerWithDataSrc', () => {
describe('redirectToDiscoverWithDataSrc', () => {
it('navigates to the explorer with the correct state', () => {
const mockNavigateToApp = jest.fn();
const application = ({ navigateToApp: mockNavigateToApp } as unknown) as ApplicationStart;

redirectToExplorerWithDataSrc(
redirectToDiscoverWithDataSrc(
'testDataSource',
'testType',
'testDatabase',
Expand All @@ -65,12 +64,12 @@ describe('AssociatedObjectsTab utils', () => {
});
});

describe('redirectToExplorerOSIdx', () => {
describe('redirectToDiscoverOSIdx', () => {
it('navigates to the explorer with the correct state', () => {
const mockNavigateToApp = jest.fn();
const application = ({ navigateToApp: mockNavigateToApp } as unknown) as ApplicationStart;

redirectToExplorerOSIdx('testIndex', application);
redirectToDiscoverOSIdx('testIndex', application);

expect(mockNavigateToApp).toHaveBeenCalledWith(observabilityLogsID, {
path: '#/explorer',
Expand All @@ -83,12 +82,13 @@ describe('AssociatedObjectsTab utils', () => {
});
});

describe('redirectToExplorerS3', () => {
describe.skip('redirectToExplorerS3', () => {
it('navigates to the explorer with the correct state', () => {
const mockNavigateToApp = jest.fn();
const application = ({ navigateToApp: mockNavigateToApp } as unknown) as ApplicationStart;

redirectToExplorerS3('testDataSource', application);
// TODO: test when redirection to discover accepts only datasource
// redirectToExplorerS3('testDataSource', application);

expect(mockNavigateToApp).toHaveBeenCalledWith(observabilityLogsID, {
path: '#/explorer',
Expand Down
57 changes: 23 additions & 34 deletions ...ta_sources_components/associated_object_management/utils/associated_objects_tab_utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,49 +43,38 @@
);
};

export const redirectToExplorerWithDataSrc = (
export const redirectToDiscoverWithDataSrc = (
datasourceName: string,
datasourceType: string,
datasourceMDSId: string | undefined,
databaseName: string,
tableName: string,
application: ApplicationStart
) => {
const queryIndex = `${datasourceName}.${databaseName}.${tableName}`;
redirectToExplorerWithQuery(datasourceName, datasourceType, queryIndex, application);
};

export const redirectToExplorerOSIdx = (indexName: string, application: ApplicationStart) => {
redirectToExplorerWithQuery(
DEFAULT_DATA_SOURCE_NAME,
DEFAULT_DATA_SOURCE_TYPE,
indexName,
application
);
};

export const redirectToExplorerS3 = (datasourceName: string, application: ApplicationStart) => {
application.navigateToApp(observabilityLogsID, {
path: `#/explorer`,
state: {
datasourceName,
datasourceType: DATA_SOURCE_TYPES.S3Glue,
},
application.navigateToApp('data-explorer', {

Check warning on line 53 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/utils/associated_objects_tab_utils.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/utils/associated_objects_tab_utils.tsx#L53 

Added line #L53 was not covered by tests
path: `discover#?_a=(discover:(columns:!(_source),isDirty:!f,sort:!()),metadata:(view:discover))&_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-15m,to:now))&_q=(filters:!(),query:(dataset:(dataSource:(id:'${
Copy link
Member

Choose a reason for hiding this comment

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

nit: Feel like these two functions have a lot of duplicate code, could be worth a try to either combine them or wrap the actual path in another function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally when it was just the 'state' being redirected to explorer through navigateToApp, that made a lot of sense. However, since Discover's url redirection has different forms between redirecting to a flint index vs an index pattern, the differences i would need to put in between both strings would be too much for it to be worth it, as I think it would greatly reduce readability.

datasourceMDSId ?? ''
}',meta:(name:${datasourceName},type:CUSTOM),title:'',type:DATA_SOURCE),id:'${
datasourceMDSId ?? ''
}::${datasourceName}.${databaseName}.${tableName}',title:${datasourceName}.${databaseName}.${tableName},type:S3),language:SQL,query:'SELECT%20*%20FROM%20${datasourceName}.${databaseName}.${tableName}%20LIMIT%2010'))`,
paulstn marked this conversation as resolved.
Show resolved Hide resolved
});
};

const redirectToExplorerWithQuery = (
datasourceName: string,
datasourceType: string,
queriedIndex: string,
export const redirectToDiscoverOSIdx = (
paulstn marked this conversation as resolved.
Show resolved Hide resolved
indexName: string,
datasourceMDSId: string | undefined,
application: ApplicationStart
) => {
// navigate to explorer
application.navigateToApp(observabilityLogsID, {
path: `#/explorer`,
state: {
datasourceName,
datasourceType,
queryToRun: `source = ${queriedIndex} | head 10`,
},
application.navigateToApp('data-explorer', {

Check warning on line 67 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/utils/associated_objects_tab_utils.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/utils/associated_objects_tab_utils.tsx#L67 

Added line #L67 was not covered by tests
path: `discover#?_a=(discover:(columns:!(_source),isDirty:!f,sort:!()),metadata:(view:discover))&_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-15m,to:now))&_q=(filters:!(),query:(dataset:(dataSource:(id:${
datasourceMDSId ?? ''
},title:'',type:DATA_SOURCE),id:'${
datasourceMDSId ?? ''
}::${indexName}',title:${indexName},type:INDEXES),language:SQL,query:'SELECT%20*%20FROM%20${indexName}%20LIMIT%2010'))`,
});
};

export const redirectToDiscover = (application: ApplicationStart) => {
application.navigateToApp('data-explorer', {

Check warning on line 77 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/utils/associated_objects_tab_utils.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/utils/associated_objects_tab_utils.tsx#L77 

Added line #L77 was not covered by tests
path: `discover#`,
});
};
2 changes: 1 addition & 1 deletion ...t_query_data_sources_components/connection_detail/direct_query_connection_detail.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ jest.mock('../associated_object_management/associated_objects_tab', () => ({
}));

jest.mock('../associated_object_management/utils/associated_objects_tab_utils', () => ({
redirectToExplorerS3: jest.fn(),
redirectToDiscover: jest.fn(),
}));

const mockApplication = coreMock.createSetup().application;
Expand Down
47 changes: 44 additions & 3 deletions ...direct_query_data_sources_components/connection_detail/direct_query_connection_detail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
EuiPanel,
EuiSpacer,
EuiTabbedContent,
EuiTitle,
EuiText,
} from '@elastic/eui';
import { escapeRegExp } from 'lodash';
Expand Down Expand Up @@ -46,14 +47,16 @@ import { createDataSourceMenu, DataSourceViewConfig } from '../../data_source_me
import { getDataSourcesWithFields, isPluginInstalled } from '../../utils';
import { AccelerationTable } from '../acceleration_management/acceleration_table';
import { AssociatedObjectsTab } from '../associated_object_management/associated_objects_tab';
import { redirectToExplorerS3 } from '../associated_object_management/utils/associated_objects_tab_utils';
import { redirectToDiscover } from '../associated_object_management/utils/associated_objects_tab_utils';
import {
InstalledIntegrationsTable,
InstallIntegrationFlyout,
} from '../integrations/installed_integrations_table';
import { AccessControlTab } from './access_control_tab';
import { InactiveDataConnectionCallout } from './utils/inactive_data_connection_callout';
import { NoAccess } from './utils/no_access_page';
import prometheusSvg from '../../direct_query_data_sources_components/icons/prometheus_logo.svg';
import s3Svg from '../../direct_query_data_sources_components/icons/s3_logo.svg';

interface DirectQueryDataConnectionDetailProps {
featureFlagStatus: boolean;
Expand Down Expand Up @@ -92,7 +95,7 @@ export const DirectQueryDataConnectionDetail: React.FC<DirectQueryDataConnection
const [hasAccess, setHasAccess] = useState(true);

const onclickDiscoverCard = () => {
redirectToExplorerS3(dataSourceName, application);
redirectToDiscover(application);
};

const dataSourceMenuView = useMemo(() => {
Expand Down Expand Up @@ -304,7 +307,7 @@ export const DirectQueryDataConnectionDetail: React.FC<DirectQueryDataConnection
selectable={{
onClick: onclickDiscoverCard,
isDisabled: false,
children: 'Query in Observability Logs',
children: 'Query in Discover',
}}
/>
</EuiFlexItem>
Expand Down Expand Up @@ -339,6 +342,25 @@ export const DirectQueryDataConnectionDetail: React.FC<DirectQueryDataConnection
const S3DatasourceOverview = () => (
<EuiPanel>
<EuiFlexGroup>
<EuiFlexItem grow={false}>
paulstn marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexGroup direction="column" alignItems="center">
<EuiFlexItem grow={false}>
<EuiText className="overview-title">Data Source Type</EuiText>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup alignItems="center" gutterSize="s">
<EuiFlexItem grow={false}>
<EuiIcon type={s3Svg} size="xl" />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiTitle size="s">
<h3>Amazon S3</h3>
</EuiTitle>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem>
<EuiFlexGroup direction="column">
<EuiFlexItem grow={false}>
Expand Down Expand Up @@ -369,6 +391,25 @@ export const DirectQueryDataConnectionDetail: React.FC<DirectQueryDataConnection
const PrometheusDatasourceOverview = () => (
<EuiPanel>
<EuiFlexGroup>
<EuiFlexItem grow={false}>
<EuiFlexGroup direction="column" alignItems="center">
<EuiFlexItem grow={false}>
<EuiText className="overview-title">Data Source Type</EuiText>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup alignItems="center" gutterSize="s">
<EuiFlexItem grow={false}>
<EuiIcon type={prometheusSvg} size="xl" />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiTitle size="s">
<h3>Prometheus</h3>
</EuiTitle>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem>
<EuiFlexGroup direction="column">
<EuiFlexItem grow={false}>
Expand Down
Loading