Skip to content

Commit d06a1f0

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #153 from alanconway/alert-update
NO-JIRA: fix: korrel8r AlertDomain gets alert name from numeric rule ID.
2 parents eba353a + 5b696c1 commit d06a1f0

14 files changed

Lines changed: 109 additions & 58 deletions

‎web/src/__tests__/alert.spec.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe('AlertNode.fromURL', () => {
99
'container=bad-deployment&endpoint=https-main&job=kube-state-metrics&namespace=default&pod=bad-deployment&' +
1010
'reason=CrashLoopBackOff&service=kube-state-metrics&uid=00000000-0000-0000-0000-000000000000',
1111
query:
12-
'alert:alert:{"severity":"warning","alertname":"KubePodCrashLooping","container":"bad-deployment",' +
12+
'alert:alert:{"prometheus":"openshift-monitoring/k8s","severity":"warning","alertname":"KubePodCrashLooping","container":"bad-deployment",' +
1313
'"endpoint":"https-main","job":"kube-state-metrics","namespace":"default","pod":"bad-deployment",' +
1414
'"reason":"CrashLoopBackOff","service":"kube-state-metrics","uid":"00000000-0000-0000-0000-000000000000"}',
1515
},
@@ -19,9 +19,33 @@ describe('AlertNode.fromURL', () => {
1919
query:
2020
'alert:alert:{"alertname":"KubePodCrashLooping","container":"bad-deployment","namespace":"default","pod":"bad-pod"}',
2121
},
22-
])('converts $url', ({ url, query }) =>
23-
expect(new AlertDomain().linkToQuery(new URIRef(url))).toEqual(Query.parse(query)),
24-
);
22+
{
23+
url: 'monitoring/alerts/12345',
24+
query: 'alert:alert:{"alertname":"FooAlert"}',
25+
},
26+
{
27+
url: 'monitoring/alertrules/12345',
28+
query: 'alert:alert:{"alertname":"FooAlert"}',
29+
},
30+
{
31+
url: 'monitoring/alertrules/12345?alertname=BarAlert',
32+
query: 'alert:alert:{"alertname":"BarAlert"}',
33+
},
34+
])('converts $url', ({ url, query }) => {
35+
const domain = new AlertDomain(new Map([['12345', 'FooAlert']]));
36+
expect(domain.linkToQuery(new URIRef(url)).toString()).toEqual(query);
37+
});
38+
});
39+
40+
describe('AlertNode.fromURL raises error', () => {
41+
it.each([
42+
{
43+
url: 'monitoring/alertrules/999',
44+
error: 'invalid link for domain alert: monitoring/alertrules/999: cannot find alertname',
45+
},
46+
])('converts $url', ({ url, error }) => {
47+
expect(() => new AlertDomain().linkToQuery(new URIRef(url))).toThrow(error);
48+
});
2549
});
2650

