Skip to content

Commit 4f09cc1

Browse files
authored
fix(3707/4021/4479): notebooks alerts multiple predicates, don't hold pipe threshold state after pipe deletion, and have flux not spam the endpoint. (#4480)
Partial fix of the various bugs associated with the notebooks Alert panel. These include: * fix: do not explicitly set the threshold field with an incorrect default `_value` * fix(4021): join multiple threshold conditions correctly * fix(4080): force user to select a single measurement, and do form validation * fix(3707): provide only 1 measurement to monitor.check() (else will error), and apply filters before the monitor.notify() call * fix(3707): only send 1 notification using a keep() to reduce cardinality, then a limit() * fix(4479): have the initial state set by deep cloned object, so does not persist deeply referenced data after a pipe is dropped. * chore: cleanup, so a single definition of equal and not-equal, in thresholds
1 parent 6ac3b2f commit 4f09cc1

File tree

18 files changed

+223
-52
lines changed

18 files changed

+223
-52
lines changed

cypress/e2e/shared/flowsAlerts.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,20 @@
11
import {Organization} from '../../src/types'
22

33
describe('flows alert panel', () => {
4+
const selectAlertMeasurementAndField = () => {
5+
cy.log('Select alert measurement')
6+
cy.getByTestID('dropdown--measurements').click()
7+
cy.getByTestID('dropdown-item--measurement')
8+
.first()
9+
.click()
10+
11+
cy.log('Select alert threshold field')
12+
cy.getByTestID('dropdown--threshold-fields').click()
13+
cy.getByTestID('dropdown-item--threshold-field')
14+
.first()
15+
.click()
16+
}
17+
418
beforeEach(() =>
519
cy.flush().then(() =>
620
cy.signin().then(() =>
@@ -242,6 +256,12 @@ describe('flows alert panel', () => {
242256
cy.getByTestID('dropdown-item--mySecret').click()
243257
cy.getByTestID('input--email').type(fakeEmail)
244258

259+
cy.log('Export attempt without selecting a measurement, will error')
260+
cy.getByTestID('task-form-save').click()
261+
cy.getByTestID('notification-error').should('be.visible')
262+
cy.getByTestID('notification-error--dismiss').click()
263+
selectAlertMeasurementAndField()
264+
245265
cy.getByTestID('task-form-save').click()
246266
/* NOTE: we used to be able to test that the generated flux contained the the
247267
* selected values, but that mechanism has been removed from the product

src/flows/context/flow.current.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,8 @@ export const FlowProvider: FC = ({children}) => {
272272
[currentFlow, update]
273273
)
274274

275-
const addPipe = (initial: PipeData, index?: number) => {
275+
const addPipe = (initialTemplate: PipeData, index?: number) => {
276+
const initial = JSON.parse(JSON.stringify(initialTemplate))
276277
const id = prettyid()
277278
const title =
278279
initial.title ||

src/flows/pipes/Notification/ExportTask.tsx

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,13 @@ const ExportTask: FC = () => {
7171
return acc
7272
}, {})
7373

74-
const conditions = THRESHOLD_TYPES[deadmanType].condition(deadman)
74+
if (!data.measurement) {
75+
throw new Error('please select a measurement.')
76+
}
77+
const measurement = `(r) => r["_measurement"] == "${data.measurement}"`
78+
79+
const conditions =
80+
`(r) => ` + THRESHOLD_TYPES[deadmanType].condition(deadman)
7581
const imports = parse(`
7682
import "strings"
7783
import "regexp"
@@ -113,7 +119,10 @@ task_data = ${format_from_js_file(ast)}
113119
trigger = ${conditions}
114120
messageFn = (r) => ("${data.message}")
115121
116-
${ENDPOINT_DEFINITIONS[data.endpoint]?.generateQuery(data.endpointData)}
122+
${ENDPOINT_DEFINITIONS[data.endpoint]?.generateQuery(
123+
data.endpointData,
124+
measurement
125+
)}
117126
|> monitor["deadman"](t: experimental["subDuration"](from: now(), d: ${
118127
deadman.deadmanCheckValue
119128
}))`
@@ -154,6 +163,7 @@ ${ENDPOINT_DEFINITIONS[data.endpoint]?.generateQuery(data.endpointData)}
154163
data.offset,
155164
data.endpointData,
156165
data.endpoint,
166+
data.measurement,
157167
data.thresholds,
158168
data.message,
159169
])
@@ -194,9 +204,16 @@ ${ENDPOINT_DEFINITIONS[data.endpoint]?.generateQuery(data.endpointData)}
194204
return acc
195205
}, {})
196206

197-
const conditions = data.thresholds
198-
.map(threshold => THRESHOLD_TYPES[threshold.type].condition(threshold))
199-
.join(' and ')
207+
if (!data.measurement) {
208+
throw new Error('please select a measurement.')
209+
}
210+
const measurement = `(r) => r["_measurement"] == "${data.measurement}"`
211+
212+
const conditions =
213+
`(r) => ` +
214+
data.thresholds
215+
.map(threshold => THRESHOLD_TYPES[threshold.type].condition(threshold))
216+
.join(' and ')
200217

201218
const imports = parse(`
202219
import "strings"
@@ -238,7 +255,10 @@ task_data = ${format_from_js_file(ast)}
238255
trigger = ${conditions}
239256
messageFn = (r) => ("${data.message}")
240257
241-
${ENDPOINT_DEFINITIONS[data.endpoint]?.generateQuery(data.endpointData)}`
258+
${ENDPOINT_DEFINITIONS[data.endpoint]?.generateQuery(
259+
data.endpointData,
260+
measurement
261+
)}`
242262

243263
const newAST = parse(newQuery)
244264

@@ -276,6 +296,7 @@ ${ENDPOINT_DEFINITIONS[data.endpoint]?.generateQuery(data.endpointData)}`
276296
data.offset,
277297
data.endpointData,
278298
data.endpoint,
299+
data.measurement,
279300
data.thresholds,
280301
data.message,
281302
])
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import React, {FC, useCallback, useContext} from 'react'
2+
import {
3+
ComponentSize,
4+
FlexBox,
5+
AlignItems,
6+
TextBlock,
7+
Dropdown,
8+
FlexDirection,
9+
ComponentStatus,
10+
} from '@influxdata/clockface'
11+
12+
import {PipeContext} from 'src/flows/context/pipe'
13+
import 'src/flows/pipes/Notification/Threshold.scss'
14+
15+
// Utils
16+
import {event} from 'src/cloud/utils/reporting'
17+
18+
interface Props {
19+
readOnly?: boolean
20+
}
21+
22+
const Measurement: FC<Props> = ({readOnly}) => {
23+
const {data, update, results} = useContext(PipeContext)
24+
const {measurement} = data
25+
26+
const measurements = Array.from(
27+
new Set(results.parsed.table.columns['_measurement']?.data as string[])
28+
)
29+
30+
const onSelect = useCallback(
31+
(measurement: string) => {
32+
event('Changed Notification Measurement')
33+
update({measurement})
34+
},
35+
[measurements, update]
36+
)
37+
38+
const menuItems = measurements.map(key => (
39+
<Dropdown.Item
40+
testID="dropdown-item--measurement"
41+
key={key}
42+
value={key}
43+
onClick={() => onSelect(key)}
44+
selected={key === measurement}
45+
title={key}
46+
disabled={!!readOnly}
47+
>
48+
{key}
49+
</Dropdown.Item>
50+
))
51+
52+
const menu = onCollapse => (
53+
<Dropdown.Menu onCollapse={onCollapse}>{menuItems}</Dropdown.Menu>
54+
)
55+
56+
const menuButton = (active, onClick) => (
57+
<Dropdown.Button
58+
testID="dropdown--measurements"
59+
onClick={onClick}
60+
active={active}
61+
size={ComponentSize.Medium}
62+
status={!!readOnly ? ComponentStatus.Disabled : ComponentStatus.Default}
63+
>
64+
{measurement || 'Select a measurement'}
65+
</Dropdown.Button>
66+
)
67+
68+
return (
69+
<FlexBox
70+
direction={FlexDirection.Row}
71+
margin={ComponentSize.Medium}
72+
alignItems={AlignItems.FlexStart}
73+
testID="component-spacer"
74+
style={{padding: '24px 0'}}
75+
>
76+
<TextBlock
77+
testID="measurement-value-text-block"
78+
text="For"
79+
style={{minWidth: 56, textAlign: 'center'}}
80+
/>
81+
<FlexBox.Child grow={0}>
82+
<Dropdown menu={menu} button={menuButton} />
83+
</FlexBox.Child>
84+
</FlexBox>
85+
)
86+
}
87+
88+
export default Measurement

src/flows/pipes/Notification/Threshold.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const Threshold: FC<Props> = ({readOnly}) => {
5454
}
5555

5656
threshold.type = type
57-
threshold.field = threshold.field || '_value'
57+
threshold.field = threshold?.field
5858

5959
let updatedThreshold = thresholds
6060

@@ -67,7 +67,7 @@ const Threshold: FC<Props> = ({readOnly}) => {
6767
type: deadmanType,
6868
deadmanCheckValue: '5s',
6969
deadmanStopValue: '90s',
70-
field: threshold.field || '_value',
70+
field: threshold.field || 'Select a numeric field',
7171
},
7272
]
7373
} else {
@@ -104,6 +104,7 @@ const Threshold: FC<Props> = ({readOnly}) => {
104104
})
105105
.map(([key, value]) => (
106106
<Dropdown.Item
107+
testID="dropdown-item--threshold-field"
107108
key={key}
108109
value={key}
109110
onClick={type => setThresholdType(type, index)}
@@ -119,6 +120,7 @@ const Threshold: FC<Props> = ({readOnly}) => {
119120
)
120121
const menuButton = (active, onClick) => (
121122
<Dropdown.Button
123+
testID="dropdown--threshold-fields"
122124
onClick={onClick}
123125
active={active}
124126
size={ComponentSize.Medium}

src/flows/pipes/Notification/endpoints/AWS/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,18 @@ export default register => {
2626
['array', 'http', 'influxdata/influxdb/secrets']
2727
.map(i => `import "${i}"`)
2828
.join('\n'),
29-
generateQuery: data => `task_data
29+
generateQuery: (data, measurement) => `task_data
3030
|> schema["fieldsAsCols"]()
3131
|> set(key: "_notebook_link", value: "${window.location.href}")
32+
|> filter(fn: ${measurement})
3233
|> monitor["check"](
3334
data: check,
3435
messageFn: messageFn,
3536
crit: trigger,
3637
)
38+
|> filter(fn: trigger)
39+
|> keep(columns: ["_value", "_time", "_measurement"])
40+
|> limit(n: 1, offset: 0)
3741
|> monitor["notify"](
3842
data: notification,
3943
endpoint: ((r) => {

src/flows/pipes/Notification/endpoints/HTTP/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export default register => {
2323
['array', 'http', 'influxdata/influxdb/secrets', 'json']
2424
.map(i => `import "${i}"`)
2525
.join('\n'),
26-
generateQuery: data => {
26+
generateQuery: (data, measurement) => {
2727
const headers = [['"Content-Type"', '"application/json"']]
2828
let prefixSecrets = ''
2929

@@ -55,11 +55,15 @@ export default register => {
5555
task_data
5656
|> schema["fieldsAsCols"]()
5757
|> set(key: "_notebook_link", value: "${window.location.href}")
58+
|> filter(fn: ${measurement})
5859
|> monitor["check"](
5960
data: check,
6061
messageFn: messageFn,
6162
crit: trigger,
6263
)
64+
|> filter(fn: trigger)
65+
|> keep(columns: ["_value", "_time", "_measurement"])
66+
|> limit(n: 1, offset: 0)
6367
|> monitor["notify"](data: notification, endpoint: http.endpoint(url: "${data.url}")(
6468
mapFn: (r) => {
6569
body = {r with _version: 1}

src/flows/pipes/Notification/endpoints/Mailgun/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export default register => {
2121
['array', 'http', 'influxdata/influxdb/secrets']
2222
.map(i => `import "${i}"`)
2323
.join('\n'),
24-
generateQuery: data => {
24+
generateQuery: (data, measurement) => {
2525
const subject = encodeURIComponent('InfluxDB Alert')
2626
const fromEmail = `mailgun@${data.domain}`
2727

@@ -31,11 +31,15 @@ auth = http.basicAuth(u: "api", p: "\${apiKey}")
3131
task_data
3232
|> schema["fieldsAsCols"]()
3333
|> set(key: "_notebook_link", value: "${window.location.href}")
34+
|> filter(fn: ${measurement})
3435
|> monitor["check"](
3536
data: check,
3637
messageFn: messageFn,
3738
crit: trigger,
3839
)
40+
|> filter(fn: trigger)
41+
|> keep(columns: ["_value", "_time", "_measurement"])
42+
|> limit(n: 1, offset: 0)
3943
|> monitor["notify"](
4044
data: notification,
4145
endpoint: http.endpoint(url: "https://api.mailgun.net/v3/${data.domain}/messages")(

src/flows/pipes/Notification/endpoints/Mailjet/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,18 @@ export default register => {
2424
['array', 'http', 'influxdata/influxdb/secrets', 'json']
2525
.map(i => `import "${i}"`)
2626
.join('\n'),
27-
generateQuery: data => `task_data
27+
generateQuery: (data, measurement) => `task_data
2828
|> schema["fieldsAsCols"]()
2929
|> set(key: "_notebook_link", value: "${window.location.href}")
30-
|> monitor["check"](
30+
|> filter(fn: ${measurement})
31+
|> monitor["check"](
3132
data: check,
3233
messageFn: messageFn,
3334
crit: trigger,
3435
)
36+
|> filter(fn: trigger)
37+
|> keep(columns: ["_value", "_time", "_measurement"])
38+
|> limit(n: 1, offset: 0)
3539
|> monitor["notify"](
3640
data: notification,
3741
endpoint: http.endpoint(url: "${data.url}")(

src/flows/pipes/Notification/endpoints/PagerDuty/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,18 @@ export default register => {
2121
['pagerduty', 'influxdata/influxdb/secrets']
2222
.map(i => `import "${i}"`)
2323
.join('\n'),
24-
generateQuery: data => `task_data
24+
generateQuery: (data, measurement) => `task_data
2525
|> schema["fieldsAsCols"]()
26-
|> set(key: "_notebook_link", value: "${window.location.href}")
26+
|> set(key: "_notebook_link", value: "${window.location.href}")
27+
|> filter(fn: ${measurement})
2728
|> monitor["check"](
2829
data: check,
2930
messageFn: messageFn,
3031
crit: trigger,
3132
)
33+
|> filter(fn: trigger)
34+
|> keep(columns: ["_value", "_time", "_measurement"])
35+
|> limit(n: 1, offset: 0)
3236
|> monitor["notify"](data: notification, endpoint: pagerduty.endpoint()(mapFn: (r) => ({ r with
3337
routingKey: "${data.key}",
3438
client: "influxdata",

0 commit comments

Comments
 (0)