Skip to content

Commit

Permalink
Distinguish error types(source) for actions serverless metrics (elast…
Browse files Browse the repository at this point in the history 
…ic#171416)

Resolves: elastic#168635

This PR intends to add `source` data (user or framework) to the errors
that returned by the actions plugin and use them to create two new
metrics in TaskManager.

New metrics: 
```
USER_ERRORS = 'user_errors'
FRAMEWORK_ERRORS = 'framework_errors'
```

Error source types:
```
  FRAMEWORK = 'framework'
  USER = 'user'
```

I tried to keep the errorSource definition close to the place that the
error is thrown as much as possible.

To check the types / numbers for these, you can `curl
$KB_URL/api/task_manager/metrics?reset=false` to get the values that our
support infrastructure will be using. To test how well this code works,
you can inject errors (add code) into a connector executor that you use
in an alert action, and then use the endpoint to see how the errors were
counted.

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
@ersin-erdal @kibanamachine
ersin-erdal and kibanamachine authored Dec 20, 2023
1 parent 4c2ea7d commit dc25e89
Show file tree
Hide file tree
Showing 30 changed files with 2,269 additions and 339 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/actions/common/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { LicenseType } from '@kbn/licensing-plugin/common/types';
import { TaskErrorSource } from '@kbn/task-manager-plugin/common';

export {
AlertingConnectorFeatureId,
Expand Down Expand Up @@ -47,6 +48,7 @@ export interface ActionTypeExecutorResult<Data> {
serviceMessage?: string;
data?: Data;
retry?: null | boolean | Date;
errorSource?: TaskErrorSource;
}

export type ActionTypeExecutorRawResult<Data> = ActionTypeExecutorResult<Data> & {
Expand Down
86 changes: 75 additions & 11 deletions x-pack/plugins/actions/server/lib/action_executor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import { securityMock } from '@kbn/security-plugin/server/mocks';
import { finished } from 'stream/promises';
import { PassThrough } from 'stream';
import { SecurityConnectorFeatureId } from '../../common';
import { TaskErrorSource } from '@kbn/task-manager-plugin/common';
import { getErrorSource } from '@kbn/task-manager-plugin/server/task_running';

const actionExecutor = new ActionExecutor({ isESOCanEncrypt: true });
const services = actionsMock.createServices();
Expand Down Expand Up @@ -911,6 +913,53 @@ test('successfully authorize system actions', async () => {
});
});

test('actionType Executor returns status "error" and an error message', async () => {
const actionType: jest.Mocked<ActionType> = {
id: 'test',
name: 'Test',
minimumLicenseRequired: 'basic',
supportedFeatureIds: ['alerting'],
validate: {
config: { schema: schema.any() },
secrets: { schema: schema.any() },
params: { schema: schema.any() },
},
executor: jest.fn().mockReturnValue({
actionId: 'test',
status: 'error',
message: 'test error message',
retry: true,
}),
};
const actionSavedObject = {
id: '1',
type: 'action',
attributes: {
name: '1',
actionTypeId: 'test',
config: {
bar: true,
},
secrets: {
baz: true,
},
isMissingSecrets: false,
},
references: [],
};
encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(actionSavedObject);
actionTypeRegistry.get.mockReturnValueOnce(actionType);
const result = await actionExecutor.execute(executeParams);

expect(result).toEqual({
actionId: 'test',
errorSource: TaskErrorSource.USER,
message: 'test error message',
retry: true,
status: 'error',
});
});

test('Execute of SentinelOne sub-actions require create privilege', async () => {
const actionType: jest.Mocked<ActionType> = {
id: '.sentinelone',
Expand Down Expand Up @@ -1130,6 +1179,7 @@ test('throws an error when config is invalid', async () => {
status: 'error',
retry: false,
message: `error validating action type config: [param1]: expected value of type [string] but got [undefined]`,
errorSource: TaskErrorSource.FRAMEWORK,
});
});

Expand Down Expand Up @@ -1169,6 +1219,7 @@ test('returns an error when connector is invalid', async () => {
status: 'error',
retry: false,
message: `error validating action type connector: config must be defined`,
errorSource: TaskErrorSource.FRAMEWORK,
});
});

Expand Down Expand Up @@ -1207,16 +1258,21 @@ test('throws an error when params is invalid', async () => {
status: 'error',
retry: false,
message: `error validating action params: [param1]: expected value of type [string] but got [undefined]`,
errorSource: TaskErrorSource.FRAMEWORK,
});
});

test('throws an error when failing to load action through savedObjectsClient', async () => {
encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockRejectedValueOnce(
new Error('No access')
);
await expect(actionExecutor.execute(executeParams)).rejects.toThrowErrorMatchingInlineSnapshot(
`"No access"`
);

try {
await actionExecutor.execute(executeParams);
} catch (e) {
expect(e.message).toBe('No access');
expect(getErrorSource(e)).toBe(TaskErrorSource.FRAMEWORK);
}
});

