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

Prefer DataView client over SavedObjects client when possible #136694

Merged
merged 5 commits into from
Jul 22, 2022
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
4 changes: 0 additions & 4 deletions x-pack/plugins/apm/public/application/application.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { embeddablePluginMock } from '@kbn/embeddable-plugin/public/mocks';
import { ApmPluginSetupDeps, ApmPluginStartDeps } from '../plugin';

jest.mock('../services/rest/data_view', () => ({
createStaticDataView: () => Promise.resolve(undefined),
}));

describe('renderApp (APM)', () => {
let mockConsole: jest.SpyInstance;
beforeAll(() => {
Expand Down
7 changes: 0 additions & 7 deletions x-pack/plugins/apm/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { ConfigSchema } from '..';
import { ApmPluginSetupDeps, ApmPluginStartDeps } from '../plugin';
import { createCallApmApi } from '../services/rest/create_call_apm_api';
import { createStaticDataView } from '../services/rest/data_view';
import { setHelpExtension } from '../set_help_extension';
import { setReadonlyBadge } from '../update_badge';
import { ApmAppRoot } from '../components/routing/app_root';
Expand Down Expand Up @@ -61,12 +60,6 @@ export const renderApp = ({
setReadonlyBadge(coreStart);
createCallApmApi(coreStart);

// Automatically creates static data view and stores as saved object
createStaticDataView().catch((e) => {
// eslint-disable-next-line no-console
console.log('Error creating static data view', e);
});

// add .kbnAppWrappers class to root element
element.classList.add(APP_WRAPPER_CLASS);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ const stories: Meta<{}> = {
default:
return {};
}
return {};
},
},
notifications: { toasts: { add: () => {}, addWarning: () => {} } },
uiSettings: { get: () => [] },
dataViews: { get: async () => {} },
} as unknown as CoreStart;

const KibanaReactContext = createKibanaReactContext(coreMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
TraceSearchQuery,
TraceSearchType,
} from '../../../../../common/trace_explorer';
import { useStaticDataView } from '../../../../hooks/use_static_data_view';
import { useApmDataView } from '../../../../hooks/use_apm_data_view';
import { useApmPluginContext } from '../../../../context/apm_plugin/use_apm_plugin_context';
import { EQLCodeEditorSuggestionType } from '../../../shared/eql_code_editor/constants';
import { LazilyLoadedEQLCodeEditor } from '../../../shared/eql_code_editor/lazily_loaded_code_editor';
Expand Down Expand Up @@ -57,7 +57,7 @@ export function TraceSearchBox({
loading,
}: Props) {
const { unifiedSearch } = useApmPluginContext();
const { value: dataView } = useStaticDataView();
const { dataView } = useApmDataView();

return (
<EuiFlexGroup direction="column">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type { DataView } from '@kbn/data-views-plugin/public';
import { useApmPluginContext } from '../../../context/apm_plugin/use_apm_plugin_context';
import { useLegacyUrlParams } from '../../../context/url_params_context/use_url_params';
import { useApmParams } from '../../../hooks/use_apm_params';
import { useDynamicDataViewFetcher } from '../../../hooks/use_dynamic_data_view';
import { useApmDataView } from '../../../hooks/use_apm_data_view';
import { fromQuery, toQuery } from '../links/url_helpers';
import { getBoolFilter } from './get_bool_filter';
// @ts-expect-error
Expand Down Expand Up @@ -71,8 +71,7 @@ export function KueryBar(props: {
};

const example = examples[processorEvent || 'defaults'];

const { dataView } = useDynamicDataViewFetcher();
const { dataView } = useApmDataView();

const placeholder =
props.placeholder ??
Expand Down Expand Up @@ -106,7 +105,7 @@ export function KueryBar(props: {
const suggestions = (
(await unifiedSearch.autocomplete.getQuerySuggestions({
language: 'kuery',
indexPatterns: [dataView as DataView],
indexPatterns: [dataView],
boolFilter:
props.boolFilter ??
getBoolFilter({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { getByTestId, fireEvent, getByText } from '@testing-library/react';
import { getByTestId, fireEvent, getByText, act } from '@testing-library/react';
import { createMemoryHistory, MemoryHistory } from 'history';
import React from 'react';
import { Router } from 'react-router-dom';
Expand Down Expand Up @@ -37,6 +37,7 @@ function setup({

const KibanaReactContext = createKibanaReactContext({
usageCollection: { reportUiCounter: () => {} },
dataViews: { get: async () => {} },
} as Partial<CoreStart>);

// mock transaction types
Expand Down Expand Up @@ -91,7 +92,7 @@ describe('when transactionType is selected and multiple transaction types are gi
expect(dropdown).toHaveValue('secondType');
});

it('should update the URL when a transaction type is selected', () => {
it('should update the URL when a transaction type is selected', async () => {
const { container } = setup({
history,
serviceTransactionTypes: ['firstType', 'secondType'],
Expand All @@ -112,7 +113,9 @@ describe('when transactionType is selected and multiple transaction types are gi
expect(getByText(dropdown, 'secondType')).toBeInTheDocument();

// change dropdown value
fireEvent.change(dropdown, { target: { value: 'firstType' } });
await act(async () => {
fireEvent.change(dropdown, { target: { value: 'firstType' } });
});

// assert that value was changed
expect(dropdown).toHaveValue('firstType');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const mockCore = merge({}, coreStart, {
capabilities: {
apm: {},
ml: {},
savedObjectsManagement: { edit: true },
},
},
uiSettings: {
Expand Down
66 changes: 66 additions & 0 deletions x-pack/plugins/apm/public/hooks/use_apm_data_view.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* 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 { DataView } from '@kbn/data-views-plugin/common';
import { useKibana } from '@kbn/kibana-react-plugin/public';
import { SavedObjectNotFound } from '@kbn/kibana-utils-plugin/common';
import { useEffect, useState } from 'react';
import { APM_STATIC_DATA_VIEW_ID } from '../../common/data_view_constants';
import { useApmPluginContext } from '../context/apm_plugin/use_apm_plugin_context';
import { ApmPluginStartDeps } from '../plugin';
import { callApmApi } from '../services/rest/create_call_apm_api';

async function createStaticApmDataView() {
const res = await callApmApi('POST /internal/apm/data_view/static', {
signal: null,
});
return res.dataView;
}

async function getApmDataViewTitle() {
const res = await callApmApi('GET /internal/apm/data_view/title', {
signal: null,
});
return res.apmDataViewTitle;
}

export function useApmDataView() {
const { services } = useKibana<ApmPluginStartDeps>();
const { core } = useApmPluginContext();
const [dataView, setDataView] = useState<DataView | undefined>();

const canCreateDataView =
core.application.capabilities.savedObjectsManagement.edit;

useEffect(() => {
async function fetchDataView() {
try {
// load static data view
return await services.dataViews.get(APM_STATIC_DATA_VIEW_ID);
} catch (e) {
// re-throw if an unhandled error occurred
const notFound = e instanceof SavedObjectNotFound;
if (!notFound) {
throw e;
}

// create static data view if user has permissions
if (canCreateDataView) {
return createStaticApmDataView();
} else {
// or create dynamic data view if user does not have permissions to create a static
const title = await getApmDataViewTitle();
return services.dataViews.create({ title });
}
}
}

fetchDataView().then((dv) => setDataView(dv));
}, [canCreateDataView, services.dataViews]);

return { dataView };
}
21 changes: 0 additions & 21 deletions x-pack/plugins/apm/public/hooks/use_dynamic_data_view.ts

This file was deleted.

16 changes: 0 additions & 16 deletions x-pack/plugins/apm/public/hooks/use_static_data_view.ts

This file was deleted.

14 changes: 0 additions & 14 deletions x-pack/plugins/apm/public/services/rest/data_view.ts

This file was deleted.

6 changes: 0 additions & 6 deletions x-pack/plugins/apm/server/lib/helpers/setup_request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ jest.mock('../../routes/settings/apm_indices/get_apm_indices', () => ({
} as Awaited<ReturnType<typeof getApmIndices>>),
}));

jest.mock('../../routes/data_view/get_dynamic_data_view', () => ({
getDynamicDataView: async () => {
return;
},
}));

function getMockResources() {
const esClientMock = elasticsearchServiceMock.createScopedClusterClient();
// @ts-expect-error incomplete definition
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/apm/server/lib/helpers/setup_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ export async function setupRequest({
config,
}),
withApmSpan('get_ui_settings', () =>
coreContext.uiSettings.client.get(UI_SETTINGS.SEARCH_INCLUDE_FROZEN)
coreContext.uiSettings.client.get<boolean>(
UI_SETTINGS.SEARCH_INCLUDE_FROZEN
)
),
]);

Expand Down
Loading