Skip to content

Commit

Permalink
[ML] Anomaly Detection: Fix anomaly marker click in Single Metric Vie…
Browse files Browse the repository at this point in the history
…wer embeddable (elastic#176788)

## Summary

Part of elastic#176651 and elastic#153476. Follow up to elastic#175556.

Fixes the click on anomaly markers in the Single Metric Viewer
embeddable. This required refactoring some dependencies to be properly
passed in via React contexts instead of the legacy dependency cache.

[smv-embeddable-click-0001.webm](https://github.com/elastic/kibana/assets/230104/6d0bb0e5-bfde-4429-8876-bdc94ce646c8)

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
walterra authored Mar 5, 2024
1 parent 15cdcf2 commit dda8c8f
Show file tree
Hide file tree
Showing 60 changed files with 550 additions and 443 deletions.
35 changes: 21 additions & 14 deletions x-pack/plugins/ml/public/application/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import './_index.scss';
import ReactDOM from 'react-dom';
import { pick } from 'lodash';

import type { DataViewsContract } from '@kbn/data-views-plugin/public';
import type { AppMountParameters, CoreStart, HttpStart } from '@kbn/core/public';
import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/public';
import { DatePickerContextProvider, type DatePickerDependencies } from '@kbn/ml-date-picker';
Expand All @@ -33,6 +34,8 @@ import { HttpService } from './services/http_service';
import type { PageDependencies } from './routing/router';
import { EnabledFeaturesContextProvider } from './contexts/ml';
import type { StartServices } from './contexts/kibana';
import { fieldFormatServiceFactory } from './services/field_format_service_factory';
import { indexServiceFactory } from './util/index_service';

export type MlDependencies = Omit<
MlSetupDependencies,
Expand All @@ -53,13 +56,29 @@ const localStorage = new Storage(window.localStorage);
/**
* Provides global services available across the entire ML app.
*/
export function getMlGlobalServices(httpStart: HttpStart, usageCollection?: UsageCollectionSetup) {
export function getMlGlobalServices(
httpStart: HttpStart,
dataViews: DataViewsContract,
usageCollection?: UsageCollectionSetup
) {
const httpService = new HttpService(httpStart);
const mlApiServices = mlApiServicesProvider(httpService);
// Note on the following services:
// - `mlIndexUtils` is just instantiated here to be passed on to `mlFieldFormatService`,
// but it's not being made available as part of global services. Since it's just
// some stateless utils `useMlIndexUtils()` should be used from within components.
// - `mlFieldFormatService` is a stateful legacy service that relied on "dependency cache",
// so because of its own state it needs to be made available as a global service.
// In the long run we should again try to get rid of it here and make it available via
// its own context or possibly without having a singleton like state at all, since the
// way this manages its own state right now doesn't consider React component lifecycles.
const mlIndexUtils = indexServiceFactory(dataViews);
const mlFieldFormatService = fieldFormatServiceFactory(mlApiServices, mlIndexUtils);

return {
httpService,
mlApiServices,
mlFieldFormatService,
mlUsageCollection: mlUsageCollectionProvider(usageCollection),
mlCapabilities: new MlCapabilitiesService(mlApiServices),
mlLicense: new MlLicense(),
Expand Down Expand Up @@ -106,7 +125,7 @@ const App: FC<AppProps> = ({ coreStart, deps, appMountParams, isServerless, mlFe
unifiedSearch: deps.unifiedSearch,
usageCollection: deps.usageCollection,
...coreStart,
mlServices: getMlGlobalServices(coreStart.http, deps.usageCollection),
mlServices: getMlGlobalServices(coreStart.http, deps.data.dataViews, deps.usageCollection),
};
}, [deps, coreStart]);

Expand Down Expand Up @@ -168,25 +187,13 @@ export const renderApp = (
setDependencyCache({
timefilter: deps.data.query.timefilter,
fieldFormats: deps.fieldFormats,
autocomplete: deps.unifiedSearch.autocomplete,
config: coreStart.uiSettings!,
chrome: coreStart.chrome!,
docLinks: coreStart.docLinks!,
toastNotifications: coreStart.notifications.toasts,
overlays: coreStart.overlays,
theme: coreStart.theme,
recentlyAccessed: coreStart.chrome!.recentlyAccessed,
basePath: coreStart.http.basePath,
savedSearch: deps.savedSearch,
application: coreStart.application,
http: coreStart.http,
security: deps.security,
dashboard: deps.dashboard,
maps: deps.maps,
dataVisualizer: deps.dataVisualizer,
dataViews: deps.data.dataViews,
share: deps.share,
lens: deps.lens,
});

appMountParams.onAppLeave((actions) => actions.default());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { EuiFlexGroup, EuiFlexItem, EuiInMemoryTable, EuiText } from '@elastic/e

import { FormattedMessage } from '@kbn/i18n-react';
import { usePageUrlState } from '@kbn/ml-url-state';
import { context } from '@kbn/kibana-react-plugin/public';

import { getColumns } from './anomalies_table_columns';

Expand All @@ -29,6 +30,8 @@ import { ml } from '../../services/ml_api_service';
import { INFLUENCERS_LIMIT, ANOMALIES_TABLE_TABS, MAX_CHARS } from './anomalies_table_constants';

export class AnomaliesTableInternal extends Component {
static contextType = context;

constructor(props) {
super(props);

Expand Down Expand Up @@ -189,6 +192,7 @@ export class AnomaliesTableInternal extends Component {
}

const columns = getColumns(
this.context.services.mlServices.mlFieldFormatService,
tableData.anomalies,
tableData.jobIds,
tableData.examplesByJobId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ jest.mock('../../license', () => ({
jest.mock('../../capabilities/get_capabilities', () => ({
getCapabilities: () => {},
}));
jest.mock('../../services/field_format_service', () => ({
getFieldFormat: () => {},
}));
jest.mock('./links_menu', () => () => <div id="mocLinkCom">mocked link component</div>);
jest.mock('./description_cell', () => () => (
<div id="mockDescriptorCom">mocked description component</div>
Expand All @@ -31,6 +28,10 @@ jest.mock('./influencers_cell', () => () => (
<div id="mocInfluencerCom">mocked influencer component</div>
));

const mlFieldFormatServiceMock = {
getFieldFormat: () => {},
};

const columnData = {
items: mockAnomaliesTableData.default.anomalies,
jobIds: mockAnomaliesTableData.default.jobIds,
Expand All @@ -48,6 +49,7 @@ const columnData = {
describe('AnomaliesTable', () => {
test('all columns created', () => {
const columns = getColumns(
mlFieldFormatServiceMock,
columnData.items,
columnData.jobIds,
columnData.examplesByJobId,
Expand Down Expand Up @@ -103,6 +105,7 @@ describe('AnomaliesTable', () => {
};

const columns = getColumns(
mlFieldFormatServiceMock,
noEntityValueColumnData.items,
noEntityValueColumnData.jobIds,
noEntityValueColumnData.examplesByJobId,
Expand Down Expand Up @@ -133,6 +136,7 @@ describe('AnomaliesTable', () => {
};

const columns = getColumns(
mlFieldFormatServiceMock,
noInfluencersColumnData.items,
noInfluencersColumnData.jobIds,
noInfluencersColumnData.examplesByJobId,
Expand Down Expand Up @@ -163,6 +167,7 @@ describe('AnomaliesTable', () => {
};

const columns = getColumns(
mlFieldFormatServiceMock,
noActualColumnData.items,
noActualColumnData.jobIds,
noActualColumnData.examplesByJobId,
Expand Down Expand Up @@ -193,6 +198,7 @@ describe('AnomaliesTable', () => {
};

const columns = getColumns(
mlFieldFormatServiceMock,
noTypicalColumnData.items,
noTypicalColumnData.jobIds,
noTypicalColumnData.examplesByJobId,
Expand Down Expand Up @@ -223,6 +229,7 @@ describe('AnomaliesTable', () => {
};

const columns = getColumns(
mlFieldFormatServiceMock,
multipleJobIdsData.items,
multipleJobIdsData.jobIds,
multipleJobIdsData.examplesByJobId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import { EntityCell } from '../entity_cell';
import { InfluencersCell } from './influencers_cell';
import { LinksMenu } from './links_menu';
import { checkPermission } from '../../capabilities/check_capabilities';
import { mlFieldFormatService } from '../../services/field_format_service';
import { formatValue } from '../../formatters/format_value';
import { INFLUENCERS_LIMIT, ANOMALIES_TABLE_TABS } from './anomalies_table_constants';
import { SeverityCell } from './severity_cell';
Expand Down Expand Up @@ -56,6 +55,7 @@ function showLinksMenuForItem(item, showViewSeriesLink, sourceIndicesWithGeoFiel
}

export function getColumns(
mlFieldFormatService,
items,
jobIds,
examplesByJobId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import { escapeQuotes } from '@kbn/es-query';
import { isQuery } from '@kbn/data-plugin/public';

import { PLUGIN_ID } from '../../../../common/constants/app';
import { findMessageField, getDataViewIdFromName } from '../../util/index_utils';
import { findMessageField } from '../../util/index_utils';
import { getInitialAnomaliesLayers, getInitialSourceIndexFieldLayers } from '../../../maps/util';
import { parseInterval } from '../../../../common/util/parse_interval';
import { ML_APP_LOCATOR, ML_PAGES } from '../../../../common/constants/locator';
Expand All @@ -61,6 +61,7 @@ import { usePermissionCheck } from '../../capabilities/check_capabilities';
import type { TimeRangeBounds } from '../../util/time_buckets';
import { useMlKibana } from '../../contexts/kibana';
import { getFieldTypeFromMapping } from '../../services/mapping_service';
import { useMlIndexUtils } from '../../util/index_service';

import { getQueryStringForInfluencers } from './get_query_string_for_influencers';

Expand Down Expand Up @@ -97,6 +98,7 @@ export const LinksMenuUI = (props: LinksMenuProps) => {
const {
services: { data, share, application, uiActions },
} = kibana;
const { getDataViewIdFromName } = useMlIndexUtils();

const job = useMemo(() => {
return mlJobService.getJob(props.anomaly.jobId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
import { FormattedMessage } from '@kbn/i18n-react';

import { i18n } from '@kbn/i18n';
import { withKibana } from '@kbn/kibana-react-plugin/public';
import { context } from '@kbn/kibana-react-plugin/public';
import type { DataViewListItem } from '@kbn/data-views-plugin/common';
import type { MlUrlConfig } from '@kbn/ml-anomaly-utils';
import { isDataFrameAnalyticsConfigs } from '@kbn/ml-data-frame-analytics-utils';
Expand All @@ -42,9 +42,9 @@ import {
getTestUrl,
type CustomUrlSettings,
} from './custom_url_editor/utils';
import { loadDataViewListItems } from '../../jobs/jobs_list/components/edit_job_flyout/edit_utils';
import { openCustomUrlWindow } from '../../util/custom_url_utils';
import type { CustomUrlsWrapperProps } from './custom_urls_wrapper';
import { indexServiceFactory } from '../../util/index_service';

interface CustomUrlsState {
customUrls: MlUrlConfig[];
Expand All @@ -55,13 +55,15 @@ interface CustomUrlsState {
supportedFilterFields: string[];
}
interface CustomUrlsProps extends CustomUrlsWrapperProps {
kibana: MlKibanaReactContextValue;
currentTimeFilter?: EsQueryTimeRange;
dashboardService: DashboardService;
isPartialDFAJob?: boolean;
}

class CustomUrlsUI extends Component<CustomUrlsProps, CustomUrlsState> {
export class CustomUrls extends Component<CustomUrlsProps, CustomUrlsState> {
static contextType = context;
declare context: MlKibanaReactContextValue;

private toastNotificationService: ToastNotificationService | undefined;

constructor(props: CustomUrlsProps) {
Expand All @@ -84,9 +86,10 @@ class CustomUrlsUI extends Component<CustomUrlsProps, CustomUrlsState> {
}

componentDidMount() {
const { toasts } = this.props.kibana.services.notifications;
const { toasts } = this.context.services.notifications;
this.toastNotificationService = toastNotificationServiceProvider(toasts);
const { dashboardService } = this.props;
const mlIndexUtils = indexServiceFactory(this.context.services.data.dataViews);

dashboardService
.fetchDashboards()
Expand All @@ -105,7 +108,8 @@ class CustomUrlsUI extends Component<CustomUrlsProps, CustomUrlsState> {
);
});

loadDataViewListItems()
mlIndexUtils
.loadDataViewListItems()
.then((dataViewListItems) => {
this.setState({ dataViewListItems });
})
Expand Down Expand Up @@ -146,7 +150,7 @@ class CustomUrlsUI extends Component<CustomUrlsProps, CustomUrlsState> {
};

addNewCustomUrl = () => {
const { dashboard } = this.props.kibana.services;
const { dashboard } = this.context.services;

buildCustomUrlFromSettings(dashboard, this.state.editorSettings as CustomUrlSettings)
.then((customUrl) => {
Expand All @@ -173,7 +177,7 @@ class CustomUrlsUI extends Component<CustomUrlsProps, CustomUrlsState> {
http: { basePath },
data: { dataViews },
dashboard,
} = this.props.kibana.services;
} = this.context.services;
const dataViewId = this.state?.editorSettings?.kibanaSettings?.discoverIndexPatternId;
const job = this.props.job;
dataViews
Expand Down Expand Up @@ -361,5 +365,3 @@ class CustomUrlsUI extends Component<CustomUrlsProps, CustomUrlsState> {
);
}
}

export const CustomUrls = withKibana(CustomUrlsUI);
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ export const kibanaContextMock = {
mlCapabilities: {
refreshCapabilities: jest.fn(),
},
mlFieldFormatService: {
getFieldFormat: jest.fn(),
},
},
notifications: notificationServiceMock.createStartContract(),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import {
} from '@kbn/ml-data-frame-analytics-utils';

import { useMlKibana } from '../../contexts/kibana';
import { getDataViewIdFromName } from '../../util/index_utils';
import { ml } from '../../services/ml_api_service';
import { newJobCapsServiceAnalytics } from '../../services/new_job_capabilities/new_job_capabilities_service_analytics';
import { useMlIndexUtils } from '../../util/index_service';

import { isGetDataFrameAnalyticsStatsResponseOk } from '../pages/analytics_management/services/analytics_service/get_analytics';
import { useTrainedModelsApiService } from '../../services/ml_api_service/trained_models';
Expand All @@ -35,6 +35,7 @@ export const useResultsViewConfig = (jobId: string) => {
data: { dataViews },
},
} = useMlKibana();
const { getDataViewIdFromName } = useMlIndexUtils();
const trainedModelsApiService = useTrainedModelsApiService();

const [dataView, setDataView] = useState<DataView | undefined>(undefined);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ import {
useMlKibana,
} from '../../../../contexts/kibana';
import { useEnabledFeatures } from '../../../../contexts/ml';
import { getDataViewIdFromName } from '../../../../util/index_utils';
import { useNavigateToWizardWithClonedJob } from '../../analytics_management/components/action_clone/clone_action_name';
import {
useDeleteAction,
DeleteActionModal,
} from '../../analytics_management/components/action_delete';
import { DeleteSpaceAwareItemCheckModal } from '../../../../components/delete_space_aware_item_check_modal';
import { useMlIndexUtils } from '../../../../util/index_service';

interface Props {
details: Record<string, any>;
Expand Down Expand Up @@ -115,6 +115,7 @@ export const Controls: FC<Props> = React.memo(
application: { navigateToUrl, capabilities },
},
} = useMlKibana();
const { getDataViewIdFromName } = useMlIndexUtils();

const hasIngestPipelinesCapabilities =
capabilities.management?.ingest?.ingest_pipelines === true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
import { from } from 'rxjs';
import { map } from 'rxjs/operators';

import { mlFieldFormatService } from '../../services/field_format_service';
import type { MlFieldFormatService } from '../../services/field_format_service';
import { mlJobService } from '../../services/job_service';

import { EXPLORER_ACTION } from '../explorer_constants';
import { createJobs } from '../explorer_utils';

export function jobSelectionActionCreator(selectedJobIds: string[]) {
export function jobSelectionActionCreator(
mlFieldFormatService: MlFieldFormatService,
selectedJobIds: string[]
) {
return from(mlFieldFormatService.populateFormats(selectedJobIds)).pipe(
map((resp) => {
if (resp.error) {
Expand Down
Loading

0 comments on commit dda8c8f

Please sign in to comment.