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

Update Observability Data Sources with Type and Redirection #8492

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.

Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { OpenSearchDashboardsResponse } from '../../../../../../core/server/http/router';
import { DSL_BASE } from '../../../../framework/utils/shared';
import { getUiSettings } from '../../utils';

export interface AccelerationDetailsFlyoutProps {
acceleration: CachedAcceleration;
Expand Down Expand Up @@ -231,8 +232,15 @@ export const AccelerationDetailsFlyout = (props: AccelerationDetailsFlyoutProps)
const DiscoverIcon = () => {
return (
<EuiButtonEmpty
isDisabled={(() => {
try {
return !getUiSettings().get('query:enhancements:enabled');
} catch (e) {
return false;
}
})()}
onClick={() => {
onDiscoverIconClick(acceleration, dataSourceName, application);
onDiscoverIconClick(acceleration, dataSourceName, dataSourceMDSId, application);
resetFlyout();
}}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
getRenderAccelerationDetailsFlyout,
getRenderCreateAccelerationFlyout,
} from '../../../plugin';
import { getUiSettings } from '../../utils';

interface AccelerationTableProps {
dataSourceName: string;
Expand Down Expand Up @@ -206,11 +207,18 @@
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);

Check warning on line 214 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/acceleration_management/acceleration_table.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_table.tsx#L214

Added line #L214 was not covered by tests
paulstn marked this conversation as resolved.
Show resolved Hide resolved
},
enabled: () => {
try {
return getUiSettings().get('query:enhancements:enabled');
} catch (e) {
return false;
}
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,10 @@ describe('acceleration_utils', () => {
navigateToApp: jest.fn(),
} as unknown) as ApplicationStart;

onDiscoverIconClick(acceleration, 'test_data_source', application);
expect(application.navigateToApp).toHaveBeenCalledWith('observability-logs', {
path: '#/explorer',
state: {
datasourceName: 'test_data_source',
datasourceType: 's3glue',
queryToRun: 'source = test_data_source.default.test_table | head 10',
},
onDiscoverIconClick(acceleration, 'test_data_source', 'testMDSId', application);
expect(application.navigateToApp).toHaveBeenCalledWith('data-explorer', {
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:'testMDSId',meta:(name:test_data_source,type:CUSTOM),title:'',type:DATA_SOURCE),id:'testMDSId::test_data_source.default.test_table',title:test_data_source.default.test_table,type:S3),language:SQL,query:'SELECT%20*%20FROM%20test_data_source.default.test_table%20LIMIT%2010'))",
});
});

Expand All @@ -226,14 +222,10 @@ describe('acceleration_utils', () => {
navigateToApp: jest.fn(),
} as unknown) as ApplicationStart;

