Skip to content

Commit 254dfb6

Browse files
DuologicDuologic
authored
fix(mixin-utils): remove std.prune and use std.filter instead (#1441)
* fix(mixin-utils): remove std.prune and use std.filter instead The use of `std.prune(std.map())` is slow and uses a big stack, on big rulesets this can cause a 'RUNTIME ERROR: max stack frames exceeded.'. It is better to use `std.filter()` to remove an element from an array. * fix: support both object and array as input * nit: remove implicit plus * fix: don't override field visibility * tests: add test cases * tests: ensure both tests suites get run
1 parent f5a7f45 commit 254dfb6

File tree

3 files changed

+299
-15
lines changed

3 files changed

+299
-15
lines changed

‎mixin-utils/test/Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ vendor jsonnetfile.lock.json: jsonnetfile.json
44
jb install
55

66
tests: jsonnetfile.lock.json vendor
7-
jsonnet -J vendor/ test_*.libsonnet
7+
jsonnet -J vendor/ ./test_native-classic-histogram.libsonnet
8+
jsonnet -J vendor/ ./test_remove_rules.libsonnet

‎mixin-utils/test/test_remove_rules.libsonnet

Lines changed: 284 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,284 @@
1+
local utils = import '../utils.libsonnet';
2+
local test = import 'github.com/jsonnet-libs/testonnet/main.libsonnet';
3+
4+
local config = {
5+
prometheusAlerts: {
6+
groups: [
7+
{
8+
name: 'group1',
9+
rules: [
10+
{
11+
alert: 'alert1',
12+
},
13+
{
14+
alert: 'alert2',
15+
},
16+
],
17+
},
18+
{
19+
name: 'group2',
20+
rules: [
21+
{
22+
alert: 'alert3',
23+
},
24+
{
25+
alert: 'alert4',
26+
},
27+
],
28+
},
29+
],
30+
},
31+
prometheusRules: {
32+
groups: [
33+
{
34+
name: 'group3',
35+
rules: [
36+
{
37+
record: 'record1',
38+
},
39+
{
40+
record: 'record2',
41+
},
42+
],
43+
},
44+
{
45+
name: 'group4',
46+
rules: [
47+
{
48+
record: 'record3',
49+
},
50+
{
51+
record: 'record4',
52+
},
53+
],
54+
},
55+
],
56+
},
57+
};
58+
59+
test.new(std.thisFile)
60+
61+
+ test.case.new(
62+
'removeAlertRuleGroup',
63+
test.expect.eq(
64+
actual=config + utils.removeAlertRuleGroup('group1'),
65+
expected={
66+
prometheusAlerts: {
67+
groups: [
68+
{
69+
name: 'group2',
70+
rules: [
71+
{
72+
alert: 'alert3',
73+
},
74+
{
75+
alert: 'alert4',
76+
},
77+
],
78+
},
79+
],
80+
},
81+
prometheusRules: {
82+
groups: [
83+
{
84+
name: 'group3',
85+
rules: [
86+
{
87+
record: 'record1',
88+
},
89+
{
90+
record: 'record2',
91+
},
92+
],
93+
},
94+
{
95+
name: 'group4',
96+
rules: [
97+
{
98+
record: 'record3',
99+
},
100+
{
101+
record: 'record4',
102+
},
103+
],
104+
},
105+
],
106+
},
107+
}
108+
)
109+
)
110+
111+
+ test.case.new(
112+
'removeRecordingRuleGroup',
113+
test.expect.eq(
114+
actual=config + utils.removeRecordingRuleGroup('group4'),
115+
expected={
116+
prometheusAlerts: {
117+
groups: [
118+
{
119+
name: 'group1',
120+
rules: [
121+
{
122+
alert: 'alert1',
123+
},
124+
{
125+
alert: 'alert2',
126+
},
127+
],
128+
},
129+
{
130+
name: 'group2',
131+
rules: [
132+
{
133+
alert: 'alert3',
134+
},
135+
{
136+
alert: 'alert4',
137+
},
138+
],
139+
},
140+
],
141+
},
142+
prometheusRules: {
143+
groups: [
144+
{
145+
name: 'group3',
146+
rules: [
147+
{
148+
record: 'record1',
149+
},
150+
{
151+
record: 'record2',
152+
},
153+
],
154+
},
155+
],
156+
},
157+
}
158+
)
159+
)
160+
161+
+ test.case.new(
162+
'removeAlertRuleGroup with groupname from recording rules (noop)',
163+
test.expect.eq(
164+
actual=config + utils.removeAlertRuleGroup('group4'),
165+
expected=config
166+
)
167+
)
168+
+ test.case.new(
169+
'removeRecordingRuleGroup with groupname from alert rules (noop)',
170+
test.expect.eq(
171+
actual=config + utils.removeRecordingRuleGroup('group2'),
172+
expected=config
173+
)
174+
)
175+
176+
+ test.case.new(
177+
'removeAlerts',
178+
test.expect.eq(
179+
actual=config + utils.removeAlerts(['alert1', 'alert4']),
180+
expected={
181+
prometheusAlerts: {
182+
groups: [
183+
{
184+
name: 'group1',
185+
rules: [
186+
{
187+
alert: 'alert2',
188+
},
189+
],
190+
},
191+
{
192+
name: 'group2',
193+
rules: [
194+
{
195+
alert: 'alert3',
196+
},
197+
],
198+
},
199+
],
200+
},
201+
prometheusRules: {
202+
groups: [
203+
{
204+
name: 'group3',
205+
rules: [
206+
{
207+
record: 'record1',
208+
},
209+
{
210+
record: 'record2',
211+
},
212+
],
213+
},
214+
{
215+
name: 'group4',
216+
rules: [
217+
{
218+
record: 'record3',
219+
},
220+
{
221+
record: 'record4',
222+
},
223+
],
224+
},
225+
],
226+
},
227+
}
228+
)
229+
)
230+
231+
+ test.case.new(
232+
'removeAlerts - object (backwards compatible)',
233+
test.expect.eq(
234+
actual=config + utils.removeAlerts({ alert1: {}, alert4: {} }),
235+
expected={
236+
prometheusAlerts: {
237+
groups: [
238+
{
239+
name: 'group1',
240+
rules: [
241+
{
242+
alert: 'alert2',
243+
},
244+
],
245+
},
246+
{
247+
name: 'group2',
248+
rules: [
249+
{
250+
alert: 'alert3',
251+
},
252+
],
253+
},
254+
],
255+
},
256+
prometheusRules: {
257+
groups: [
258+
{
259+
name: 'group3',
260+
rules: [
261+
{
262+
record: 'record1',
263+
},
264+
{
265+
record: 'record2',
266+
},
267+
],
268+
},
269+
{
270+
name: 'group4',
271+
rules: [
272+
{
273+
record: 'record3',
274+
},
275+
{
276+
record: 'record4',
277+
},
278+
],
279+
},
280+
],
281+
},
282+
}
283+
)
284+
)

‎mixin-utils/utils.libsonnet

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -392,18 +392,16 @@ local g = import 'grafana-builder/grafana.libsonnet';
392392
for group in groups
393393
],
394394

