Skip to content

Commit

Permalink
Add tags to connector run failures indicating if it's user or framewo…
Browse files Browse the repository at this point in the history
…rk error (#197818)

Resolves #197315

In this PR, I'm adding the following tags to the connector failure logs
so it makes it easier to filter for systematic errors.

- `connector-run-failed` for logs specific to connector run failures
- `user-error` for errors caused by the user
- `framework-error` for systematic errors

## To verify

You can either use the jest test to observe the returned flags or set
your logging to JSON and make connectors fail.

kibana.yml to set logging to JSON
```
logging:
  appenders:
    json-layout:
      type: console
      layout:
        type: json
  root:
    appenders: [json-layout]
```
  • Loading branch information
mikecote authored Oct 28, 2024
1 parent a168458 commit e6bb35a
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 e6bb35a

Please sign in to comment.