onDiscoverIconClick(acceleration, 'test_data_source', application);
expect(application.navigateToApp).toHaveBeenCalledWith('observability-logs', {
path: '#/explorer',
state: {
datasourceName: 'Default cluster',
datasourceType: 'DEFAULT_INDEX_PATTERNS',
queryToRun: 'source = flint_index | head 10',
},
onDiscoverIconClick(acceleration, 'test_data_source', 'testMDSId', application);
expect(application.navigateToApp).toHaveBeenCalledWith('data-explorer', {
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:'testMDSId',title:'',type:DATA_SOURCE),id:'testMDSId::flint_index',title:flint_index,type:INDEXES),language:SQL,query:'SELECT%20*%20FROM%20flint_index%20LIMIT%2010'))",
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import { ApplicationStart } from 'opensearch-dashboards/public';
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 AccelerationHealth = ({ health }: { health: string }) => {
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(
dataSourceName,
DATA_SOURCE_TYPES.S3Glue,
dataSourceMDSId,
acceleration.database,
acceleration.table,
application
);
} else {
redirectToExplorerOSIdx(acceleration.flintIndexName, application);
redirectToDiscoverOSIdx(acceleration.flintIndexName, dataSourceMDSId, application);
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,9 @@ describe('AssociatedObjectsDetailsFlyout', () => {
renderComponent();
fireEvent.click(screen.getAllByRole('button')[0]);
await waitFor(() => {
expect(mockApplication.navigateToApp).toHaveBeenCalledWith('observability-logs', {
path: '#/explorer',
state: {
datasourceName: 'testDatasource',
datasourceType: 's3glue',
queryToRun: 'source = testDatasource.testDatabase.testTable | head 10',
},
expect(mockApplication.navigateToApp).toHaveBeenCalledWith('data-explorer', {
paulstn marked this conversation as resolved.
Show resolved Hide resolved
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:'',meta:(name:testDatasource,type:CUSTOM),title:'',type:DATA_SOURCE),id:'::testDatasource.testDatabase.testTable',title:testDatasource.testDatabase.testTable,type:S3),language:SQL,query:'SELECT%20*%20FROM%20testDatasource.testDatabase.testTable%20LIMIT%2010'))",
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ import {
} from '../acceleration_management/acceleration_utils';
import {
isCatalogCacheFetching,
redirectToExplorerWithDataSrc,
redirectToDiscoverWithDataSrc,
} from './utils/associated_objects_tab_utils';
import { getUiSettings } from '../../utils';

export interface AssociatedObjectsFlyoutProps {
tableDetail: AssociatedObject;
Expand Down Expand Up @@ -76,11 +77,18 @@ export const AssociatedObjectsDetailsFlyout = ({
const DiscoverButton = () => {
return (
<EuiButtonEmpty
isDisabled={(() => {
try {
return !getUiSettings().get('query:enhancements:enabled');
} catch (e) {
return false;
}
})()}
onClick={() => {
if (tableDetail.type !== 'table') return;
redirectToExplorerWithDataSrc(
redirectToDiscoverWithDataSrc(
tableDetail.datasource,
DATA_SOURCE_TYPES.S3Glue,
dataSourceMDSId,
tableDetail.database,
tableDetail.name,
application
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
getRenderAssociatedObjectsDetailsFlyout,
getRenderCreateAccelerationFlyout,
} from '../../../plugin';
import * as utils from '../../utils';
import { coreMock } from '../../../../../../core/public/mocks';

// Mock the imported functions
jest.mock('../../../plugin', () => ({
Expand Down Expand Up @@ -106,7 +108,11 @@ describe('AssociatedObjectsTable', () => {
});
/* eslint-dsiable no-shadow */

it('should call the correct action when clicking on the "Discover" button', async () => {
it.skip('should call the correct action when clicking on the "Discover" button', async () => {
// TODO: need to enable MDS
const { uiSettings } = coreMock.createSetup();
spyOn(utils, 'getUiSettings').and.returnValue(uiSettings);

renderComponent(props);

const discoverButton = screen.getAllByRole('button', { name: /Discover/i })[0];
Expand All @@ -116,4 +122,11 @@ describe('AssociatedObjectsTable', () => {
expect(mockApplication.navigateToApp).toHaveBeenCalled();
});
});

it('should call the correct action when clicking on the "Discover" button without query enhancements enabled', async () => {
renderComponent(props);

const discoverButton = screen.getAllByRole('button', { name: /Discover/i })[0];
expect(discoverButton).toBeDisabled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import { i18n } from '@osd/i18n';
import React, { useEffect, useState } from 'react';
import { ApplicationStart } from 'opensearch-dashboards/public';
import { ACCELERATION_INDEX_TYPES, DATA_SOURCE_TYPES } from '../../../../framework/constants';
import { ACCELERATION_INDEX_TYPES } from '../../../../framework/constants';
import { AssociatedObject, CachedAcceleration } from '../../../../framework/types';
import {
getRenderAccelerationDetailsFlyout,
Expand All @@ -24,9 +24,10 @@
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';
import { getUiSettings } from '../../utils';

interface AssociatedObjectsTableProps {
datasourceName: string;
Expand Down Expand Up @@ -177,9 +178,16 @@
description: i18n.translate(
'dataSourcesManagement.associatedObjectsTab.action.discover.description',
{
defaultMessage: 'Query in Observability Logs',
defaultMessage: 'Query in Discover',
}
),
enabled: () => {
try {
return getUiSettings().get('query:enhancements:enabled');
} catch (e) {
return false;
}
},
type: 'icon',
icon: 'discoverApp',
onClick: (asscObj: AssociatedObject) => {
Expand All @@ -188,11 +196,11 @@
const acceleration = cachedAccelerations.find(
(acc) => getAccelerationName(acc) === asscObj.name
);
redirectToExplorerOSIdx(acceleration!.flintIndexName, application);
redirectToDiscoverOSIdx(acceleration!.flintIndexName, dataSourceMDSId, application);

Check warning on line 199 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/associated_objects_table.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_table.tsx#L199

Added line #L199 was not covered by tests
} else if (asscObj.type === 'table' || asscObj.type === 'skipping') {
redirectToExplorerWithDataSrc(
redirectToDiscoverWithDataSrc(

Check warning on line 201 in src/plugins/data_source_management/public/components/direct_query_data_sources_components/associated_object_management/associated_objects_table.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_table.tsx#L201

Added line #L201 was not covered by tests
asscObj.datasource,
DATA_SOURCE_TYPES.S3Glue,
dataSourceMDSId,
asscObj.database,
asscObj.tableName,
application
Expand Down
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,54 +40,47 @@ 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',
'testMDSID',
'testDatabase',
'testTable',
application
);

expect(mockNavigateToApp).toHaveBeenCalledWith(observabilityLogsID, {
path: '#/explorer',
state: {
datasourceName: 'testDataSource',
datasourceType: 'testType',
queryToRun: 'source = testDataSource.testDatabase.testTable | head 10',
},
expect(mockNavigateToApp).toHaveBeenCalledWith('data-explorer', {
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:'testMDSID',meta:(name:testDataSource,type:CUSTOM),title:'',type:DATA_SOURCE),id:'testMDSID::testDataSource.testDatabase.testTable',title:testDataSource.testDatabase.testTable,type:S3),language:SQL,query:'SELECT%20*%20FROM%20testDataSource.testDatabase.testTable%20LIMIT%2010'))",
});
});
});

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', 'testMDSId', application);

expect(mockNavigateToApp).toHaveBeenCalledWith(observabilityLogsID, {
path: '#/explorer',
state: {
datasourceName: DEFAULT_DATA_SOURCE_NAME,
datasourceType: DEFAULT_DATA_SOURCE_TYPE,
queryToRun: 'source = testIndex | head 10',
},
expect(mockNavigateToApp).toHaveBeenCalledWith('data-explorer', {
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:'testMDSId',title:'',type:DATA_SOURCE),id:'testMDSId::testIndex',title:testIndex,type:INDEXES),language:SQL,query:'SELECT%20*%20FROM%20testIndex%20LIMIT%2010'))",
});
});
});

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
Loading
Loading