-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AO] - Add scaffolding and the main chart to the Logs threshold Alert Details page #153081
Changes from all commits
9e7fdae
a4e2754
00ed7c1
a70ba16
553eb96
9ced201
c13fdf0
f402c01
06a0bd4
2d9831e
62d24bb
d39c7bb
3cca372
138972a
a065a5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; | ||
import { ALERT_DURATION, ALERT_END } from '@kbn/rule-data-utils'; | ||
import moment from 'moment'; | ||
import React from 'react'; | ||
import { type PartialCriterion } from '../../../../../common/alerting/logs/log_threshold'; | ||
import { CriterionPreview } from '../expression_editor/criterion_preview_chart'; | ||
import { AlertDetailsAppSectionProps } from './types'; | ||
|
||
const AlertDetailsAppSection = ({ rule, alert }: AlertDetailsAppSectionProps) => { | ||
const ruleWindowSizeMS = moment | ||
.duration(rule.params.timeSize, rule.params.timeUnit) | ||
.asMilliseconds(); | ||
const alertDurationMS = alert.fields[ALERT_DURATION]! / 1000; | ||
const TWENTY_TIMES_RULE_WINDOW_MS = 20 * ruleWindowSizeMS; | ||
/** | ||
* This is part or the requirements (RFC). | ||
* If the alert is less than 20 units of `FOR THE LAST <x> <units>` then we should draw a time range of 20 units. | ||
* IE. The user set "FOR THE LAST 5 minutes" at a minimum we should show 100 minutes. | ||
*/ | ||
const rangeFrom = | ||
alertDurationMS < TWENTY_TIMES_RULE_WINDOW_MS | ||
? Number(moment(alert.start).subtract(TWENTY_TIMES_RULE_WINDOW_MS, 'millisecond').format('x')) | ||
: Number(moment(alert.start).subtract(ruleWindowSizeMS, 'millisecond').format('x')); | ||
|
||
const rangeTo = alert.active | ||
? Date.now() | ||
: Number(moment(alert.fields[ALERT_END]).add(ruleWindowSizeMS, 'millisecond').format('x')); | ||
|
||
return ( | ||
// Create a chart per-criteria | ||
<EuiFlexGroup> | ||
{rule.params.criteria.map((criteria) => { | ||
fkanout marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const chartCriterion = criteria as PartialCriterion; | ||
return ( | ||
<EuiFlexItem> | ||
<CriterionPreview | ||
key={chartCriterion.field} | ||
ruleParams={rule.params} | ||
sourceId={rule.params.logView.logViewId} | ||
chartCriterion={chartCriterion} | ||
showThreshold={true} | ||
executionTimeRange={{ gte: rangeFrom, lte: rangeTo }} | ||
/> | ||
</EuiFlexItem> | ||
); | ||
})} | ||
</EuiFlexGroup> | ||
); | ||
}; | ||
// eslint-disable-next-line import/no-default-export | ||
export default AlertDetailsAppSection; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { Rule } from '@kbn/alerting-plugin/common'; | ||
import { TopAlert } from '@kbn/observability-plugin/public'; | ||
import { PartialRuleParams } from '../../../../../common/alerting/logs/log_threshold'; | ||
|
||
export interface AlertDetailsAppSectionProps { | ||
rule: Rule<PartialRuleParams>; | ||
alert: TopAlert; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,6 +112,11 @@ export interface LensOptions { | |
breakdownSize: number; | ||
} | ||
|
||
export interface ExecutionTimeRange { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this interface needed? There is also one in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. One with |
||
gte: number; | ||
lte: number; | ||
} | ||
|
||
type PropsOf<T> = T extends React.ComponentType<infer ComponentProps> ? ComponentProps : never; | ||
type FirstArgumentOf<Func> = Func extends (arg1: infer FirstArgument, ...rest: any[]) => any | ||
? FirstArgument | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,7 +140,9 @@ const baseRuleParams: Pick<RuleParams, 'count' | 'timeSize' | 'timeUnit' | 'logV | |
|
||
const TIMESTAMP_FIELD = '@timestamp'; | ||
const FILEBEAT_INDEX = 'filebeat-*'; | ||
const EXECUTION_TIMESTAMP = new Date('2022-01-01T00:00:00.000Z').valueOf(); | ||
const EXECUTION_TIMERANGE = { | ||
lte: new Date('2022-01-01T00:00:00.000Z').valueOf(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we might have
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @maryam-saeidi, no need here. The executor doesn't use |
||
}; | ||
|
||
const runtimeMappings: estypes.MappingRuntimeFields = { | ||
runtime_field: { | ||
|
@@ -173,7 +175,7 @@ describe('Log threshold executor', () => { | |
...baseRuleParams, | ||
criteria: positiveCriteria, | ||
}; | ||
const filters = buildFiltersFromCriteria(ruleParams, TIMESTAMP_FIELD, EXECUTION_TIMESTAMP); | ||
const filters = buildFiltersFromCriteria(ruleParams, TIMESTAMP_FIELD, EXECUTION_TIMERANGE); | ||
expect(filters.mustFilters).toEqual(expectedPositiveFilterClauses); | ||
}); | ||
|
||
|
@@ -182,14 +184,14 @@ describe('Log threshold executor', () => { | |
...baseRuleParams, | ||
criteria: negativeCriteria, | ||
}; | ||
const filters = buildFiltersFromCriteria(ruleParams, TIMESTAMP_FIELD, EXECUTION_TIMESTAMP); | ||
const filters = buildFiltersFromCriteria(ruleParams, TIMESTAMP_FIELD, EXECUTION_TIMERANGE); | ||
|
||
expect(filters.mustNotFilters).toEqual(expectedNegativeFilterClauses); | ||
}); | ||
|
||
test('Handles time range', () => { | ||
const ruleParams: RuleParams = { ...baseRuleParams, criteria: [] }; | ||
const filters = buildFiltersFromCriteria(ruleParams, TIMESTAMP_FIELD, EXECUTION_TIMESTAMP); | ||
const filters = buildFiltersFromCriteria(ruleParams, TIMESTAMP_FIELD, EXECUTION_TIMERANGE); | ||
expect(typeof filters.rangeFilter.range[TIMESTAMP_FIELD].gte).toBe('number'); | ||
expect(typeof filters.rangeFilter.range[TIMESTAMP_FIELD].lte).toBe('number'); | ||
expect(filters.rangeFilter.range[TIMESTAMP_FIELD].format).toBe('epoch_millis'); | ||
|
@@ -212,7 +214,7 @@ describe('Log threshold executor', () => { | |
TIMESTAMP_FIELD, | ||
FILEBEAT_INDEX, | ||
runtimeMappings, | ||
EXECUTION_TIMESTAMP | ||
EXECUTION_TIMERANGE | ||
); | ||
expect(query).toEqual({ | ||
index: 'filebeat-*', | ||
|
@@ -264,7 +266,7 @@ describe('Log threshold executor', () => { | |
TIMESTAMP_FIELD, | ||
FILEBEAT_INDEX, | ||
runtimeMappings, | ||
EXECUTION_TIMESTAMP | ||
EXECUTION_TIMERANGE | ||
); | ||
|
||
expect(query).toEqual({ | ||
|
@@ -344,7 +346,7 @@ describe('Log threshold executor', () => { | |
TIMESTAMP_FIELD, | ||
FILEBEAT_INDEX, | ||
runtimeMappings, | ||
EXECUTION_TIMESTAMP | ||
EXECUTION_TIMERANGE | ||
); | ||
|
||
expect(query).toEqual({ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this is not used on the server side, it's used only if we define it for the Logs chart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I see, we use both on the server side: https://github.com/elastic/kibana/pull/153081/files#diff-a71ca536380c1fde8805744b23566ce795707f92b94a03af73347cac46ccac63R654-R655
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maryam-saeidi, only the Alert Details page (frontend) will fill up the
gte
, which is sent to the server side.Something to mention is
ExecutionTimeRange
is not only used to get the chart data (where we could usegte
). But also in the log threshold executor (wheregte
is not used)