Skip to content
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

[RAC] Link inventory alerts to the right inventory view #113553

Merged
merged 6 commits into from
Oct 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions x-pack/plugins/infra/common/alerting/metrics/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
* 2.0.
*/
import * as rt from 'io-ts';
import { Unit } from '@elastic/datemath';
import { ANOMALY_THRESHOLD } from '../../infra_ml';
import { InventoryItemType, SnapshotMetricType } from '../../inventory_models/types';
import { SnapshotCustomMetricInput } from '../../http_api';

// TODO: Have threshold and inventory alerts import these types from this file instead of from their
// local directories
Expand Down Expand Up @@ -54,3 +57,25 @@ export interface MetricAnomalyParams {
threshold: Exclude<ANOMALY_THRESHOLD, ANOMALY_THRESHOLD.LOW>;
influencerFilter: rt.TypeOf<typeof metricAnomalyInfluencerFilterRT> | undefined;
}

// Types for the executor

export interface InventoryMetricConditions {
metric: SnapshotMetricType;
timeSize: number;
timeUnit: Unit;
sourceId?: string;
threshold: number[];
comparator: Comparator;
customMetric?: SnapshotCustomMetricInput;
warningThreshold?: number[];
warningComparator?: Comparator;
}

export interface InventoryMetricThresholdParams {
criteria: InventoryMetricConditions[];
filterQuery?: string;
nodeType: InventoryItemType;
sourceId?: string;
alertOnNoData?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,48 @@
* 2.0.
*/

import { ALERT_REASON } from '@kbn/rule-data-utils';
import { ALERT_REASON, ALERT_RULE_PARAMS, TIMESTAMP } from '@kbn/rule-data-utils';
import { encode } from 'rison-node';
import { stringify } from 'query-string';
import { ObservabilityRuleTypeFormatter } from '../../../../observability/public';
import { InventoryMetricThresholdParams } from '../../../common/alerting/metrics';

export const formatReason: ObservabilityRuleTypeFormatter = ({ fields }) => {
const reason = fields[ALERT_REASON] ?? '-';
const link = '/app/metrics/inventory'; // TODO https://github.com/elastic/kibana/issues/106497
const ruleParams = parseRuleParams(fields[ALERT_RULE_PARAMS]);

let link = '/app/metrics/link-to/inventory?';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably use getUrlForApp here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used within a React context so calling useKibana is not possible. I'm looking how easy is to inject the kibana services and it's not that straightforward.

The link is correctly prefixed when used (the prepend() call in the linked code), so I wonder if that's good enough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shakes fist at sky "Damn you hooks!"


if (ruleParams) {
const linkToParams: Record<string, any> = {
nodeType: ruleParams.nodeType,
timestamp: Date.parse(fields[TIMESTAMP]),
customMetric: '',
};

// We always pick the first criteria metric for the URL
const criteria = ruleParams.criteria[0];
if (criteria.customMetric && criteria.customMetric.id !== 'alert-custom-metric') {
const customMetric = encode(criteria.customMetric);
linkToParams.customMetric = customMetric;
linkToParams.metric = customMetric;
} else {
linkToParams.metric = encode({ type: criteria.metric });
}

link += stringify(linkToParams);
}

return {
reason,
link,
};
};

function parseRuleParams(params?: string): InventoryMetricThresholdParams | undefined {
try {
return typeof params === 'string' ? JSON.parse(params) : undefined;
} catch (_) {
return;
}
}
2 changes: 2 additions & 0 deletions x-pack/plugins/infra/public/pages/link_to/link_to_metrics.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { match as RouteMatch, Redirect, Route, Switch } from 'react-router-dom';

import { RedirectToNodeDetail } from './redirect_to_node_detail';
import { RedirectToHostDetailViaIP } from './redirect_to_host_detail_via_ip';
import { RedirectToInventory } from './redirect_to_inventory';
import { inventoryModels } from '../../../common/inventory_models';

interface LinkToPageProps {
Expand All @@ -29,6 +30,7 @@ export const LinkToMetricsPage: React.FC<LinkToPageProps> = (props) => {
path={`${props.match.url}/host-detail-via-ip/:hostIp`}
component={RedirectToHostDetailViaIP}
/>
<Route path={`${props.match.url}/inventory`} component={RedirectToInventory} />
<Redirect to="/" />
</Switch>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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 React from 'react';
import { parse } from 'query-string';
import { Redirect, RouteComponentProps } from 'react-router-dom';

// FIXME what would be the right way to build this query string?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we cannot find a good way in this PR we can address this later.

If I recall correctly Kibana provides a service already to create good URLs? Maybe it's worth leaving this as is and wait until that service is used in Metrics to update this code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would need to use the useKibana hook to get ahold of the application service like this:

  const kibana = useKibana<InfraClientSetupDeps>();
  const link = kibana.services.application?.getUrlForApp('infra', { path: `/inventory${inventoryQueryString}` });

As for the QUERY_STRING_TEMPLATE what you're doing is probably fine BUT if we wanted to make sure the Rison is serialized correctly, we could use the Rison library to encode the objects (like you did for the custom metric).

Copy link
Contributor Author

@afgomez afgomez Oct 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more concerned about what happens if/when the parameters in the URL change. It feels I'm duplicating things here and it's just a matter of time until it gets changed somewhere else and this breaks.

Is there something in metrics to generate valid URLs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, apparently the <Redirect /> component where this link is used is handling already the app prefix. When I use getUrlForApp the end result is /guk/app/metrics/guk/app/infra/inventory (when using infra as an app).

I think moving forward it's better to leave the code as it is now (revisiting how the query string is constructed). What do you think @simianhacker?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

const QUERY_STRING_TEMPLATE =
"?waffleFilter=(expression:'',kind:kuery)&waffleTime=(currentTime:{timestamp},isAutoReloading:!f)&waffleOptions=(accountId:'',autoBounds:!t,boundsOverride:(max:1,min:0),customMetrics:!({customMetric}),customOptions:!(),groupBy:!(),legend:(palette:cool,reverseColors:!f,steps:10),metric:{metric},nodeType:{nodeType},region:'',sort:(by:name,direction:desc),timelineOpen:!f,view:map)";

export const RedirectToInventory: React.FC<RouteComponentProps> = ({ location }) => {
const parsedQueryString = parseQueryString(location.search);

const inventoryQueryString = QUERY_STRING_TEMPLATE.replace(
/{(\w+)}/g,
(_, key) => parsedQueryString[key] || ''
);

return <Redirect to={'/inventory' + inventoryQueryString} />;
};

function parseQueryString(search: string): Record<string, string> {
if (search.length === 0) {
return {};
}

const obj = parse(search.substring(1));

// Force all values into string. If they are empty don't create the keys
for (const key in obj) {
if (Object.hasOwnProperty.call(obj, key)) {
if (!obj[key]) {
delete obj[key];
}
if (Array.isArray(obj.key)) {
obj[key] = obj[key]![0];
}
}
}

return obj as Record<string, string>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { ALERT_REASON, ALERT_RULE_PARAMS } from '@kbn/rule-data-utils';
import moment from 'moment';
import { getCustomMetricLabel } from '../../../../common/formatters/get_custom_metric_label';
import { toMetricOpt } from '../../../../common/snapshot_metric_i18n';
import { AlertStates, InventoryMetricConditions } from './types';
import { AlertStates } from './types';
import {
ActionGroupIdsOf,
ActionGroup,
Expand All @@ -20,10 +20,11 @@ import {
RecoveredActionGroup,
} from '../../../../../alerting/common';
import { AlertInstance, AlertTypeState } from '../../../../../alerting/server';
import { InventoryItemType, SnapshotMetricType } from '../../../../common/inventory_models/types';
import { SnapshotMetricType } from '../../../../common/inventory_models/types';
import { InfraBackendLibs } from '../../infra_types';
import { METRIC_FORMATTERS } from '../../../../common/formatters/snapshot_metric_formats';
import { createFormatter } from '../../../../common/formatters';
import { InventoryMetricThresholdParams } from '../../../../common/alerting/metrics';
import {
buildErrorAlertReason,
buildFiredAlertReason,
Expand All @@ -33,19 +34,10 @@ import {
} from '../common/messages';
import { evaluateCondition } from './evaluate_condition';

interface InventoryMetricThresholdParams {
criteria: InventoryMetricConditions[];
filterQuery: string | undefined;
nodeType: InventoryItemType;
sourceId?: string;
alertOnNoData?: boolean;
}

type InventoryMetricThresholdAllowedActionGroups = ActionGroupIdsOf<
typeof FIRED_ACTIONS | typeof WARNING_ACTIONS
>;

export type InventoryMetricThresholdAlertTypeParams = Record<string, any>;
export type InventoryMetricThresholdAlertTypeState = AlertTypeState; // no specific state used
export type InventoryMetricThresholdAlertInstanceState = AlertInstanceState; // no specific state used
export type InventoryMetricThresholdAlertInstanceContext = AlertInstanceContext; // no specific instance context used
Expand All @@ -64,14 +56,13 @@ type InventoryMetricThresholdAlertInstanceFactory = (

export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =>
libs.metricsRules.createLifecycleRuleExecutor<
InventoryMetricThresholdAlertTypeParams,
InventoryMetricThresholdParams & Record<string, unknown>,
InventoryMetricThresholdAlertTypeState,
InventoryMetricThresholdAlertInstanceState,
InventoryMetricThresholdAlertInstanceContext,
InventoryMetricThresholdAllowedActionGroups
>(async ({ services, params }) => {
const { criteria, filterQuery, sourceId, nodeType, alertOnNoData } =
params as InventoryMetricThresholdParams;
const { criteria, filterQuery, sourceId, nodeType, alertOnNoData } = params;
if (criteria.length === 0) throw new Error('Cannot execute an alert with 0 conditions');
const { alertWithLifecycle, savedObjectsClient } = services;
const alertInstanceFactory: InventoryMetricThresholdAlertInstanceFactory = (id, reason) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* 2.0.
*/

import { schema } from '@kbn/config-schema';
import { schema, Type } from '@kbn/config-schema';
import { Unit } from '@elastic/datemath';
import { i18n } from '@kbn/i18n';
import { PluginSetupContract } from '../../../../../alerting/server';
import {
Expand All @@ -26,21 +27,32 @@ import {
metricActionVariableDescription,
thresholdActionVariableDescription,
} from '../common/messages';
import {
SnapshotMetricTypeKeys,
SnapshotMetricType,
InventoryItemType,
} from '../../../../common/inventory_models/types';
import {
SNAPSHOT_CUSTOM_AGGREGATIONS,
SnapshotCustomAggregation,
} from '../../../../common/http_api/snapshot_api';

const condition = schema.object({
threshold: schema.arrayOf(schema.number()),
comparator: oneOfLiterals(Object.values(Comparator)),
timeUnit: schema.string(),
comparator: oneOfLiterals(Object.values(Comparator)) as Type<Comparator>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these type assertions are necessary to make the kibana schema play nicely with the io-ts generated types.

timeUnit: schema.string() as Type<Unit>,
timeSize: schema.number(),
metric: schema.string(),
metric: oneOfLiterals(Object.keys(SnapshotMetricTypeKeys)) as Type<SnapshotMetricType>,
warningThreshold: schema.maybe(schema.arrayOf(schema.number())),
warningComparator: schema.maybe(oneOfLiterals(Object.values(Comparator))),
warningComparator: schema.maybe(oneOfLiterals(Object.values(Comparator))) as Type<
Comparator | undefined
>,
customMetric: schema.maybe(
schema.object({
type: schema.literal('custom'),
id: schema.string(),
field: schema.string(),
aggregation: schema.string(),
aggregation: oneOfLiterals(SNAPSHOT_CUSTOM_AGGREGATIONS) as Type<SnapshotCustomAggregation>,
label: schema.maybe(schema.string()),
})
),
Expand All @@ -59,7 +71,7 @@ export async function registerMetricInventoryThresholdAlertType(
params: schema.object(
{
criteria: schema.arrayOf(condition),
nodeType: schema.string(),
nodeType: schema.string() as Type<InventoryItemType>,
filterQuery: schema.maybe(
schema.string({ validate: validateIsStringElasticsearchJSONFilter })
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
import { Unit } from '@elastic/datemath';
import { SnapshotCustomMetricInput } from '../../../../common/http_api/snapshot_api';
import { SnapshotMetricType } from '../../../../common/inventory_models/types';
import { Comparator, AlertStates } from '../common/types';
import { Comparator, AlertStates, Aggregators } from '../common/types';

export { Comparator, AlertStates };
export { Comparator, AlertStates, Aggregators };

export const METRIC_INVENTORY_THRESHOLD_ALERT_TYPE_ID = 'metrics.alert.inventory.threshold';

Expand Down