Skip to content

Commit

Permalink
Merge branch 'main' into 186156-track-field-usage
Browse files Browse the repository at this point in the history
  • Loading branch information
jughosta authored Sep 30, 2024
2 parents a442993 + 0dada14 commit 73f2bca
Show file tree
Hide file tree
Showing 30 changed files with 278 additions and 244 deletions.
1 change: 0 additions & 1 deletion .buildkite/scripts/steps/artifacts/cloud.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ jq '
.resources.kibana[0].plan.kibana.docker_image = "'$KIBANA_TEST_IMAGE'" |
.resources.kibana[0].plan.kibana.version = "'$FULL_VERSION'" |
.resources.elasticsearch[0].plan.elasticsearch.version = "'$FULL_VERSION'" |
.resources.enterprise_search[0].plan.enterprise_search.version = "'$FULL_VERSION'" |
.resources.integrations_server[0].plan.integrations_server.version = "'$FULL_VERSION'"
' .buildkite/scripts/steps/cloud/deploy.json > "$DEPLOYMENT_SPEC"

Expand Down
1 change: 0 additions & 1 deletion .buildkite/scripts/steps/cloud/build_and_deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ if [ -z "${CLOUD_DEPLOYMENT_ID}" ] || [ "${CLOUD_DEPLOYMENT_ID}" = 'null' ]; the
.name = "'$CLOUD_DEPLOYMENT_NAME'" |
.resources.kibana[0].plan.kibana.version = "'$VERSION'" |
.resources.elasticsearch[0].plan.elasticsearch.version = "'$VERSION'" |
.resources.enterprise_search[0].plan.enterprise_search.version = "'$VERSION'" |
.resources.integrations_server[0].plan.integrations_server.version = "'$VERSION'"
' .buildkite/scripts/steps/cloud/deploy.json > /tmp/deploy.json

Expand Down
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -1239,6 +1239,7 @@ x-pack/test/observability_ai_assistant_functional @elastic/obs-ai-assistant
/x-pack/test/functional/apps/canvas/ @elastic/kibana-presentation
/x-pack/test_serverless/functional/test_suites/search/dashboards/ @elastic/kibana-presentation
/test/plugin_functional/test_suites/panel_actions @elastic/kibana-presentation
/x-pack/test/functional/es_archives/canvas/logstash_lens @elastic/kibana-presentation
#CC# /src/plugins/kibana_react/public/code_editor/ @elastic/kibana-presentation