test('throws an error if actionType is not enabled', async () => {
Expand Down Expand Up @@ -1246,9 +1302,13 @@ test('throws an error if actionType is not enabled', async () => {
actionTypeRegistry.ensureActionTypeEnabled.mockImplementationOnce(() => {
throw new Error('not enabled for test');
});
await expect(actionExecutor.execute(executeParams)).rejects.toThrowErrorMatchingInlineSnapshot(
`"not enabled for test"`
);

try {
await actionExecutor.execute(executeParams);
} catch (e) {
expect(e.message).toBe('not enabled for test');
expect(getErrorSource(e)).toBe(TaskErrorSource.FRAMEWORK);
}

expect(actionTypeRegistry.ensureActionTypeEnabled).toHaveBeenCalledWith('test');
});
Expand Down Expand Up @@ -1364,11 +1424,15 @@ test('throws an error when passing isESOCanEncrypt with value of false', async (
inMemoryConnectors: [],
getActionsAuthorizationWithRequest,
});
await expect(
customActionExecutor.execute(executeParams)
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Unable to execute action because the Encrypted Saved Objects plugin is missing encryption key. Please set xpack.encryptedSavedObjects.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command."`
);

try {
await customActionExecutor.execute(executeParams);
} catch (e) {
expect(e.message).toBe(
'Unable to execute action because the Encrypted Saved Objects plugin is missing encryption key. Please set xpack.encryptedSavedObjects.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.'
);
expect(getErrorSource(e)).toBe(TaskErrorSource.USER);
}
});

test('should not throw error if action is preconfigured and isESOCanEncrypt is false', async () => {
Expand Down
79 changes: 47 additions & 32 deletions x-pack/plugins/actions/server/lib/action_executor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,33 @@
*/

import type { PublicMethodsOf } from '@kbn/utility-types';
import { Logger, KibanaRequest } from '@kbn/core/server';
import { KibanaRequest, Logger } from '@kbn/core/server';
import { cloneDeep } from 'lodash';
import { set } from '@kbn/safer-lodash-set';
import { withSpan } from '@kbn/apm-utils';
import { EncryptedSavedObjectsClient } from '@kbn/encrypted-saved-objects-plugin/server';
import { SpacesServiceStart } from '@kbn/spaces-plugin/server';
import { IEventLogger, SAVED_OBJECT_REL_PRIMARY } from '@kbn/event-log-plugin/server';
import { SecurityPluginStart } from '@kbn/security-plugin/server';
import { createTaskRunError, TaskErrorSource } from '@kbn/task-manager-plugin/server';
import { getGenAiTokenTracking, shouldTrackGenAiToken } from './gen_ai_token_tracking';
import {
validateParams,
validateConfig,
validateSecrets,
validateConnector,
validateParams,
validateSecrets,
} from './validate_with_schema';
import {
ActionType,
ActionTypeExecutorResult,
ActionTypeConfig,
ActionTypeExecutorRawResult,
ActionTypeExecutorResult,
ActionTypeRegistryContract,
ActionTypeSecrets,
GetServicesFunction,
InMemoryConnector,
RawAction,
ValidatorServices,
ActionTypeSecrets,
ActionTypeConfig,
} from '../types';
import { EVENT_LOG_ACTIONS } from '../constants/event_log';
import { ActionExecutionSource } from './action_execution_source';
Expand Down Expand Up @@ -147,7 +148,11 @@ export class ActionExecutor {
}

if (!actionTypeRegistry.isActionExecutable(actionId, actionTypeId, { notifyUsage: true })) {
actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
try {
actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
} catch (e) {
throw createTaskRunError(e, TaskErrorSource.FRAMEWORK);
}
}
const actionType = actionTypeRegistry.get(actionTypeId);
const configurationUtilities = actionTypeRegistry.getUtils();
Expand Down Expand Up @@ -261,11 +266,12 @@ export class ActionExecutor {
source,
...(actionType.isSystemActionType ? { request } : {}),
});

if (rawResult && rawResult.status === 'error') {
rawResult.errorSource = TaskErrorSource.USER;
}
} catch (err) {
if (
err.reason === ActionExecutionErrorReason.Validation ||
err.reason === ActionExecutionErrorReason.Authorization
) {
if (err.reason === ActionExecutionErrorReason.Authorization) {
rawResult = err.result;
} else {
rawResult = {
Expand All @@ -275,6 +281,7 @@ export class ActionExecutor {
serviceMessage: err.message,
error: err,
retry: true,
errorSource: TaskErrorSource.USER,
};
}
}
Expand Down Expand Up @@ -451,31 +458,37 @@ export class ActionExecutor {
}

if (!this.isESOCanEncrypt) {
throw new Error(
`Unable to execute action because the Encrypted Saved Objects plugin is missing encryption key. Please set xpack.encryptedSavedObjects.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.`
throw createTaskRunError(
new Error(
`Unable to execute action because the Encrypted Saved Objects plugin is missing encryption key. Please set xpack.encryptedSavedObjects.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.`
),
TaskErrorSource.USER
);
}

const rawAction = await encryptedSavedObjectsClient.getDecryptedAsInternalUser<RawAction>(
'action',
actionId,
{
namespace: namespace === 'default' ? undefined : namespace,
}
);

const {
attributes: { secrets, actionTypeId, config, name },
} = rawAction;
try {
const rawAction = await encryptedSavedObjectsClient.getDecryptedAsInternalUser<RawAction>(
'action',
actionId,
{
namespace: namespace === 'default' ? undefined : namespace,
}
);
const {
attributes: { secrets, actionTypeId, config, name },
} = rawAction;

return {
actionTypeId,
name,
config,
secrets,
actionId,
rawAction: rawAction.attributes,
};
return {
actionTypeId,
name,
config,
secrets,
actionId,
rawAction: rawAction.attributes,
};
} catch (e) {
throw createTaskRunError(e, TaskErrorSource.FRAMEWORK);
}
}
}

Expand Down Expand Up @@ -540,6 +553,7 @@ function validateAction(
status: 'error',
message: err.message,
retry: !!taskInfo,
errorSource: TaskErrorSource.FRAMEWORK,
});
}
}
Expand Down Expand Up @@ -585,6 +599,7 @@ const ensureAuthorizedToExecute = async ({
status: 'error',
message: error.message,
retry: false,
errorSource: TaskErrorSource.USER,
});
}
};
Loading

0 comments on commit dc25e89

Please sign in to comment.