Skip to content

Commit

Permalink
Add error handling to logging methods of Rule Execution Log
Browse files Browse the repository at this point in the history
  • Loading branch information
banderror committed Dec 1, 2021
1 parent 79079a7 commit bd24153
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,32 @@ export class EventLogAdapter implements IRuleExecutionLogClient {
// EventLog execution events are immutable, nothing to do here
}

private async logExecutionMetrics(args: LogExecutionMetricsArgs) {
public async logStatusChange(args: LogStatusChangeArgs): Promise<void> {
await Promise.all([
this.logStatusChangeToSavedObjects(args),
this.logStatusChangeToEventLog(args),
]);
}

private async logStatusChangeToSavedObjects(args: LogStatusChangeArgs): Promise<void> {
await this.savedObjectsAdapter.logStatusChange(args);
}

private async logStatusChangeToEventLog(args: LogStatusChangeArgs): Promise<void> {
if (args.metrics) {
this.logExecutionMetrics({
ruleId: args.ruleId,
ruleName: args.ruleName,
ruleType: args.ruleType,
spaceId: args.spaceId,
metrics: args.metrics,
});
}

this.eventLogClient.logStatusChange(args);
}

private logExecutionMetrics(args: LogExecutionMetricsArgs): void {
const { ruleId, spaceId, ruleType, ruleName, metrics } = args;

this.eventLogClient.logExecutionMetrics({
Expand All @@ -98,20 +123,4 @@ export class EventLogAdapter implements IRuleExecutionLogClient {
},
});
}

public async logStatusChange(args: LogStatusChangeArgs) {
await this.savedObjectsAdapter.logStatusChange(args);

if (args.metrics) {
await this.logExecutionMetrics({
ruleId: args.ruleId,
ruleName: args.ruleName,
ruleType: args.ruleType,
spaceId: args.spaceId,
metrics: args.metrics,
});
}

this.eventLogClient.logStatusChange(args);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { SavedObjectsClientContract } from '../../../../../../../src/core/server';
import { Logger, SavedObjectsClientContract } from 'src/core/server';
import { IEventLogClient, IEventLogService } from '../../../../../event_log/server';
import { IRuleStatusSOAttributes } from '../rules/types';
import { EventLogAdapter } from './event_log_adapter/event_log_adapter';
Expand All @@ -20,6 +20,7 @@ import {
GetCurrentStatusArgs,
GetCurrentStatusBulkArgs,
GetCurrentStatusBulkResult,
ExtMeta,
} from './types';
import { truncateMessage } from './utils/normalization';

Expand All @@ -28,13 +29,16 @@ interface ConstructorParams {
savedObjectsClient: SavedObjectsClientContract;
eventLogService: IEventLogService;
eventLogClient?: IEventLogClient;
logger: Logger;
}

export class RuleExecutionLogClient implements IRuleExecutionLogClient {
private client: IRuleExecutionLogClient;
private readonly client: IRuleExecutionLogClient;
private readonly logger: Logger;

constructor(params: ConstructorParams) {
const { underlyingClient, eventLogService, eventLogClient, savedObjectsClient } = params;
const { underlyingClient, eventLogService, eventLogClient, savedObjectsClient, logger } =
params;

switch (underlyingClient) {
case UnderlyingLogClient.savedObjects:
Expand All @@ -44,6 +48,10 @@ export class RuleExecutionLogClient implements IRuleExecutionLogClient {
this.client = new EventLogAdapter(eventLogService, eventLogClient, savedObjectsClient);
break;
}

// We write rule execution logs via a child console logger with the context
// "plugins.securitySolution.ruleExecution"
this.logger = logger.get('ruleExecution');
}

/** @deprecated */
Expand Down Expand Up @@ -74,11 +82,34 @@ export class RuleExecutionLogClient implements IRuleExecutionLogClient {
return this.client.deleteCurrentStatus(ruleId);
}

public async logStatusChange(args: LogStatusChangeArgs) {
const message = args.message ? truncateMessage(args.message) : args.message;
return this.client.logStatusChange({
...args,
message,
});
public async logStatusChange(args: LogStatusChangeArgs): Promise<void> {
const { newStatus, message, ruleId, ruleName, ruleType, spaceId } = args;

try {
const truncatedMessage = message ? truncateMessage(message) : message;
await this.client.logStatusChange({
...args,
message: truncatedMessage,
});
} catch (e) {
const logMessage = 'Error logging rule execution status change';
const logAttributes = `status: "${newStatus}", rule id: "${ruleId}", rule name: "${ruleName}"`;
const logReason = e instanceof Error ? `${e.stack}` : `${e}`;
const logMeta: ExtMeta = {
rule: {
id: ruleId,
name: ruleName,
type: ruleType,
execution: {
status: newStatus,
},
},
kibana: {
spaceId,
},
};

this.logger.error<ExtMeta>(`${logMessage}; ${logAttributes}; ${logReason}`, logMeta);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { Duration } from 'moment';
import { SavedObjectsFindResult } from '../../../../../../../src/core/server';
import { LogMeta, SavedObjectsFindResult } from 'src/core/server';
import { RuleExecutionStatus } from '../../../../common/detection_engine/schemas/common/schemas';
import { IRuleStatusSOAttributes } from '../rules/types';

Expand Down Expand Up @@ -103,3 +103,18 @@ export interface ExecutionMetrics {
lastLookBackDate?: string;
executionGap?: Duration;
}

/**
* Custom extended log metadata that rule execution logger can attach to every log record.
*/
export type ExtMeta = LogMeta & {
rule?: LogMeta['rule'] & {
type?: string;
execution?: {
status?: RuleExecutionStatus;
};
};
kibana?: {
spaceId?: string;
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper =
underlyingClient: config.ruleExecutionLog.underlyingClient,
savedObjectsClient,
eventLogService,
logger,
});

const completeRule = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export const signalRulesAlertType = ({
underlyingClient: config.ruleExecutionLog.underlyingClient,
savedObjectsClient: services.savedObjectsClient,
eventLogService,
logger,
});

const completeRule: CompleteRule<RuleParams> = {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export class Plugin implements ISecuritySolutionPlugin {
const eventLogService = plugins.eventLog;
registerEventLogProvider(eventLogService);

const requestContextFactory = new RequestContextFactory({ config, core, plugins });
const requestContextFactory = new RequestContextFactory({ config, logger, core, plugins });
const router = core.http.createRouter<SecuritySolutionRequestHandlerContext>();
core.http.registerRouteHandlerContext<SecuritySolutionRequestHandlerContext, typeof APP_ID>(
APP_ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { KibanaRequest, RequestHandlerContext } from 'kibana/server';
import { Logger, KibanaRequest, RequestHandlerContext } from 'kibana/server';
import { ExceptionListClient } from '../../lists/server';

import { DEFAULT_SPACE_ID } from '../common/constants';
Expand All @@ -28,6 +28,7 @@ export interface IRequestContextFactory {

interface ConstructorOptions {
config: ConfigType;
logger: Logger;
core: SecuritySolutionPluginCoreSetupDependencies;
plugins: SecuritySolutionPluginSetupDependencies;
}
Expand All @@ -44,7 +45,7 @@ export class RequestContextFactory implements IRequestContextFactory {
request: KibanaRequest
): Promise<SecuritySolutionApiRequestHandlerContext> {
const { options, appClientFactory } = this;
const { config, core, plugins } = options;
const { config, logger, core, plugins } = options;
const { lists, ruleRegistry, security } = plugins;

const [, startPlugins] = await core.getStartServices();
Expand Down Expand Up @@ -73,6 +74,7 @@ export class RequestContextFactory implements IRequestContextFactory {
savedObjectsClient: context.core.savedObjects.client,
eventLogService: plugins.eventLog,
eventLogClient: startPlugins.eventLog.getClient(request),
logger,
}),

getExceptionListClient: () => {
Expand Down

0 comments on commit bd24153

Please sign in to comment.