# Machine Learning
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { description } from '../../common/description';
import { name } from '../../common/name';
import { _version } from '../../common/underscore_version';
import { id } from '../../common/id';
import { item_id } from '../../common/item_id';
import { meta } from '../../common/meta';
import { namespace_type } from '../../common/namespace_type';
import { ExpireTimeOrUndefined, expireTimeOrUndefined } from '../../common';
Expand All @@ -40,7 +41,7 @@ export const updateExceptionListItemSchema = t.intersection([
comments: DefaultUpdateCommentsArray, // defaults to empty array if not set during decode
expire_time: expireTimeOrUndefined,
id, // defaults to undefined if not set during decode
item_id: t.union([t.string, t.undefined]),
item_id,
meta, // defaults to undefined if not set during decode
namespace_type, // defaults to 'single' if not set during decode
os_types: osTypeArrayOrUndefined, // defaults to empty array if not set during decode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { fromKueryExpression } from '@kbn/es-query';
import { KueryNode, fromKueryExpression, toKqlExpression } from '@kbn/es-query';
import { KibanaRequest } from '@kbn/core/server';
import { ruleTypeRegistryMock } from '../rule_type_registry.mock';
import { securityMock } from '@kbn/security-plugin/server/mocks';
Expand Down Expand Up @@ -910,20 +910,19 @@ describe('AlertingAuthorization', () => {
getSpaceId,
});
ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes);
expect(
(
await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
type: AlertingAuthorizationFilterType.KQL,
fieldNames: {
ruleTypeId: 'path.to.rule_type_id',
consumer: 'consumer-field',
},
})
).filter
).toEqual(
fromKueryExpression(
`((path.to.rule_type_id:myAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (path.to.rule_type_id:mySecondAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (path.to.rule_type_id:myOtherAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
)

const filter = (
await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
type: AlertingAuthorizationFilterType.KQL,
fieldNames: {
ruleTypeId: 'path.to.rule_type_id',
consumer: 'consumer-field',
},
})
).filter;

expect(toKqlExpression(filter as KueryNode)).toMatchInlineSnapshot(
`"((path.to.rule_type_id: myAppAlertType AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: myApp OR consumer-field: myOtherApp OR consumer-field: myAppWithSubFeature)) OR (path.to.rule_type_id: mySecondAppAlertType AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: myApp OR consumer-field: myOtherApp OR consumer-field: myAppWithSubFeature)) OR (path.to.rule_type_id: myOtherAppAlertType AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: myApp OR consumer-field: myOtherApp OR consumer-field: myAppWithSubFeature)))"`
);
});
test('throws if user has no privileges to any rule type', async () => {
Expand Down Expand Up @@ -1274,6 +1273,10 @@ describe('AlertingAuthorization', () => {
"all": true,
"read": true,
},
"discover": Object {
"all": true,
"read": true,
},
"myApp": Object {
"all": true,
"read": true,
Expand Down Expand Up @@ -1311,6 +1314,10 @@ describe('AlertingAuthorization', () => {
"all": true,
"read": true,
},
"discover": Object {
"all": true,
"read": true,
},
"myApp": Object {
"all": true,
"read": true,
Expand Down Expand Up @@ -2251,20 +2258,18 @@ describe('AlertingAuthorization', () => {
});
});
test('creates a filter based on the privileged types', async () => {
expect(
(
await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
type: AlertingAuthorizationFilterType.KQL,
fieldNames: {
ruleTypeId: 'path.to.rule_type_id',
consumer: 'consumer-field',
},
})
).filter
).toEqual(
fromKueryExpression(
`path.to.rule_type_id:.esQuery and consumer-field:(alerts or stackAlerts or discover)`
)
const filter = (
await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
type: AlertingAuthorizationFilterType.KQL,
fieldNames: {
ruleTypeId: 'path.to.rule_type_id',
consumer: 'consumer-field',
},
})
).filter;

expect(toKqlExpression(filter as KueryNode)).toMatchInlineSnapshot(
`"(path.to.rule_type_id: .esQuery AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: stackAlerts))"`
);
});
});
Expand Down Expand Up @@ -2557,21 +2562,20 @@ describe('AlertingAuthorization', () => {
expect(ruleTypeRegistry.get).toHaveBeenCalledTimes(1);
});
});

test('creates a filter based on the privileged types', async () => {
expect(
(
await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
type: AlertingAuthorizationFilterType.KQL,
fieldNames: {
ruleTypeId: 'path.to.rule_type_id',
consumer: 'consumer-field',
},
})
).filter
).toEqual(
fromKueryExpression(
`(path.to.rule_type_id:.esQuery and consumer-field:(alerts or stackAlerts or logs or discover)) or (path.to.rule_type_id:.logs-threshold-o11y and consumer-field:(alerts or stackAlerts or logs or discover)) or (path.to.rule_type_id:.threshold-rule-o11y and consumer-field:(alerts or stackAlerts or logs or discover))`
)
const filter = (
await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
type: AlertingAuthorizationFilterType.KQL,
fieldNames: {
ruleTypeId: 'path.to.rule_type_id',
consumer: 'consumer-field',
},
})
).filter;

expect(toKqlExpression(filter as KueryNode)).toMatchInlineSnapshot(
`"((path.to.rule_type_id: .esQuery AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: stackAlerts OR consumer-field: logs)) OR (path.to.rule_type_id: .logs-threshold-o11y AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: stackAlerts OR consumer-field: logs)) OR (path.to.rule_type_id: .threshold-rule-o11y AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: stackAlerts OR consumer-field: logs)))"`
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { KueryNode } from '@kbn/es-query';
import { SecurityPluginSetup } from '@kbn/security-plugin/server';
import { FeaturesPluginStart } from '@kbn/features-plugin/server';
import { Space } from '@kbn/spaces-plugin/server';
import { STACK_ALERTS_FEATURE_ID } from '@kbn/rule-data-utils';
import { RegistryRuleType } from '../rule_type_registry';
import { ALERTING_FEATURE_ID, RuleTypeRegistry } from '../types';
import {
Expand Down Expand Up @@ -88,6 +89,8 @@ export interface ConstructorOptions {
authorization?: SecurityPluginSetup['authz'];
}

const DISCOVER_FEATURE_ID = 'discover';

export class AlertingAuthorization {
private readonly ruleTypeRegistry: RuleTypeRegistry;
private readonly request: KibanaRequest;
Expand Down Expand Up @@ -135,7 +138,7 @@ export class AlertingAuthorization {

this.allPossibleConsumers = this.featuresIds.then((featuresIds) => {
return featuresIds.size
? asAuthorizedConsumers([ALERTING_FEATURE_ID, ...featuresIds], {
? asAuthorizedConsumers([ALERTING_FEATURE_ID, DISCOVER_FEATURE_ID, ...featuresIds], {
read: true,
all: true,
})
Expand Down Expand Up @@ -328,7 +331,22 @@ export class AlertingAuthorization {
hasAllRequested: boolean;
authorizedRuleTypes: Set<RegistryAlertTypeWithAuth>;
}> {
const fIds = featuresIds ?? (await this.featuresIds);
const fIds = new Set(featuresIds ?? (await this.featuresIds));

/**
* Temporary hack to fix issues with the discover consumer.
* Issue: https://github.com/elastic/kibana/issues/184595.
* PR https://github.com/elastic/kibana/pull/183756 will
* remove the hack and fix it in a generic way.
*
* The discover consumer should be authorized
* as the stackAlerts consumer.
*/
if (fIds.has(DISCOVER_FEATURE_ID)) {
fIds.delete(DISCOVER_FEATURE_ID);
fIds.add(STACK_ALERTS_FEATURE_ID);
}

if (this.authorization && this.shouldCheckAuthorization()) {
const checkPrivileges = this.authorization.checkPrivilegesDynamicallyWithRequest(
this.request
Expand All @@ -347,11 +365,15 @@ export class AlertingAuthorization {
>();
const allPossibleConsumers = await this.allPossibleConsumers;
const addLegacyConsumerPrivileges = (legacyConsumer: string) =>
legacyConsumer === ALERTING_FEATURE_ID || isEmpty(featuresIds);
legacyConsumer === ALERTING_FEATURE_ID ||
legacyConsumer === DISCOVER_FEATURE_ID ||
isEmpty(featuresIds);

for (const feature of fIds) {
const featureDef = this.features
.getKibanaFeatures()
.find((kFeature) => kFeature.id === feature);

for (const ruleTypeId of featureDef?.alerting ?? []) {
const ruleTypeAuth = ruleTypesWithAuthorization.find((rtwa) => rtwa.id === ruleTypeId);
if (ruleTypeAuth) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,14 +368,17 @@ export class ExplorerChartSingleMetric extends React.Component {
// These are used for displaying tooltips on mouseover.
// Don't render dots where value=null (data gaps, with no anomalies)
// or for multi-bucket anomalies.
// Except for scheduled events.
const dots = lineChartGroup
.append('g')
.attr('class', 'chart-markers')
.selectAll('.metric-value')
.data(
data.filter(
(d) =>
(d.value !== null || typeof d.anomalyScore === 'number') &&
(d.value !== null ||
typeof d.anomalyScore === 'number' ||
d.scheduledEvents !== undefined) &&
!showMultiBucketAnomalyMarker(d)
)
);
Expand Down Expand Up @@ -407,7 +410,11 @@ export class ExplorerChartSingleMetric extends React.Component {
// Update all dots to new positions.
dots
.attr('cx', (d) => lineChartXScale(d.date))
.attr('cy', (d) => lineChartYScale(d.value))
// Fallback with domain's min value if value is null
// To ensure event markers are rendered properly at the bottom of the chart
.attr('cy', (d) =>
lineChartYScale(d.value !== null ? d.value : lineChartYScale.domain()[0])
)
.attr('class', (d) => {
let markerClass = 'metric-value';
if (isAnomalyVisible(d)) {
Expand Down Expand Up @@ -470,7 +477,14 @@ export class ExplorerChartSingleMetric extends React.Component {
// Update all markers to new positions.
scheduledEventMarkers
.attr('x', (d) => lineChartXScale(d.date) - LINE_CHART_ANOMALY_RADIUS)
.attr('y', (d) => lineChartYScale(d.value) - SCHEDULED_EVENT_SYMBOL_HEIGHT / 2);
.attr(
'y',
(d) =>
// Fallback with domain's min value if value is null
// To ensure event markers are rendered properly at the bottom of the chart
lineChartYScale(d.value !== null ? d.value : lineChartYScale.domain()[0]) -
SCHEDULED_EVENT_SYMBOL_HEIGHT / 2
);
}

function showAnomalyPopover(marker, circle) {
Expand Down Expand Up @@ -596,7 +610,7 @@ export class ExplorerChartSingleMetric extends React.Component {
});
}
}
} else {
} else if (marker.value !== null) {
tooltipData.push({
label: i18n.translate(
'xpack.ml.explorer.singleMetricChart.valueWithoutAnomalyScoreLabel',
Expand Down
26 changes: 21 additions & 5 deletions x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1055,9 +1055,11 @@ export function anomalyChartsDataProvider(mlClient: MlClient, client: IScopedClu
// differently because of how the source data is structured.
// For rare chart values we are only interested wether a value is either `0` or not,
// `0` acts like a flag in the chart whether to display the dot/marker.
// All other charts (single metric, population) are metric based and with
// For single metric chart, we need to pass null values to display data gaps.
// All other charts are distribution based and with
// those a value of `null` acts as the flag to hide a data point.
if (
chartType === CHART_TYPE.SINGLE_METRIC ||
(chartType === CHART_TYPE.EVENT_DISTRIBUTION && value > 0) ||
(chartType !== CHART_TYPE.EVENT_DISTRIBUTION && value !== null)
) {
Expand All @@ -1079,6 +1081,7 @@ export function anomalyChartsDataProvider(mlClient: MlClient, client: IScopedClu
// Iterate through the anomaly records, adding anomalyScore properties
// to the chartData entries for anomalous buckets.
const chartDataForPointSearch = getChartDataForPointSearch(chartData, records[0], chartType);
let shouldSortChartData = false;
each(records, (record) => {
// Look for a chart point with the same time as the record.
// If none found, insert a point for anomalies due to a gap in the data.
Expand All @@ -1087,10 +1090,17 @@ export function anomalyChartsDataProvider(mlClient: MlClient, client: IScopedClu
if (chartPoint === undefined) {
chartPoint = { date: recordTime, value: null };
chartData.push(chartPoint);
shouldSortChartData = true;
}
if (chartPoint !== undefined) {
chartPoint.anomalyScore = record.record_score;

// If it is an empty chart point, set the value to the actual value
// To properly display the anomaly marker on the chart
if (chartPoint.value === null) {
chartPoint.value = Array.isArray(record.actual) ? record.actual[0] : record.actual;
}

if (record.actual !== undefined) {
chartPoint.actual = record.actual;
chartPoint.typical = record.typical;
Expand Down Expand Up @@ -1119,21 +1129,27 @@ export function anomalyChartsDataProvider(mlClient: MlClient, client: IScopedClu
}
});

// Chart data is sorted by default, but if we added points for anomalies,
// we need to sort again to ensure the points are in the correct order.
if (shouldSortChartData) {
chartData.sort((a, b) => a.date - b.date);
}

// Add a scheduledEvents property to any points in the chart data set
// which correspond to times of scheduled events for the job.
if (scheduledEvents !== undefined) {
each(scheduledEvents, (events, time) => {
const chartPoint = findChartPointForTime(chartDataForPointSearch, Number(time));
if (chartPoint !== undefined) {
chartPoint.scheduledEvents = events;
// We do not want to create additional points for single metric charts
// as it could break the chart.
} else if (chartType !== CHART_TYPE.SINGLE_METRIC) {
} else {
// If there's no underlying metric data point for the scheduled event,
// create a new chart point with a value of 0.
// Except for Single Metric Charts, where we want to create a point at the bottom of the chart.
// Which is not always `0`.
const eventChartPoint: ChartPoint = {
date: Number(time),
value: 0,
value: chartType === CHART_TYPE.SINGLE_METRIC ? null : 0,
entity: SCHEDULE_EVENT_MARKER_ENTITY,
scheduledEvents: events,
};
Expand Down
Loading

0 comments on commit 73f2bca

Please sign in to comment.