Skip to content

Commit

Permalink
[8.x] Add tags to connector run failures indicating if it's user…
Browse files Browse the repository at this point in the history
… or framework error (elastic#197818) (elastic#198006)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Add tags to connector run failures indicating if it's user or
framework error
(elastic#197818)](elastic#197818)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Mike
Côté","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-28T14:21:25Z","message":"Add
tags to connector run failures indicating if it's user or framework
error (elastic#197818)\n\nResolves
https://github.com/elastic/kibana/issues/197315\r\n\r\nIn this PR, I'm
adding the following tags to the connector failure logs\r\nso it makes
it easier to filter for systematic errors.\r\n\r\n-
`connector-run-failed` for logs specific to connector run failures\r\n-
`user-error` for errors caused by the user\r\n- `framework-error` for
systematic errors\r\n\r\n## To verify\r\n\r\nYou can either use the jest
test to observe the returned flags or set\r\nyour logging to JSON and
make connectors fail.\r\n\r\nkibana.yml to set logging to
JSON\r\n```\r\nlogging:\r\n appenders:\r\n json-layout:\r\n type:
console\r\n layout:\r\n type: json\r\n root:\r\n appenders:
[json-layout]\r\n```","sha":"e6bb35ac3d2376b616df4e687517d8ae650dca50","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:Actions","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.17.0"],"title":"Add
tags to connector run failures indicating if it's user or framework
error","number":197818,"url":"https://github.com/elastic/kibana/pull/197818","mergeCommit":{"message":"Add
tags to connector run failures indicating if it's user or framework
error (elastic#197818)\n\nResolves
https://github.com/elastic/kibana/issues/197315\r\n\r\nIn this PR, I'm
adding the following tags to the connector failure logs\r\nso it makes
it easier to filter for systematic errors.\r\n\r\n-
`connector-run-failed` for logs specific to connector run failures\r\n-
`user-error` for errors caused by the user\r\n- `framework-error` for
systematic errors\r\n\r\n## To verify\r\n\r\nYou can either use the jest
test to observe the returned flags or set\r\nyour logging to JSON and
make connectors fail.\r\n\r\nkibana.yml to set logging to
JSON\r\n```\r\nlogging:\r\n appenders:\r\n json-layout:\r\n type:
console\r\n layout:\r\n type: json\r\n root:\r\n appenders:
[json-layout]\r\n```","sha":"e6bb35ac3d2376b616df4e687517d8ae650dca50"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197818","number":197818,"mergeCommit":{"message":"Add
tags to connector run failures indicating if it's user or framework
error (elastic#197818)\n\nResolves
https://github.com/elastic/kibana/issues/197315\r\n\r\nIn this PR, I'm
adding the following tags to the connector failure logs\r\nso it makes
it easier to filter for systematic errors.\r\n\r\n-
`connector-run-failed` for logs specific to connector run failures\r\n-
`user-error` for errors caused by the user\r\n- `framework-error` for
systematic errors\r\n\r\n## To verify\r\n\r\nYou can either use the jest
test to observe the returned flags or set\r\nyour logging to JSON and
make connectors fail.\r\n\r\nkibana.yml to set logging to
JSON\r\n```\r\nlogging:\r\n appenders:\r\n json-layout:\r\n type:
console\r\n layout:\r\n type: json\r\n root:\r\n appenders:
[json-layout]\r\n```","sha":"e6bb35ac3d2376b616df4e687517d8ae650dca50"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Mike Côté <[email protected]>
  • Loading branch information
kibanamachine and mikecote authored Oct 28, 2024
1 parent 15aa7b4 commit 3b31c5b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 12 deletions.
15 changes: 12 additions & 3 deletions x-pack/plugins/actions/server/lib/task_runner_factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,8 @@ describe('Task Runner Factory', () => {
expect(err).toBeDefined();
expect(isRetryableError(err)).toEqual(false);
expect(taskRunnerFactoryInitializerParams.logger.error as jest.Mock).toHaveBeenCalledWith(
`Action '2' failed: Error message`
`Action '2' failed: Error message`,
{ tags: ['connector-run-failed', 'framework-error'] }
);
expect(getErrorSource(err)).toBe(TaskErrorSource.FRAMEWORK);
});
Expand Down Expand Up @@ -934,7 +935,8 @@ describe('Task Runner Factory', () => {

expect(err).toBeDefined();
expect(taskRunnerFactoryInitializerParams.logger.error as jest.Mock).toHaveBeenCalledWith(
`Action '2' failed: Error message: Service message`
`Action '2' failed: Error message: Service message`,
{ tags: ['connector-run-failed', 'framework-error'] }
);
});

Expand Down Expand Up @@ -1033,7 +1035,8 @@ describe('Task Runner Factory', () => {
}
expect(err).toBeDefined();
expect(taskRunnerFactoryInitializerParams.logger.error as jest.Mock).toHaveBeenCalledWith(
`Action '2' failed: Fail`
`Action '2' failed: Fail`,
{ tags: ['connector-run-failed', 'framework-error'] }
);
expect(thrownError).toEqual(err);
expect(getErrorSource(err)).toBe(TaskErrorSource.FRAMEWORK);
Expand Down Expand Up @@ -1140,10 +1143,16 @@ describe('Task Runner Factory', () => {

try {
await taskRunner.run();
throw new Error('Should have thrown');
} catch (e) {
expect(mockedEncryptedSavedObjectsClient.getDecryptedAsInternalUser).toHaveBeenCalledTimes(1);
expect(getErrorSource(e)).toBe(TaskErrorSource.FRAMEWORK);
expect(e).toEqual(error);

expect(taskRunnerFactoryInitializerParams.logger.error).toHaveBeenCalledWith(
`Failed to load action task params ${mockedTaskInstance.params.actionTaskParamsId}: test`,
{ tags: ['connector-run-failed', 'framework-error'] }
);
}
});
});
36 changes: 27 additions & 9 deletions x-pack/plugins/actions/server/lib/task_runner_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ export class TaskRunnerFactory {
} = await getActionTaskParams(
actionTaskExecutorParams,
encryptedSavedObjectsClient,
spaceIdToNamespace
spaceIdToNamespace,
logger
);

const { spaceId } = actionTaskExecutorParams;
Expand All @@ -139,12 +140,18 @@ export class TaskRunnerFactory {
...getSource(references, source),
});
} catch (e) {
logger.error(`Action '${actionId}' failed: ${e.message}`);
const errorSource =
e instanceof ActionTypeDisabledError
? TaskErrorSource.USER
: getErrorSource(e) || TaskErrorSource.FRAMEWORK;
logger.error(`Action '${actionId}' failed: ${e.message}`, {
tags: ['connector-run-failed', `${errorSource}-error`],
});
if (e instanceof ActionTypeDisabledError) {
// We'll stop re-trying due to action being forbidden
throwUnrecoverableError(createTaskRunError(e, TaskErrorSource.USER));
throwUnrecoverableError(createTaskRunError(e, errorSource));
}
throw createTaskRunError(e, getErrorSource(e) || TaskErrorSource.FRAMEWORK);
throw createTaskRunError(e, errorSource);
}

inMemoryMetrics.increment(IN_MEMORY_METRICS.ACTION_EXECUTIONS);
Expand All @@ -155,7 +162,9 @@ export class TaskRunnerFactory {
if (executorResult.serviceMessage) {
message = `${message}: ${executorResult.serviceMessage}`;
}
logger.error(`Action '${actionId}' failed: ${message}`);
logger.error(`Action '${actionId}' failed: ${message}`, {
tags: ['connector-run-failed', `${executorResult.errorSource}-error`],
});

// Task manager error handler only kicks in when an error thrown (at this time)
// So what we have to do is throw when the return status is `error`.
Expand All @@ -175,7 +184,8 @@ export class TaskRunnerFactory {
} = await getActionTaskParams(
actionTaskExecutorParams,
encryptedSavedObjectsClient,
spaceIdToNamespace
spaceIdToNamespace,
logger
);

const request = getFakeRequest(apiKey);
Expand Down Expand Up @@ -239,7 +249,8 @@ function getFakeRequest(apiKey?: string) {
async function getActionTaskParams(
executorParams: ActionTaskExecutorParams,
encryptedSavedObjectsClient: EncryptedSavedObjectsClient,
spaceIdToNamespace: SpaceIdToNamespaceFunction
spaceIdToNamespace: SpaceIdToNamespaceFunction,
logger: Logger
): Promise<TaskParams> {
const { spaceId } = executorParams;
const namespace = spaceIdToNamespace(spaceId);
Expand Down Expand Up @@ -268,10 +279,17 @@ async function getActionTaskParams(
},
};
} catch (e) {
const errorSource = SavedObjectsErrorHelpers.isNotFoundError(e)
? TaskErrorSource.USER
: TaskErrorSource.FRAMEWORK;
logger.error(
`Failed to load action task params ${executorParams.actionTaskParamsId}: ${e.message}`,
{ tags: ['connector-run-failed', `${errorSource}-error`] }
);
if (SavedObjectsErrorHelpers.isNotFoundError(e)) {
throw createRetryableError(createTaskRunError(e, TaskErrorSource.USER), true);
throw createRetryableError(createTaskRunError(e, errorSource), true);
}
throw createRetryableError(createTaskRunError(e, TaskErrorSource.FRAMEWORK), true);
throw createRetryableError(createTaskRunError(e, errorSource), true);
}
} else {
return { attributes: executorParams.taskParams, references: executorParams.references ?? [] };
Expand Down

0 comments on commit 3b31c5b

Please sign in to comment.