2751
describe('AlertDomain.fromQuery', () => {

‎web/src/__tests__/all-domains.spec.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { allDomains } from '../korrel8r/all-domains';
2-
import { Constraint, Query, URIRef } from '../korrel8r/types';
2+
import { Constraint, Domains, Query, URIRef } from '../korrel8r/types';
33

44
beforeAll(() => {
55
// Mock API discovery resources.
@@ -71,8 +71,9 @@ it.each([
7171
'log:infrastructure:{kubernetes_namespace_name="openshift-image-registry"}|json|kubernetes_labels_docker_registry="default"',
7272
},
7373
])('convert URL<=>link', ({ url, query, constraint }) => {
74-
expect(allDomains.linkToQuery(new URIRef(url))).toEqual(Query.parse(query));
75-
expect(allDomains.queryToLink(Query.parse(query), Constraint.fromAPI(constraint))).toEqual(
74+
const d = new Domains(...allDomains);
75+
expect(d.linkToQuery(new URIRef(url))).toEqual(Query.parse(query));
76+
expect(d.queryToLink(Query.parse(query), Constraint.fromAPI(constraint))).toEqual(
7677
new URIRef(url),
7778
);
7879
});

‎web/src/__tests__/log.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ describe('expected errors', () => {
8383
it.each([
8484
{
8585
url: 'monitoring/log',
86-
expected: 'domain log: invalid link: monitoring/log',
86+
expected: 'invalid link for domain log: monitoring/log',
8787
},
8888
{
8989
url: 'monitoring/logs?q={kubernetes_namespace_name="default",kubernetes_pod_name="foo"}',
9090
expected:
91-
'domain log: invalid link: monitoring/logs?q=%7Bkubernetes_namespace_name%3D%22default%22%2Ckubernetes_pod_name%3D%22foo%22%7D',
91+
'invalid link for domain log: monitoring/logs?q=%7Bkubernetes_namespace_name%3D%22default%22%2Ckubernetes_pod_name%3D%22foo%22%7D',
9292
},
9393
])('error from url: $url', ({ url, expected }) => {
9494
expect(() => new LogDomain().linkToQuery(new URIRef(url))).toThrow(expected);
@@ -97,11 +97,11 @@ describe('expected errors', () => {
9797
it.each([
9898
{
9999
query: 'foo:bar:baz',
100-
expected: 'domain log: invalid query, unknown class: foo:bar:baz',
100+
expected: 'invalid query for domain log: foo:bar:baz: unknown class',
101101
},
102102
{
103103
query: 'log:incorrect:{}',
104-
expected: 'domain log: invalid query, unknown class: log:incorrect:{}',
104+
expected: 'invalid query for domain log: log:incorrect:{}: unknown class',
105105
},
106106
])('error from query: $query', ({ query, expected }) => {
107107
expect(() => new LogDomain().queryToLink(Query.parse(query))).toThrow(expected);

‎web/src/__tests__/metric.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ describe('metric', () => {
3333
it.each([
3434
{
3535
url: 'foobar',
36-
error: new TypeError('domain metric: invalid link: foobar'),
36+
error: new TypeError('invalid link for domain metric: foobar'),
3737
},
3838
{
3939
url: 'monitoring/query-browser',
40-
error: 'domain metric: invalid link: monitoring/query-browser',
40+
error: 'invalid link for domain metric: monitoring/query-browser',
4141
},
4242
])('$url', ({ url, error }) => {
4343
expect(() => metric.linkToQuery(new URIRef(url))).toThrow(error);
@@ -48,11 +48,11 @@ describe('metric', () => {
4848
it.each([
4949
{
5050
query: 'wrongdomain:metric:foo',
51-
error: 'domain metric: invalid query, wrong domain: wrongdomain:metric:foo',
51+
error: 'invalid query for domain metric: wrongdomain:metric:foo',
5252
},
5353
{
5454
query: 'metric:badclass:foo',
55-
error: 'domain metric: invalid class: badclass',
55+
error: 'invalid class for domain metric: badclass',
5656
},
5757
])('$query', ({ query, error }) => {
5858
expect(() => metric.queryToLink(Query.parse(query))).toThrow(error);

‎web/src/__tests__/netflow.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ describe('', () => {
7272
it.each([
7373
{
7474
url: 'netflow-traffi',
75-
expected: 'domain netflow: invalid link: netflow-traffi',
75+
expected: 'invalid link for domain netflow: netflow-traffi',
7676
},
7777
])('expect error fromURL($url)', ({ url, expected }) => {
7878
expect(() => new NetflowDomain().linkToQuery(new URIRef(url))).toThrow(expected);
@@ -85,15 +85,15 @@ describe('', () => {
8585
},
8686
{
8787
query: 'netflow:incorrect:{}',
88-
expected: 'invalid query, unknown class: netflow:incorrect:{}',
88+
expected: 'invalid query for domain netflow: netflow:incorrect:{}: unknown class',
8989
},
9090
{
9191
query: 'netflow:network:{SrcK8S_Type="Pod"=wrong}',
92-
expected: 'domain netflow: invalid query: netflow:network:{SrcK8S_Type="Pod"=wrong}',
92+
expected: 'invalid query for domain netflow: netflow:network:{SrcK8S_Type="Pod"=wrong}',
9393
},
9494
{
9595
query: 'netflow:network:{SrcK8S_Type}',
96-
expected: 'domain netflow: invalid query: netflow:network:{SrcK8S_Type}',
96+
expected: 'invalid query for domain netflow: netflow:network:{SrcK8S_Type}',
9797
},
9898
])('expect error fromQuery($query)', ({ query, expected }) => {
9999
expect(() => new NetflowDomain().queryToLink(Query.parse(query))).toThrow(expected);

‎web/src/__tests__/types.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ describe('Domain', () => {
6868
it('queryToLink', () => {
6969
expect(d.queryToLink(abc).toString()).toEqual('a/b?c=d');
7070
const query = Query.parse('x:b:c');
71-
expect(() => d.queryToLink(query)).toThrow('domain a: invalid query: x:b:c');
71+
expect(() => d.queryToLink(query)).toThrow('invalid query for domain a: x:b:c');
7272
});
7373
it('linkToQuery', () => {
7474
expect(d.linkToQuery(new URIRef('a/b?c=d'))).toEqual(abc);

‎web/src/components/Korrel8rPanel.tsx‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { useDispatch, useSelector } from 'react-redux';
2222
import { useLocationQuery } from '../hooks/useLocationQuery';
2323
import { usePluginAvailable } from '../hooks/usePluginAvailable';
2424
import { getGoalsGraph, getNeighborsGraph } from '../korrel8r-client';
25+
import { AlertDomain } from '../korrel8r/alert';
26+
import { allDomains } from '../korrel8r/all-domains';
2527
import * as api from '../korrel8r/client';
2628
import * as korrel8r from '../korrel8r/types';
2729
import { defaultSearch, Search, SearchType, setPersistedSearch } from '../redux-actions';
@@ -48,7 +50,16 @@ export default function Korrel8rPanel() {
4850
const dispatch = useDispatch();
4951

5052
// State
51-
const locationQuery = useLocationQuery();
53+
const alertRules = useSelector((state: State) => state.observe?.get('rules'));
54+
const alertIDs = React.useMemo(
55+
() => new Map<string, string>(alertRules.map(({ id, name }) => [id, name])),
56+
[alertRules],
57+
);
58+
const domains = React.useMemo(
59+
() => new korrel8r.Domains(...allDomains, new AlertDomain(alertIDs)),
60+
[alertIDs],
61+
);
62+
const locationQuery = useLocationQuery(domains);
5263
const [search, setSearch] = React.useState<Search>(
5364
persistedSearch?.queryStr
5465
? persistedSearch
@@ -205,20 +216,27 @@ export default function Korrel8rPanel() {
205216
</ExpandableSection>
206217
<Divider />
207218
<FlexItem className="tp-plugin__panel-topology-container" grow={{ default: 'grow' }}>
208-
<Topology result={result} constraint={search.constraint} t={t} setSearch={setSearch} />
219+
<Topology
220+
domains={domains}
221+
result={result}
222+
constraint={search.constraint}
223+
t={t}
224+
setSearch={setSearch}
225+
/>
209226
</FlexItem>
210227
</>
211228
);
212229
}
213230

214231
interface TopologyProps {
232+
domains: korrel8r.Domains;
215233
result?: Result;
216234
constraint: korrel8r.Constraint;
217235
t: TFunction;
218236
setSearch: (search: Search) => void;
219237
}
220238

221-
const Topology: React.FC<TopologyProps> = ({ result, t, constraint }) => {
239+
const Topology: React.FC<TopologyProps> = ({ domains, result, t, constraint }) => {
222240
const [loggingAvailable, loggingAvailableLoading] = usePluginAvailable('logging-view-plugin');
223241
const [netobserveAvailable, netobserveAvailableLoading] = usePluginAvailable('netobserv-plugin');
224242

@@ -231,6 +249,7 @@ const Topology: React.FC<TopologyProps> = ({ result, t, constraint }) => {
231249
// Non-empty graph
232250
return (
233251
<Korrel8rTopology
252+
domains={domains}
234253
graph={result.graph}
235254
loggingAvailable={loggingAvailable}
236255
netobserveAvailable={netobserveAvailable}

‎web/src/components/topology/Korrel8rTopology.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import {
3535
import * as React from 'react';
3636
import { useTranslation } from 'react-i18next';
3737
import { useNavigate } from 'react-router-dom-v5-compat';
38-
import { allDomains } from '../../korrel8r/all-domains';
3938
import * as korrel8r from '../../korrel8r/types';
4039
import './korrel8rtopology.css';
4140

@@ -98,12 +97,13 @@ const NODE_DIAMETER = 75;
9897
const PADDING = 30;
9998

10099
export const Korrel8rTopology: React.FC<{
100+
domains: korrel8r.Domains;
101101
graph: korrel8r.Graph;
102102
reFit?: boolean;
103103
loggingAvailable: boolean;
104104
netobserveAvailable: boolean;
105105
constraint: korrel8r.Constraint;
106-
}> = ({ graph, reFit, loggingAvailable, netobserveAvailable, constraint }) => {
106+
}> = ({ domains, graph, reFit, loggingAvailable, netobserveAvailable, constraint }) => {
107107
const { t } = useTranslation('plugin__troubleshooting-panel-console-plugin');
108108
const navigate = useNavigate();
109109
const [selectedIds, setSelectedIds] = React.useState<string[]>([]);
@@ -149,7 +149,7 @@ export const Korrel8rTopology: React.FC<{
149149
const navigateToQuery = React.useCallback(
150150
(query: korrel8r.Query, constraint: korrel8r.Constraint) => {
151151
try {
152-
let link = allDomains.queryToLink(query, constraint)?.toString();
152+
let link = domains.queryToLink(query, constraint)?.toString();
153153
if (!link) return;
154154
if (!link.startsWith('/')) link = '/' + link;
155155
// eslint-disable-next-line no-console
@@ -168,7 +168,7 @@ export const Korrel8rTopology: React.FC<{
168168
console.error(`korrel8r navigateToQuery: ${e}`, '\nquery', query);
169169
}
170170
},
171-
[navigate],
171+
[navigate, domains],
172172
);
173173

174174
const selectionAction = React.useCallback(

‎web/src/hooks/useLocationQuery.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import { useLocation } from 'react-router-dom-v5-compat';
2-
import { allDomains } from '../korrel8r/all-domains';
3-
import { Query, URIRef } from '../korrel8r/types';
2+
import { Domains, Query, URIRef } from '../korrel8r/types';
43

54
/** Returns the Korrel8r query for the current browser location or undefined */
6-
export const useLocationQuery = (): Query | undefined => {
5+
export const useLocationQuery = (domains: Domains): Query | undefined => {
76
const location = useLocation();
87
try {
98
const link = new URIRef(location.pathname + location.search);
10-
const q = allDomains.linkToQuery(link);
9+
const q = domains.linkToQuery(link);
1110
return q;
1211
} catch (e) {
1312
// eslint-disable-next-line no-console
14-
console.error(`useLocation error ${location.toString()} => ${e}`);
13+
console.error('useLocation', e);
1514
}
1615
};

‎web/src/korrel8r/alert.ts‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { Class, Domain, Query, URIRef, keyValueList, parseKeyValueList } from './types';
22

33
export class AlertDomain extends Domain {
4-
constructor() {
4+
// Constructor takes an optional map of alert rule ID to name mappings.
5+
// Numeric IDs are used to refer to alerting rules with no alert parameters.
6+
constructor(private idToName?: Map<string, string>) {
57
super('alert');
68
}
79

@@ -12,20 +14,25 @@ export class AlertDomain extends Domain {
1214

1315
// Convert a Query to a relative URI reference.
1416
linkToQuery(link: URIRef): Query {
15-
const m = link.pathname.match(/monitoring\/alerts(.*)$/);
17+
const m = link.pathname.match(/monitoring\/(?:alerts|alertrules)(?:\/(.*))?$/);
1618
if (!m) throw this.badLink(link);
17-
const path = m[1];
18-
let selectors = {};
19-
if (path === '') {
20-
// No ID in path, this is a search URL
21-
selectors = parseKeyValueList(link.searchParams.get('alerts'));
19+
const ruleID = m?.[1];
20+
let selector: { alertname?: string };
21+
if (ruleID) {
22+
// Search for alerts belonging to a specific alerting rule.
23+
selector = Object.fromEntries(link.searchParams);
24+
// Get the name from the path if not found in search parameters.
25+
if (!selector.alertname) {
26+
const name = this?.idToName?.get(ruleID);
27+
if (name) selector.alertname = name;
28+
}
29+
// Must have an alertname for a specific rule search.
30+
if (!selector.alertname) throw this.badLink(link, 'cannot find alertname');
2231
} else {
23-
const params = link.searchParams;
24-
params.delete('prometheus'); // Not part of the label selectors.
25-
params.delete('managed_cluster'); // Not part of the label selectors.
26-
for (const [key, value] of params) selectors[key] = value;
32+
// Generic alert search across rules. Empty selector is allowed - means "all alerts"
33+
selector = parseKeyValueList(link.searchParams.get('alerts'));
2734
}
28-
return new Query(this.class('alert'), JSON.stringify(selectors));
35+
return new Query(this.class('alert'), JSON.stringify(selector));
2936
}
3037

3138
queryToLink(query: Query): URIRef {

0 commit comments

Comments
 (0)