Skip to content

Commit

Permalink
Revert override alert timestamp (elastic#189724)
Browse files Browse the repository at this point in the history
## Revert override alert timestamp

Previously we added override of alert timestamp for manual rule runs.
Later was decided, that timestamp for manual rule run should behave the
same as regular alert and represent time when alert generated.

---------

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
nkhristinin and elasticmachine authored Aug 13, 2024
1 parent 94cec41 commit 6589cd3
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -249,13 +249,7 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
...options,
services: {
...options.services,
alertWithPersistence: async (
alerts,
refresh,
maxAlerts = undefined,
enrichAlerts,
currentTimeOverride
) => {
alertWithPersistence: async (alerts, refresh, maxAlerts = undefined, enrichAlerts) => {
const numAlerts = alerts.length;
logger.debug(`Found ${numAlerts} alerts.`);

Expand Down Expand Up @@ -307,7 +301,7 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
alerts: enrichedAlerts,
options,
kibanaVersion: ruleDataClient.kibanaVersion,
currentTimeOverride,
currentTimeOverride: undefined,
});

const response = await ruleDataClientWriter.bulk({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ export type PersistenceAlertService = <T>(
_id: string;
_source: T;
}>
>,
currentTimeOverride?: Date
>
) => Promise<PersistenceAlertServiceResult<T>>;

export type SuppressedAlertService = <T extends SuppressionFieldsLatest>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper =
params,
previousStartedAt,
startedAt,
startedAtOverridden,
services,
spaceId,
state,
Expand Down Expand Up @@ -366,13 +365,12 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper =
lists: params.exceptionsList,
});

const alertTimestampOverride = isPreview || startedAtOverridden ? startedAt : undefined;
const alertTimestampOverride = isPreview ? startedAt : undefined;
const bulkCreate = bulkCreateFactory(
alertWithPersistence,
refresh,
ruleExecutionLogger,
experimentalFeatures,
alertTimestampOverride
experimentalFeatures
);

const legacySignalFields: string[] = Object.keys(aadFieldConversion);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ export const bulkCreateFactory =
alertWithPersistence: PersistenceAlertService,
refreshForBulkCreate: RefreshTypes,
ruleExecutionLogger: IRuleExecutionLogForExecutors,
experimentalFeatures?: ExperimentalFeatures,
currentTimeOverride?: Date
experimentalFeatures?: ExperimentalFeatures
) =>
async <T extends BaseFieldsLatest>(
wrappedDocs: Array<WrappedFieldsLatest<T>>,
Expand Down Expand Up @@ -87,8 +86,7 @@ export const bulkCreateFactory =
})),
refreshForBulkCreate,
maxAlerts,
enrichAlertsWrapper,
currentTimeOverride
enrichAlertsWrapper
);

const end = performance.now();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,9 @@ export default function createBackfillTaskRunnerTests({ getService }: FtrProvide
// check timestamps in alert docs
for (const alert of alertDocsBackfill1) {
const source = alert._source!;
expect(source[ALERT_START]).to.eql(scheduleResult[0].schedule[0].run_at);
expect(source[ALERT_LAST_DETECTED]).to.eql(scheduleResult[0].schedule[0].run_at);
expect(source[TIMESTAMP]).to.eql(scheduleResult[0].schedule[0].run_at);
expect(source[ALERT_START]).to.match(timestampPattern);
expect(source[ALERT_LAST_DETECTED]).to.match(timestampPattern);
expect(source[TIMESTAMP]).not.to.eql(scheduleResult[0].schedule[0].run_at);
expect(source[ALERT_RULE_EXECUTION_TIMESTAMP]).to.match(timestampPattern);
expect(source[ALERT_RULE_EXECUTION_TIMESTAMP]).not.to.eql(
scheduleResult[0].schedule[0].run_at
Expand All @@ -331,9 +331,9 @@ export default function createBackfillTaskRunnerTests({ getService }: FtrProvide
// check timestamps in alert docs
for (const alert of alertDocsBackfill2) {
const source = alert._source!;
expect(source[ALERT_START]).to.eql(scheduleResult[0].schedule[1].run_at);
expect(source[ALERT_LAST_DETECTED]).to.eql(scheduleResult[0].schedule[1].run_at);
expect(source[TIMESTAMP]).to.eql(scheduleResult[0].schedule[1].run_at);
expect(source[ALERT_START]).to.match(timestampPattern);
expect(source[ALERT_LAST_DETECTED]).to.match(timestampPattern);
expect(source[TIMESTAMP]).not.to.eql(scheduleResult[0].schedule[1].run_at);
expect(source[ALERT_RULE_EXECUTION_TIMESTAMP]).to.match(timestampPattern);
expect(source[ALERT_RULE_EXECUTION_TIMESTAMP]).not.to.eql(
scheduleResult[0].schedule[1].run_at
Expand All @@ -351,9 +351,9 @@ export default function createBackfillTaskRunnerTests({ getService }: FtrProvide
// check timestamps in alert docs
for (const alert of alertDocsBackfill3) {
const source = alert._source!;
expect(source[ALERT_START]).to.eql(scheduleResult[0].schedule[2].run_at);
expect(source[ALERT_LAST_DETECTED]).to.eql(scheduleResult[0].schedule[2].run_at);
expect(source[TIMESTAMP]).to.eql(scheduleResult[0].schedule[2].run_at);
expect(source[ALERT_START]).to.match(timestampPattern);
expect(source[ALERT_LAST_DETECTED]).to.match(timestampPattern);
expect(source[TIMESTAMP]).not.to.eql(scheduleResult[0].schedule[2].run_at);
expect(source[ALERT_RULE_EXECUTION_TIMESTAMP]).to.match(timestampPattern);
expect(source[ALERT_RULE_EXECUTION_TIMESTAMP]).not.to.eql(
scheduleResult[0].schedule[2].run_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,6 @@ export default ({ getService }: FtrProviderContext) => {
expect.objectContaining({
'user.name': ['irrelevant'],
[TIMESTAMP]: timestamp,
[ALERT_START]: timestamp,
})
);

Expand All @@ -635,7 +634,6 @@ export default ({ getService }: FtrProviderContext) => {
expect.objectContaining({
'user.name': ['irrelevant'],
[TIMESTAMP]: timestamp,
[ALERT_START]: timestamp,
})
);
expect(previewAlerts[1]._source).toEqual(
Expand All @@ -657,7 +655,6 @@ export default ({ getService }: FtrProviderContext) => {
},
],
[TIMESTAMP]: timestamp,
[ALERT_START]: timestamp,
[ALERT_ORIGINAL_TIME]: timestamp,
[ALERT_SUPPRESSION_START]: timestamp,
[ALERT_SUPPRESSION_END]: timestamp,
Expand Down

0 comments on commit 6589cd3

Please sign in to comment.