395-
removeRuleGroup(ruleName):: {
396-
local removeRuleGroup(rule) = if rule.name == ruleName then null else rule,
397-
local currentRuleGroups = super.groups,
398-
groups: std.prune(std.map(removeRuleGroup, currentRuleGroups)),
395+
removeRuleGroup(groupName):: {
396+
groups: std.filter(function(group) group.name != groupName, super.groups),
399397
},
400398

401399
removeAlertRuleGroup(ruleName):: {
402-
prometheusAlerts+:: $.removeRuleGroup(ruleName),
400+
prometheusAlerts+: $.removeRuleGroup(ruleName),
403401
},
404402

405403
removeRecordingRuleGroup(ruleName):: {
406-
prometheusRules+:: $.removeRuleGroup(ruleName),
404+
prometheusRules+: $.removeRuleGroup(ruleName),
407405
},
408406

409407
overrideAlerts(overrides):: {
@@ -412,19 +410,20 @@ local g = import 'grafana-builder/grafana.libsonnet';
412410
then rule + overrides[rule.alert]
413411
else rule,
414412
local overrideInGroup(group) = group { rules: std.map(overrideRule, super.rules) },
415-
prometheusAlerts+:: {
413+
prometheusAlerts+: {
416414
groups: std.map(overrideInGroup, super.groups),
417415
},
418416
},
419417

420418
removeAlerts(alerts):: {
421-
local removeRule(rule) =
422-
if 'alert' in rule && std.objectHas(alerts, rule.alert)
423-
then {}
424-
else rule,
425-
local removeInGroup(group) = group { rules: std.map(removeRule, super.rules) },
426-
prometheusAlerts+:: {
427-
groups: std.prune(std.map(removeInGroup, super.groups)),
419+
local alertNames =
420+
if std.isObject(alerts)
421+
then std.objectFields(alerts)
422+
else alerts,
423+
local removeRule(rule) = !std.member(alertNames, std.get(rule, 'alert', '')),
424+
local removeInGroup(group) = group { rules: std.filter(removeRule, super.rules) },
425+
prometheusAlerts+: {
426+
groups: std.map(removeInGroup, super.groups),
428427
},
429428
},
430429
}

0 commit comments

Comments
 (0)