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

[Enterprise Search] Add telemetry to ELSER deployment buttons + error #156545

Merged
merged 5 commits into from
May 4, 2023
Merged

[Enterprise Search] Add telemetry to ELSER deployment buttons + error #156545

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0bcde83
Add telemetry to ELSER deployment buttons + error
demjened May 3, 2023
2a4edd3
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine May 3, 2023
c6d48d2
Fix missing type
demjened May 3, 2023
9372887
Merge branch 'demjened/elser-telemetry' of https://github.com/demjene…
demjened May 3, 2023
54034a5
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine May 3, 2023
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
56 changes: 48 additions & 8 deletions ...ch_content/components/search_index/pipelines/ml_inference/text_expansion_callout.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,29 +100,49 @@ describe('TextExpansionCallOut', () => {
describe('DeployModel', () => {
it('renders deploy button', () => {
const wrapper = shallow(
<DeployModel dismiss={() => {}} isCreateButtonDisabled={false} isDismissable={false} />
<DeployModel
dismiss={() => {}}
ingestionMethod="crawler"
isCreateButtonDisabled={false}
isDismissable={false}
/>
);
expect(wrapper.find(EuiButton).length).toBe(1);
const button = wrapper.find(EuiButton);
expect(button.prop('disabled')).toBe(false);
});
it('renders disabled deploy button if it is set to disabled', () => {
const wrapper = shallow(
<DeployModel dismiss={() => {}} isCreateButtonDisabled isDismissable={false} />
<DeployModel
dismiss={() => {}}
ingestionMethod="crawler"
isCreateButtonDisabled
isDismissable={false}
/>
);
expect(wrapper.find(EuiButton).length).toBe(1);
const button = wrapper.find(EuiButton);
expect(button.prop('disabled')).toBe(true);
});
it('renders dismiss button if it is set to dismissable', () => {
const wrapper = shallow(
<DeployModel dismiss={() => {}} isCreateButtonDisabled={false} isDismissable />
<DeployModel
dismiss={() => {}}
ingestionMethod="crawler"
isCreateButtonDisabled={false}
isDismissable
/>
);
expect(wrapper.find(TextExpansionDismissButton).length).toBe(1);
});
it('does not render dismiss button if it is set to non-dismissable', () => {
const wrapper = shallow(
<DeployModel dismiss={() => {}} isCreateButtonDisabled={false} isDismissable={false} />
<DeployModel
dismiss={() => {}}
ingestionMethod="crawler"
isCreateButtonDisabled={false}
isDismissable={false}
/>
);
expect(wrapper.find(TextExpansionDismissButton).length).toBe(0);
});
Expand All @@ -144,29 +164,49 @@ describe('TextExpansionCallOut', () => {
describe('ModelDeployed', () => {
it('renders start button', () => {
const wrapper = shallow(
<ModelDeployed dismiss={() => {}} isDismissable={false} isStartButtonDisabled={false} />
<ModelDeployed
dismiss={() => {}}
ingestionMethod="crawler"
isDismissable={false}
isStartButtonDisabled={false}
/>
);
expect(wrapper.find(EuiButton).length).toBe(1);
const button = wrapper.find(EuiButton);
expect(button.prop('disabled')).toBe(false);
});
it('renders disabled start button if it is set to disabled', () => {
const wrapper = shallow(
<ModelDeployed dismiss={() => {}} isDismissable={false} isStartButtonDisabled />
<ModelDeployed
dismiss={() => {}}
ingestionMethod="crawler"
isDismissable={false}
isStartButtonDisabled
/>
);
expect(wrapper.find(EuiButton).length).toBe(1);
const button = wrapper.find(EuiButton);
expect(button.prop('disabled')).toBe(true);
});
it('renders dismiss button if it is set to dismissable', () => {
const wrapper = shallow(
<ModelDeployed dismiss={() => {}} isDismissable isStartButtonDisabled={false} />
<ModelDeployed
dismiss={() => {}}
ingestionMethod="crawler"
isDismissable
isStartButtonDisabled={false}
/>
);
expect(wrapper.find(TextExpansionDismissButton).length).toBe(1);
});
it('does not render dismiss button if it is set to non-dismissable', () => {
const wrapper = shallow(
<ModelDeployed dismiss={() => {}} isDismissable={false} isStartButtonDisabled={false} />
<ModelDeployed
dismiss={() => {}}
ingestionMethod="crawler"
isDismissable={false}
isStartButtonDisabled={false}
/>
);
expect(wrapper.find(TextExpansionDismissButton).length).toBe(0);
});
Expand Down
13 changes: 11 additions & 2 deletions ..._search_content/components/search_index/pipelines/ml_inference/text_expansion_callout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ import { KibanaLogic } from '../../../../../shared/kibana';
import { useTextExpansionCallOutData } from './text_expansion_callout_data';
import { getTextExpansionError, TextExpansionCalloutLogic } from './text_expansion_callout_logic';
import { TextExpansionErrors } from './text_expansion_errors';
import { IndexViewLogic } from '../../index_view_logic';

export interface TextExpansionCallOutState {
dismiss: () => void;
ingestionMethod: string;
isCreateButtonDisabled: boolean;
isDismissable: boolean;
isStartButtonDisabled: boolean;
Expand Down Expand Up @@ -61,9 +63,10 @@ export const TextExpansionDismissButton = ({

export const DeployModel = ({
dismiss,
ingestionMethod,
isCreateButtonDisabled,
isDismissable,
}: Pick<TextExpansionCallOutState, 'dismiss' | 'isCreateButtonDisabled' | 'isDismissable'>) => {
}: Pick<TextExpansionCallOutState, 'dismiss' | 'ingestionMethod' | 'isCreateButtonDisabled' | 'isDismissable'>) => {
const { createTextExpansionModel } = useActions(TextExpansionCalloutLogic);

return (
Expand Down Expand Up @@ -117,6 +120,7 @@ export const DeployModel = ({
<EuiFlexItem grow={false}>
<EuiButton
color="success"
data-telemetry-id={`entSearchContent-${ingestionMethod}-pipelines-textExpansionCallOut-deployModel`}
disabled={isCreateButtonDisabled}
iconType="launch"
onClick={() => createTextExpansionModel(undefined)}
Expand Down Expand Up @@ -193,9 +197,10 @@ export const ModelDeploymentInProgress = ({

export const ModelDeployed = ({
dismiss,
ingestionMethod,
isDismissable,
isStartButtonDisabled,
}: Pick<TextExpansionCallOutState, 'dismiss' | 'isDismissable' | 'isStartButtonDisabled'>) => {
}: Pick<TextExpansionCallOutState, 'dismiss' | 'ingestionMethod' | 'isDismissable' | 'isStartButtonDisabled'>) => {
const { startTextExpansionModel } = useActions(TextExpansionCalloutLogic);

return (
Expand Down Expand Up @@ -249,6 +254,7 @@ export const ModelDeployed = ({
<EuiFlexItem grow={false}>
<EuiButton
color="success"
data-telemetry-id={`entSearchContent-${ingestionMethod}-pipelines-textExpansionCallOut-startModel`}
disabled={isStartButtonDisabled}
iconType="playFilled"
onClick={() => startTextExpansionModel(undefined)}
Expand Down Expand Up @@ -327,6 +333,7 @@ export const ModelStarted = ({

export const TextExpansionCallOut: React.FC<TextExpansionCallOutProps> = (props) => {
const { dismiss, isDismissable, show } = useTextExpansionCallOutData(props);
const { ingestionMethod } = useValues(IndexViewLogic);
Copy link
Contributor Author

@demjened demjened May 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Method of importing ingestionMethod copied from here.

const {
createTextExpansionModelError,
fetchTextExpansionModelError,
Expand Down Expand Up @@ -354,6 +361,7 @@ export const TextExpansionCallOut: React.FC<TextExpansionCallOutProps> = (props)
return (
<ModelDeployed
dismiss={dismiss}
ingestionMethod={ingestionMethod}
isDismissable={isDismissable}
isStartButtonDisabled={isStartButtonDisabled}
/>
Expand All @@ -365,6 +373,7 @@ export const TextExpansionCallOut: React.FC<TextExpansionCallOutProps> = (props)
return (
<DeployModel
dismiss={dismiss}
ingestionMethod={ingestionMethod}
isDismissable={isDismissable}
isCreateButtonDisabled={isCreateButtonDisabled}
/>
Expand Down
2 changes: 2 additions & 0 deletions ...e_search_content/components/search_index/pipelines/ml_inference/text_expansion_errors.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ import { i18n } from '@kbn/i18n';
import { HttpLogic } from '../../../../../shared/http';

import { ML_NOTIFICATIONS_PATH } from '../../../../routes';
import { SendEnterpriseSearchTelemetry } from '../../../../../shared/telemetry';

export const TextExpansionErrors = ({ error }: { error: { title: string; message: string } }) => {
const { http } = useValues(HttpLogic);

return (
<>
<SendEnterpriseSearchTelemetry action="error" metric="textExpansionModel-error" />
Copy link
Contributor Author

@demjened demjened May 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Per convo with @TattdCodeMonkey I think this is the correct way to send metrics when an error pops up, but let me know if it isn't.
  2. Not sure about the metric name, the only references I found use short and generic metric names.

<EuiCallOut color="danger" iconType="error" title={error.title}>
<p>{error.message}</p>
<EuiLink href={http.basePath.prepend(ML_NOTIFICATIONS_PATH)} target="_blank">
Expand Down